From e032fa181525ee12654bb834a132c0cc84c9b09a Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Thu, 30 Apr 2026 18:47:51 -0700 Subject: [PATCH 01/19] Suggestions: Register inline RichText format types for proposed adds and deletes Phase A of #77867. Registers two display-oriented format types that later phases will apply programmatically to the suggest-mode overlay so reviewers and suggesters see proposed deletions struck through and proposed additions highlighted inline within RichText, alongside the existing sidebar diff summary. - gutenberg/suggested-deletion () - gutenberg/suggested-addition () Both formats are non-interactive (no toolbar UI) and registered idempotently so the editor bootstrap and integration tests can both trigger registration safely. CSS uses --wp-admin-theme-color and a faint background so the marks read on light and dark themes. Phase B (input interception) and Phase C (apply/reject transforms) follow in subsequent PRs. --- .../src/components/suggestion-mode/index.js | 10 ++ .../suggestion-mode/inline-formats.js | 65 ++++++++++++ .../src/components/suggestion-mode/style.scss | 22 +++++ .../suggestion-mode/test/inline-formats.js | 98 +++++++++++++++++++ 4 files changed, 195 insertions(+) create mode 100644 packages/editor/src/components/suggestion-mode/inline-formats.js create mode 100644 packages/editor/src/components/suggestion-mode/test/inline-formats.js diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index 19eaec10652496..a6aee8bbad1b60 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -1,3 +1,13 @@ +// Side-effect import: registers the inline RichText format types used to +// render proposed adds and deletes inside the editor canvas. Phase A of +// #77867; consumers of those formats land in subsequent phases. +import './inline-formats'; + +export { + SUGGESTED_DELETION_FORMAT, + SUGGESTED_ADDITION_FORMAT, + registerSuggestionFormats, +} from './inline-formats'; export { SuggestionOverlayProvider, useSuggestionOverlay, diff --git a/packages/editor/src/components/suggestion-mode/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js new file mode 100644 index 00000000000000..4ee2f9c0f5404a --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -0,0 +1,65 @@ +/** + * WordPress dependencies + */ +import { registerFormatType } from '@wordpress/rich-text'; +import { __ } from '@wordpress/i18n'; + +/** + * Inline RichText format types used by Suggest mode to render proposed text + * changes within a block, alongside the sidebar diff summary. + * + * Both formats are registered without an `edit` UI so they never appear in + * the block toolbar — they are applied programmatically by the suggest-mode + * interceptor (see #77867 Phase B) when a user types or deletes text in + * Suggest mode. The persisted post is unaffected: while a suggestion is + * open the formatted value lives in the in-memory overlay (the store + * interceptor reverts the live block attribute), and on Accept the + * strip-runs transform removes the deletion characters and unwraps the + * addition format before the value is written back to the block. + * + * `gutenberg/` rather than `core/` because these are suggest-mode-specific, + * not a general-purpose rich-text primitive. + */ +export const SUGGESTED_DELETION_FORMAT = 'gutenberg/suggested-deletion'; +export const SUGGESTED_ADDITION_FORMAT = 'gutenberg/suggested-addition'; + +const suggestedDeletion = { + name: SUGGESTED_DELETION_FORMAT, + title: __( 'Suggested deletion' ), + tagName: 'del', + className: 'has-suggestion-deletion', + interactive: false, + object: false, + edit: () => null, +}; + +const suggestedAddition = { + name: SUGGESTED_ADDITION_FORMAT, + title: __( 'Suggested addition' ), + tagName: 'ins', + className: 'has-suggestion-addition', + interactive: false, + object: false, + edit: () => null, +}; + +let registered = false; + +/** + * Idempotently register the suggest-mode inline format types. Editor + * bootstrap, the suggest-mode tests, and any future entry points can all + * call this without producing duplicate-format warnings. + */ +export function registerSuggestionFormats() { + if ( registered ) { + return; + } + registerFormatType( SUGGESTED_DELETION_FORMAT, suggestedDeletion ); + registerFormatType( SUGGESTED_ADDITION_FORMAT, suggestedAddition ); + registered = true; +} + +// Register on module import so any code path that pulls the suggest-mode +// package (editor bootstrap, e2e harness, integration tests) ends up with +// the format types available without needing a separate init call. +registerSuggestionFormats(); diff --git a/packages/editor/src/components/suggestion-mode/style.scss b/packages/editor/src/components/suggestion-mode/style.scss index 198ea4b0941a25..35ba3d2db508b3 100644 --- a/packages/editor/src/components/suggestion-mode/style.scss +++ b/packages/editor/src/components/suggestion-mode/style.scss @@ -60,6 +60,28 @@ $suggestion-color: #007017; } } +// Inline diff formats applied to RichText runs while a suggestion is +// pending. The marked value lives in the suggest-mode overlay and is fed +// to the block's RichText components, so the marks render in the editor +// canvas itself — not just the sidebar summary. The persisted post is +// untouched (the store interceptor reverts the live block while the +// suggestion is open; Accept strips the formats before the value lands). +.has-suggestion-deletion { + color: var(--wp-block-synced-color, #cc1818); + text-decoration: line-through; + text-decoration-color: var(--wp-block-synced-color, #cc1818); +} + +.has-suggestion-addition { + color: var(--wp-admin-theme-color, $suggestion-color); + background: rgba(0, 112, 23, 0.08); + text-decoration: underline; + text-decoration-color: var(--wp-admin-theme-color, $suggestion-color); + box-shadow: + -1px 0 0 var(--wp-admin-theme-color, $suggestion-color), + 1px 0 0 var(--wp-admin-theme-color, $suggestion-color); +} + // Inline accept/reject buttons live in the note header, right-aligned next // to the author info. Force both icons to render at the same box so the // checkmark (a thin glyph) visually matches the close glyph next to it, diff --git a/packages/editor/src/components/suggestion-mode/test/inline-formats.js b/packages/editor/src/components/suggestion-mode/test/inline-formats.js new file mode 100644 index 00000000000000..863a77d41fe46c --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/inline-formats.js @@ -0,0 +1,98 @@ +/** + * WordPress dependencies + */ +import { + applyFormat, + create, + toHTMLString, + store as richTextStore, +} from '@wordpress/rich-text'; +import { select } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { + SUGGESTED_DELETION_FORMAT, + SUGGESTED_ADDITION_FORMAT, + registerSuggestionFormats, +} from '../inline-formats'; + +describe( 'suggestion inline formats', () => { + // The module's import side-effect already registers; calling again + // proves the helper is idempotent and safe for editor bootstrap to + // invoke in addition to the import. + beforeAll( () => { + registerSuggestionFormats(); + registerSuggestionFormats(); + } ); + + it( 'registers gutenberg/suggested-deletion as a non-interactive del format', () => { + const fmt = select( richTextStore ).getFormatType( + SUGGESTED_DELETION_FORMAT + ); + expect( fmt ).toBeDefined(); + expect( fmt.tagName ).toBe( 'del' ); + expect( fmt.className ).toBe( 'has-suggestion-deletion' ); + expect( fmt.interactive ).toBe( false ); + } ); + + it( 'registers gutenberg/suggested-addition as a non-interactive ins format', () => { + const fmt = select( richTextStore ).getFormatType( + SUGGESTED_ADDITION_FORMAT + ); + expect( fmt ).toBeDefined(); + expect( fmt.tagName ).toBe( 'ins' ); + expect( fmt.className ).toBe( 'has-suggestion-addition' ); + expect( fmt.interactive ).toBe( false ); + } ); + + it( 'serializes a deletion-formatted range as ', () => { + const value = create( { html: 'Hello' } ); + const formatted = applyFormat( + value, + { type: SUGGESTED_DELETION_FORMAT }, + 1, + 5 + ); + expect( toHTMLString( { value: formatted } ) ).toBe( + 'Hello' + ); + } ); + + it( 'serializes an addition-formatted range as ', () => { + const value = create( { html: 'Hi' } ); + const formatted = applyFormat( + value, + { type: SUGGESTED_ADDITION_FORMAT }, + 0, + 2 + ); + expect( toHTMLString( { value: formatted } ) ).toBe( + 'Hi' + ); + } ); + + it( 'applies both formats independently without merging into a single span', () => { + // A simple "Hello → Hi" diff: keep "H", delete "ello", add "i". + // The marked value contains all original characters plus the new + // ones, with deletion / addition formats spanning the right ranges. + const value = create( { html: 'Helloi' } ); + let formatted = applyFormat( + value, + { type: SUGGESTED_DELETION_FORMAT }, + 1, + 5 + ); + formatted = applyFormat( + formatted, + { type: SUGGESTED_ADDITION_FORMAT }, + 5, + 6 + ); + expect( toHTMLString( { value: formatted } ) ).toBe( + 'Hello' + + 'i' + ); + } ); +} ); From b52f596d0d3ed613b78675ec187dd7bcba199308 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 1 May 2026 09:21:05 -0700 Subject: [PATCH 02/19] Suggestions: Add markContentDiff and stripSuggestionMarks helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A wired up the inline format types but no code applied them. This adds the two helpers the overlay HOC needs to render proposed changes inline: - markContentDiff(baseline, proposed) returns marked HTML where deleted runs are wrapped in and added runs in . Tokenization is the existing word-level wordDiff so we share a definition of "what counts as a change" with the sidebar diff. - stripSuggestionMarks(marked) reverses the wrap so the overlay can store the proposed value rather than the marked rendering — without this, a re-edit through RichText would feed marked HTML back into the overlay and the next render would double up the marks. Eleven new unit tests cover the round-trip, the three golden-path scenarios (text addition, text deletion, mid-string replacement), an inline format change ("world" -> "world"), and the null/undefined edge cases the overlay can pass when an attribute is introduced for the first time. --- .../suggestion-mode/inline-formats.js | 136 ++++++++++++++++++ .../suggestion-mode/test/inline-formats.js | 115 +++++++++++++++ 2 files changed, 251 insertions(+) diff --git a/packages/editor/src/components/suggestion-mode/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js index 4ee2f9c0f5404a..5102168b5a8772 100644 --- a/packages/editor/src/components/suggestion-mode/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -4,6 +4,11 @@ import { registerFormatType } from '@wordpress/rich-text'; import { __ } from '@wordpress/i18n'; +/** + * Internal dependencies + */ +import { wordDiff } from './suggestion-diff'; + /** * Inline RichText format types used by Suggest mode to render proposed text * changes within a block, alongside the sidebar diff summary. @@ -63,3 +68,134 @@ export function registerSuggestionFormats() { // package (editor bootstrap, e2e harness, integration tests) ends up with // the format types available without needing a separate init call. registerSuggestionFormats(); + +const DELETION_CLASS = 'has-suggestion-deletion'; +const ADDITION_CLASS = 'has-suggestion-addition'; + +/** + * Wrap a chunk of HTML in the suggested-deletion ``. Used when an + * incoming run was present in the baseline but absent from the proposed + * value, so reviewers see what the suggester wants to remove. + * + * @param {string} value Run content (already-serialized HTML or plain text). + * @return {string} Wrapped run. + */ +function wrapDeletion( value ) { + return `${ value }`; +} + +/** + * Wrap a chunk of HTML in the suggested-addition ``. Used when an + * incoming run is present in the proposed value but absent from the + * baseline, so reviewers see what the suggester wants to add. + * + * @param {string} value Run content (already-serialized HTML or plain text). + * @return {string} Wrapped run. + */ +function wrapAddition( value ) { + return `${ value }`; +} + +/** + * Build a marked HTML preview of `proposed` relative to `baseline`. Equal + * runs pass through unchanged, runs that exist only in the baseline are + * wrapped in ``, and runs that exist + * only in the proposed value are wrapped in ``. The result is what gets fed back into the block's RichText + * during overlay render so reviewers see deletions struck through and + * additions highlighted inline. + * + * Tokenization is intentionally word-and-whitespace level (delegated to + * `wordDiff`), which is good enough for the golden-path scenarios this PR + * covers — pure text additions, pure text deletions, mid-string + * replacements, and inline-format additions like wrapping a word in + * ``. A whole-token format change (e.g. `world` → + * `world`) surfaces as a delete-then-insert pair, which + * intentionally renders both states so the reviewer can see what changed. + * Edge cases (partial-tag overlap, mixed nested formatting) are out of + * scope here and land in subsequent phases. + * + * Identical inputs short-circuit so the common no-op overlay render + * (e.g. baseline === proposed during an attribute-only suggestion) costs + * nothing. + * + * @param {string|null|undefined} baseline Original attribute value. + * @param {string|null|undefined} proposed Suggested attribute value. + * @return {string} Marked HTML representing the diff. + */ +export function markContentDiff( baseline, proposed ) { + const beforeHtml = + baseline === null || baseline === undefined ? '' : String( baseline ); + const afterHtml = + proposed === null || proposed === undefined ? '' : String( proposed ); + + if ( beforeHtml === afterHtml ) { + return afterHtml; + } + + const segments = wordDiff( beforeHtml, afterHtml ); + let result = ''; + for ( const seg of segments ) { + if ( seg.type === 'equal' ) { + result += seg.value; + } else if ( seg.type === 'insert' ) { + result += wrapAddition( seg.value ); + } else { + result += wrapDeletion( seg.value ); + } + } + return result; +} + +/** + * Reverse `markContentDiff`: drop deletion runs entirely (the suggester + * wants them gone) and unwrap addition runs (the suggester wants their + * inner content to remain). Used on incoming `setAttributes` payloads from + * the block's RichText so the overlay always stores the "proposed" value + * and never the marked rendering — without this, the next render would + * diff baseline against an already-marked value and double up the marks. + * + * Falls back to a string-untouched return when the input contains no + * suggestion classes, so the common edit path doesn't pay the parse cost. + * + * @param {string|null|undefined} marked Possibly-marked HTML. + * @return {string|null|undefined} HTML with suggestion marks removed. + */ +export function stripSuggestionMarks( marked ) { + if ( marked === null || marked === undefined ) { + return marked; + } + const html = String( marked ); + if ( + ! html.includes( DELETION_CLASS ) && + ! html.includes( ADDITION_CLASS ) + ) { + return html; + } + + // Use the DOM rather than regex so nested tags (an addition wrapping a + // ``, or a deletion containing arbitrary inline markup) round- + // trip cleanly. Wrapping in a single root element gives us a stable + // `innerHTML` to read back without losing leading/trailing text nodes. + const doc = new window.DOMParser().parseFromString( + `
${ html }
`, + 'text/html' + ); + const root = doc.body.firstElementChild; + if ( ! root ) { + return html; + } + + for ( const el of root.querySelectorAll( `.${ DELETION_CLASS }` ) ) { + el.remove(); + } + for ( const el of root.querySelectorAll( `.${ ADDITION_CLASS }` ) ) { + const parent = el.parentNode; + while ( el.firstChild ) { + parent.insertBefore( el.firstChild, el ); + } + parent.removeChild( el ); + } + + return root.innerHTML; +} diff --git a/packages/editor/src/components/suggestion-mode/test/inline-formats.js b/packages/editor/src/components/suggestion-mode/test/inline-formats.js index 863a77d41fe46c..d91eee07bbe523 100644 --- a/packages/editor/src/components/suggestion-mode/test/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/test/inline-formats.js @@ -16,6 +16,8 @@ import { SUGGESTED_DELETION_FORMAT, SUGGESTED_ADDITION_FORMAT, registerSuggestionFormats, + markContentDiff, + stripSuggestionMarks, } from '../inline-formats'; describe( 'suggestion inline formats', () => { @@ -96,3 +98,116 @@ describe( 'suggestion inline formats', () => { ); } ); } ); + +describe( 'markContentDiff', () => { + it( 'returns the proposed value untouched when both sides are identical', () => { + expect( markContentDiff( 'Hello world', 'Hello world' ) ).toBe( + 'Hello world' + ); + } ); + + it( 'wraps appended text in ', () => { + // Pure end-of-string addition: "Hello" → "Hello world". The added + // space and word each surface as their own insert segments because + // `wordDiff` tokenizes on whitespace runs. + expect( markContentDiff( 'Hello', 'Hello world' ) ).toBe( + 'Hello' + + ' ' + + 'world' + ); + } ); + + it( 'wraps removed text in ', () => { + // Pure end-of-string deletion: "Hello world" → "Hello". Mirrors the + // addition case but in the delete direction. + expect( markContentDiff( 'Hello world', 'Hello' ) ).toBe( + 'Hello' + + ' ' + + 'world' + ); + } ); + + it( 'marks a mid-string replacement as paired delete + insert runs', () => { + // "Hello world" → "Hello there": shared "Hello ", then `world` + // becomes `there`. Reviewers see both states adjacent so they can + // read the change at a glance. + expect( markContentDiff( 'Hello world', 'Hello there' ) ).toBe( + 'Hello ' + + 'world' + + 'there' + ); + } ); + + it( 'marks an inline format addition as a delete + insert pair around the styled run', () => { + // Bolding "world": "Hello world" → "Hello world". + // The diff sees the bare token replaced by the wrapped one. The + // `` survives inside the `` so the suggestion both + // reads as bold and shows in the addition color treatment. + expect( + markContentDiff( 'Hello world', 'Hello world' ) + ).toBe( + 'Hello ' + + 'world' + + 'world' + ); + } ); + + it( 'coerces null and undefined inputs to empty strings', () => { + // Defensive — overlay payloads can carry `undefined` for an + // attribute that is being introduced for the first time. + expect( markContentDiff( null, 'New' ) ).toBe( + 'New' + ); + expect( markContentDiff( 'Old', undefined ) ).toBe( + 'Old' + ); + } ); +} ); + +describe( 'stripSuggestionMarks', () => { + it( 'is a no-op when the value contains no suggestion classes', () => { + // Fast-path that lets the common edit case skip the DOM parse. + expect( stripSuggestionMarks( 'Hello world' ) ).toBe( 'Hello world' ); + expect( stripSuggestionMarks( 'Hello world' ) ).toBe( + 'Hello world' + ); + } ); + + it( 'removes deletion runs entirely', () => { + // The deleted text is the suggester's "remove this" — accepting + // strips it, so this is the value the overlay should store after + // the round-trip through RichText. + expect( + stripSuggestionMarks( + 'Hello' + ' world' + ) + ).toBe( 'Hello' ); + } ); + + it( 'unwraps addition runs to their inner content', () => { + // Additions are the suggester's "keep this" — accepting unwraps + // them so the inner content (any markup included) is preserved. + expect( + stripSuggestionMarks( + 'Hello ' + + 'world' + ) + ).toBe( 'Hello world' ); + } ); + + it( 'round-trips a marked diff back to the proposed value', () => { + // `markContentDiff` then `stripSuggestionMarks` should land back at + // `proposed`. This is the contract the overlay relies on so a re- + // edit (which feeds the marked HTML back through `setAttributes`) + // doesn't double up the marks on the next render. + const baseline = 'Hello world'; + const proposed = 'Hello there'; + const marked = markContentDiff( baseline, proposed ); + expect( stripSuggestionMarks( marked ) ).toBe( proposed ); + } ); + + it( 'passes null and undefined through untouched', () => { + expect( stripSuggestionMarks( null ) ).toBeNull(); + expect( stripSuggestionMarks( undefined ) ).toBeUndefined(); + } ); +} ); From 6f98b1efbb29b939c5b39e2337ba43210d67473b Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 1 May 2026 09:24:55 -0700 Subject: [PATCH 03/19] Suggestions: Render proposed text changes inline via overlay HOC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires markContentDiff and stripSuggestionMarks into withSuggestionOverlay so RichText attributes like `content` render with the suggestion marks visible in the editor canvas, not just in the sidebar summary. - mergedAttributes now passes the proposed content through markContentDiff before handing it to BlockEdit, so deleted runs render with the red strikethrough and added runs render with the green underline + side- shadow defined in the existing CSS. - wrappedSetAttributes strips suggestion marks from incoming payloads so RichText round-tripping the marked HTML back through onChange doesn't pile additional marks onto the next render. - Marks are skipped while the block is selected so the user's caret is not fighting the value-prop reconciliation on every keystroke. They reappear as soon as focus moves elsewhere — a Docs-like end state that trades live feedback for a cursor that doesn't jump mid-word. - RICH_TEXT_ATTRIBUTE_KEYS is intentionally narrow (just `content`) so primitive attribute changes (align, level, fontSize) keep flowing through the existing block-level outline rather than leaking HTML into string slots. Seven new unit tests exercise the helpers directly; the existing HOC tests continue to pass because the marking is gated on a selected block and the test harness leaves block-editor unregistered (the useSelect fallback returns true, so marks are skipped). --- .../test/with-suggestion-overlay.js | 76 +++++++++ .../with-suggestion-overlay.js | 148 +++++++++++++++++- 2 files changed, 216 insertions(+), 8 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index afacd69920caf1..e65794522f2f46 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -14,6 +14,8 @@ import { store as noticesStore } from '@wordpress/notices'; */ import withSuggestionOverlay, { mergeOverlayAttributes, + applyDiffMarks, + stripMarksFromIncoming, } from '../with-suggestion-overlay'; import { SuggestionOverlayProvider, @@ -273,3 +275,77 @@ describe( 'mergeOverlayAttributes', () => { ).toEqual( { custom: { other: 'new' } } ); } ); } ); + +describe( 'applyDiffMarks', () => { + it( 'returns merged unchanged when no baseline is available', () => { + // New blocks added during a suggestion session never get a + // baseline captured for them — `applyDiffMarks` must be a safe + // no-op rather than throw. + const merged = { content: 'Hello' }; + expect( applyDiffMarks( merged, null ) ).toBe( merged ); + } ); + + it( 'returns merged unchanged when the content attribute is unchanged', () => { + // Attribute-only suggestions (e.g. heading level) don't touch + // `content`; skipping the diff keeps object identity stable so + // React's bail-out on unchanged props still fires. + const merged = { content: 'Hello', level: 3 }; + const baseline = { content: 'Hello', level: 2 }; + expect( applyDiffMarks( merged, baseline ) ).toBe( merged ); + } ); + + it( 'wraps the diff for changed content in del/ins markup', () => { + const result = applyDiffMarks( + { content: 'Hello world', level: 2 }, + { content: 'Hello', level: 2 } + ); + expect( result.content ).toBe( + 'Hello' + + ' ' + + 'world' + ); + // Other attributes pass through untouched. + expect( result.level ).toBe( 2 ); + } ); + + it( 'leaves non-rich-text attributes unmarked even when they change', () => { + // `align: 'left' -> 'right'` is a primitive change; wrapping it + // in HTML would push garbage into a className/string slot. The + // block-level outline already signals these changes. + const merged = { align: 'right' }; + const baseline = { align: 'left' }; + expect( applyDiffMarks( merged, baseline ) ).toBe( merged ); + } ); +} ); + +describe( 'stripMarksFromIncoming', () => { + it( 'returns the payload unchanged when no rich-text key is present', () => { + // Most attribute-only suggestions land here, so the fast-path + // matters for keystroke-rate calls. + const payload = { level: 3 }; + expect( stripMarksFromIncoming( payload ) ).toBe( payload ); + } ); + + it( 'returns the payload unchanged when content has no suggestion marks', () => { + // First-time edits send plain text through; the strip should be + // a structural no-op so React props stay identity-stable. + const payload = { content: 'Hello world' }; + expect( stripMarksFromIncoming( payload ) ).toBe( payload ); + } ); + + it( 'strips suggestion marks from content before they reach the overlay', () => { + // Round-trip case: RichText emits the previously-marked HTML + // back through `setAttributes` after the user keeps typing into + // a marked block. Storing the marked form in the overlay would + // double up the marks on the next render. + const result = stripMarksFromIncoming( { + content: + 'Hello' + + ' world' + + ' there', + level: 2, + } ); + expect( result.content ).toBe( 'Hello there' ); + expect( result.level ).toBe( 2 ); + } ); +} ); diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index 7a5a4981783286..e6c3fe0a9cedf1 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -16,6 +16,110 @@ import { addFilter } from '@wordpress/hooks'; */ import { useSuggestionOverlay } from './overlay-context'; import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; +import { markContentDiff, stripSuggestionMarks } from './inline-formats'; + +const BLOCK_EDITOR_STORE_NAME = 'core/block-editor'; + +/** + * Block attribute keys whose values are RichText content and therefore + * benefit from inline diff marking. Marking primitive attributes like + * `align: 'left'` would leak `leftright` into a + * className/string slot, so this set is intentionally narrow. Most of the + * golden-path scenarios this PR covers (paragraph text edits, heading + * text edits, bolding a word) live on the `content` attribute; richer + * block coverage lands in subsequent phases. + */ +const RICH_TEXT_ATTRIBUTE_KEYS = new Set( [ 'content' ] ); + +function isStringLike( value ) { + if ( typeof value === 'string' ) { + return true; + } + // `RichTextData` is an object that stringifies to its HTML form; check + // for a usable `toString` rather than instanceof so we don't take a + // hard dependency on the rich-text package's internal class. + return ( + value !== null && + value !== undefined && + typeof value.toString === 'function' && + value.toString !== Object.prototype.toString + ); +} + +/** + * Walk the merged attribute set and replace each rich-text value with its + * baseline-vs-proposed marked diff. Skips attributes whose value matches + * the baseline (no change to mark) or whose baseline is missing (the + * suggestion has nothing to diff against). + * + * @param {Object} merged Output of `mergeOverlayAttributes`. + * @param {Object} baseline Baseline attributes captured when the suggestion + * began. + * @return {Object} `merged` with rich-text attributes replaced by marked + * HTML, or `merged` unchanged when nothing was eligible. + */ +function applyDiffMarks( merged, baseline ) { + if ( ! merged || ! baseline ) { + return merged; + } + let result = null; + for ( const key of RICH_TEXT_ATTRIBUTE_KEYS ) { + if ( ! Object.prototype.hasOwnProperty.call( merged, key ) ) { + continue; + } + const proposed = merged[ key ]; + const original = baseline[ key ]; + if ( ! isStringLike( proposed ) || ! isStringLike( original ) ) { + continue; + } + const proposedStr = String( proposed ); + const originalStr = String( original ); + if ( proposedStr === originalStr ) { + continue; + } + if ( ! result ) { + result = { ...merged }; + } + result[ key ] = markContentDiff( originalStr, proposedStr ); + } + return result ?? merged; +} + +/** + * Strip suggestion marks from incoming `setAttributes` payloads. RichText + * round-trips its `value` prop through serialization on every keystroke, + * so a previously-marked render would otherwise come back into the overlay + * as marked HTML and the next mark pass would double up. Stripping at the + * intercept keeps the overlay holding the "proposed" value rather than the + * marked rendering. + * + * @param {Object} nextAttributes Incoming attribute payload. + * @return {Object} Payload with rich-text values normalized. + */ +function stripMarksFromIncoming( nextAttributes ) { + if ( ! nextAttributes ) { + return nextAttributes; + } + let result = null; + for ( const key of RICH_TEXT_ATTRIBUTE_KEYS ) { + if ( ! Object.prototype.hasOwnProperty.call( nextAttributes, key ) ) { + continue; + } + const value = nextAttributes[ key ]; + if ( ! isStringLike( value ) ) { + continue; + } + const stripped = stripSuggestionMarks( String( value ) ); + if ( stripped === String( value ) ) { + continue; + } + if ( ! result ) { + result = { ...nextAttributes }; + } + result[ key ] = stripped; + } + return result ?? nextAttributes; +} /** * Attribute keys whose values are known to be object-valued and therefore @@ -72,29 +176,57 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const attributesRef = useRef( attributes ); attributesRef.current = attributes; - const overlayAttributes = entries[ clientId ]?.overlayAttributes ?? null; + const overlayEntry = entries[ clientId ]; + const overlayAttributes = overlayEntry?.overlayAttributes ?? null; + const baselineAttributes = overlayEntry?.baselineAttributes ?? null; + + // Whether this block is the currently selected one. While selected, we + // skip applying inline diff marks so RichText's value-prop reconciliation + // doesn't fight the user's caret on every keystroke. Marks reappear as + // soon as the user clicks/tabs away. Defaults to `true` so any + // environment without the block-editor store registered (unit tests of + // this HOC) skips marking too — production always has the store. + const isSelected = useSelect( + ( select ) => { + const blockEditor = select( BLOCK_EDITOR_STORE_NAME ); + if ( ! blockEditor?.isBlockSelected ) { + return true; + } + return blockEditor.isBlockSelected( clientId ); + }, + [ clientId ] + ); // Does an overlay entry currently exist for this block? This is the // source of truth; `captureBaseline` only creates an entry when there // isn't one, so we can skip the dispatch when we already know there is. // Relying on a local ref was fragile — it didn't reset after the entry // was cleared (auto-save trash, orphan prune, intent-switch). - const entryExists = !! entries[ clientId ]; + const entryExists = !! overlayEntry; const wrappedSetAttributes = useCallback( ( nextAttributes ) => { if ( ! entryExists ) { captureBaseline( clientId, name, attributesRef.current ); } - setOverlayAttributes( clientId, nextAttributes ); + // Strip any suggestion marks RichText round-tripped from a + // previously marked render before storing in the overlay; see + // `stripMarksFromIncoming` for the rationale. + setOverlayAttributes( + clientId, + stripMarksFromIncoming( nextAttributes ) + ); }, [ clientId, name, captureBaseline, setOverlayAttributes, entryExists ] ); - const mergedAttributes = useMemo( - () => mergeOverlayAttributes( attributes, overlayAttributes ), - [ attributes, overlayAttributes ] - ); + const mergedAttributes = useMemo( () => { + const merged = mergeOverlayAttributes( attributes, overlayAttributes ); + if ( isSelected ) { + return merged; + } + return applyDiffMarks( merged, baselineAttributes ); + }, [ attributes, overlayAttributes, baselineAttributes, isSelected ] ); return ( Date: Fri, 1 May 2026 09:29:58 -0700 Subject: [PATCH 04/19] Suggestions: Add e2e coverage for inline diff marks in the editor canvas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new playwright tests exercise the golden-path scenarios the PR description promises: - A trailing text addition lands in the paragraph wrapped in after the block deselects, so reviewers see the new word with the addition color treatment. - A trailing text deletion (selected range backspaced) survives in the paragraph wrapped in so the reviewer can read what the suggester wants removed. - Bolding a word renders the suggestion as word, keeping both the new format AND the addition treatment, with the pre-bold version surfacing as a paired so the reviewer can see what changed. Each test deselects the block via clearSelectedBlock() before asserting, because withSuggestionOverlay deliberately skips marking while a block is selected (avoiding RichText caret jumps during typing). The marks appear once focus moves elsewhere — Docs-like end state without the custom rendering pipeline. --- .../editor/various/suggestion-mode.spec.js | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/test/e2e/specs/editor/various/suggestion-mode.spec.js b/test/e2e/specs/editor/various/suggestion-mode.spec.js index 14e0fff32ed876..aa05985c3d8132 100644 --- a/test/e2e/specs/editor/various/suggestion-mode.spec.js +++ b/test/e2e/specs/editor/various/suggestion-mode.spec.js @@ -88,6 +88,133 @@ test.describe( 'Suggestion mode', () => { await expect( paragraph ).toHaveClass( /is-suggestion-pending/ ); } ); + test( 'shows appended text wrapped in after the block deselects', async ( { + editor, + page, + } ) => { + // Golden-path text addition: type at the end of a paragraph, blur + // the block, and reviewers should see the new run highlighted with + // the addition format. While the block is selected the overlay + // renders the plain proposed value so RichText's caret isn't + // disrupted by value-prop reconciliation; the marks appear after + // blur. + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { content: 'Hello' }, + } ); + + await switchIntent( page, 'Suggest' ); + + const paragraph = editor.canvas + .getByRole( 'document', { name: 'Block: Paragraph' } ) + .first(); + await paragraph.click(); + await page.keyboard.press( 'End' ); + await page.keyboard.type( ' world' ); + + // Auto-save fires after the debounce; deselect so the inline marks + // render in place of the plain proposed value. + await waitForSuggestionSaved( page ); + await page.evaluate( () => { + window.wp.data.dispatch( 'core/block-editor' ).clearSelectedBlock(); + } ); + + // `wordDiff` tokenizes on whitespace runs, so " " and "world" each + // land in their own addition span. Filter to the word-bearing run. + await expect( + paragraph + .locator( 'ins.has-suggestion-addition' ) + .filter( { hasText: 'world' } ) + ).toBeVisible(); + + // Sanity: the original text still reads as part of the paragraph. + await expect( paragraph ).toContainText( 'Hello' ); + } ); + + test( 'shows deleted text wrapped in after the block deselects', async ( { + editor, + page, + pageUtils, + } ) => { + // Golden-path text deletion: select a trailing word and press + // Backspace. The deleted run survives in the suggestion preview as + // a struck-through fragment so the reviewer can see what the + // suggester wants to remove. + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { content: 'Hello world' }, + } ); + + await switchIntent( page, 'Suggest' ); + + const paragraph = editor.canvas + .getByRole( 'document', { name: 'Block: Paragraph' } ) + .first(); + await paragraph.click(); + await page.keyboard.press( 'End' ); + // Select " world" (six characters) then delete. + await pageUtils.pressKeys( 'shift+ArrowLeft', { times: 6 } ); + await page.keyboard.press( 'Backspace' ); + + await waitForSuggestionSaved( page ); + await page.evaluate( () => { + window.wp.data.dispatch( 'core/block-editor' ).clearSelectedBlock(); + } ); + + // The deleted run tokenizes into " " + "world"; filter to the word + // so the assertion reads precisely against the text we removed. + await expect( + paragraph + .locator( 'del.has-suggestion-deletion' ) + .filter( { hasText: 'world' } ) + ).toBeVisible(); + } ); + + test( 'shows a newly bolded word wrapped in with preserved', async ( { + editor, + page, + pageUtils, + } ) => { + // Golden-path inline-format change: select a word, press Cmd/Ctrl+B + // to bold it, and the suggestion preview should mark the run as + // added (so it reads as both bold AND highlighted) without losing + // the underlying markup. The diff sees the bare token + // replaced by the wrapped one, so it surfaces as a delete + insert + // pair around the styled run. + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { content: 'Hello world' }, + } ); + + await switchIntent( page, 'Suggest' ); + + const paragraph = editor.canvas + .getByRole( 'document', { name: 'Block: Paragraph' } ) + .first(); + await paragraph.click(); + await page.keyboard.press( 'End' ); + await pageUtils.pressKeys( 'shift+ArrowLeft', { times: 5 } ); + await pageUtils.pressKeys( 'primary+b' ); + + await waitForSuggestionSaved( page ); + await page.evaluate( () => { + window.wp.data.dispatch( 'core/block-editor' ).clearSelectedBlock(); + } ); + + // The bolded run is wrapped in `` so it picks up the addition + // color treatment, with `` preserved inside so it still + // renders as bold. + await expect( + paragraph.locator( 'ins.has-suggestion-addition strong' ) + ).toContainText( 'world' ); + + // And the bare-text version is shown as a deletion so the reviewer + // can see what the run looked like before the suggestion. + await expect( + paragraph.locator( 'del.has-suggestion-deletion' ) + ).toContainText( 'world' ); + } ); + test( 'captures a heading-level change made via the block-switcher variation picker', async ( { editor, page, From 45fec273f671ab7792b79bd5bc2e12eb44cc26e3 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 4 May 2026 13:00:06 -0700 Subject: [PATCH 05/19] Suggestions: Tighten inline docs across the suggest-mode overlay path Update the suggest-mode JSDoc and inline comments so reviewers can read the files top-down without cross-referencing the tracking issue. - inline-formats.js: rewrite the registration docblock to describe the overlay HOC consumer; update markContentDiff to drop the stale golden-path-of-this-PR phrasing in favor of the v1 scenarios in #77867. - index.js: barrel comment now points at the actual consumer rather than promising a future phase. - with-suggestion-overlay.js: add a top-of-file architecture note tracing data flow from BlockEdit through the overlay to markContentDiff and back; promote isStringLike's intent to JSDoc; document mergeOverlayAttributes explicitly; explain the capture-baseline-on-first-write pattern; surface the isSelected short-circuit at the call site. - test files: add header comments listing the contracts each suite covers (HOC pass-through, merge semantics, diff/strip round-trip). - e2e spec: add a header naming the three golden-path scenarios; rename the scenario tests to 'add | delete | style' so they map 1:1 to the PR description bullets. No behavior change. --- .../src/components/suggestion-mode/index.js | 7 ++- .../suggestion-mode/inline-formats.js | 28 +++++---- .../test/with-suggestion-overlay.js | 15 +++++ .../with-suggestion-overlay.js | 58 ++++++++++++++++++- .../editor/various/suggestion-mode.spec.js | 22 ++++++- 5 files changed, 106 insertions(+), 24 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index a6aee8bbad1b60..961f13a76f3197 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -1,6 +1,7 @@ -// Side-effect import: registers the inline RichText format types used to -// render proposed adds and deletes inside the editor canvas. Phase A of -// #77867; consumers of those formats land in subsequent phases. +// Side-effect import: registers the inline RichText format types +// (`gutenberg/suggested-deletion`, `gutenberg/suggested-addition`) consumed +// by `with-suggestion-overlay.js` to render proposed adds and deletes inside +// the editor canvas. See #77867. import './inline-formats'; export { diff --git a/packages/editor/src/components/suggestion-mode/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js index 5102168b5a8772..81db1f45d2c032 100644 --- a/packages/editor/src/components/suggestion-mode/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -14,13 +14,12 @@ import { wordDiff } from './suggestion-diff'; * changes within a block, alongside the sidebar diff summary. * * Both formats are registered without an `edit` UI so they never appear in - * the block toolbar — they are applied programmatically by the suggest-mode - * interceptor (see #77867 Phase B) when a user types or deletes text in - * Suggest mode. The persisted post is unaffected: while a suggestion is - * open the formatted value lives in the in-memory overlay (the store - * interceptor reverts the live block attribute), and on Accept the - * strip-runs transform removes the deletion characters and unwraps the - * addition format before the value is written back to the block. + * the block toolbar — they are applied programmatically by the overlay HOC + * (`with-suggestion-overlay.js`), which diffs baseline against proposed on + * each render and feeds the marked HTML back into the block. The persisted + * post is unaffected: the overlay stores the *clean* proposed value (marks + * are stripped at the intercept), and the existing Accept/Reject path keeps + * working unchanged because there is no marked value on the wire. * * `gutenberg/` rather than `core/` because these are suggest-mode-specific, * not a general-purpose rich-text primitive. @@ -106,14 +105,13 @@ function wrapAddition( value ) { * additions highlighted inline. * * Tokenization is intentionally word-and-whitespace level (delegated to - * `wordDiff`), which is good enough for the golden-path scenarios this PR - * covers — pure text additions, pure text deletions, mid-string - * replacements, and inline-format additions like wrapping a word in - * ``. A whole-token format change (e.g. `world` → - * `world`) surfaces as a delete-then-insert pair, which - * intentionally renders both states so the reviewer can see what changed. - * Edge cases (partial-tag overlap, mixed nested formatting) are out of - * scope here and land in subsequent phases. + * `wordDiff`), which is good enough for the v1 scenarios listed in #77867 + * — pure text additions, pure text deletions, mid-string replacements, and + * inline-format additions like wrapping a word in ``. A whole-token + * format change (e.g. `world` → `world`) surfaces as a + * delete-then-insert pair, which intentionally renders both states so the + * reviewer can see what changed. Edge cases (partial-tag overlap, mixed + * nested formatting) are tracked as the Phase D follow-up. * * Identical inputs short-circuit so the common no-op overlay render * (e.g. baseline === proposed during an attribute-only suggestion) costs diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index e65794522f2f46..7f8c15fd3435c8 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -1,3 +1,18 @@ +/** + * Tests for `with-suggestion-overlay.js`. Coverage falls into three groups: + * + * 1. `withSuggestionOverlay` HOC — pass-through outside Suggest intent; + * in Suggest intent, diversion of `setAttributes` into the overlay, + * rendering the merged overlay-on-baseline value, surviving an overlay + * clear-and-re-edit cycle. + * 2. `mergeOverlayAttributes` — replace-vs-deep-merge contract for + * overlapping overlay keys, including the `style`/`metadata` deep merge + * that keeps untouched fields alive. + * 3. `applyDiffMarks` / `stripMarksFromIncoming` — the diff/strip round-trip + * that keeps the overlay storing the *clean* proposed value while the + * rendered attributes carry marked HTML. + */ + /** * External dependencies */ diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index e6c3fe0a9cedf1..d7b3e674ef942c 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -1,3 +1,27 @@ +/** + * Suggest-mode overlay HOC. + * + * Data flow while the editor is in `suggest` intent: + * + * BlockEdit + * └─ wrappedSetAttributes ── strip suggestion marks RichText + * round-tripped from a previous + * render, then store the *clean* + * proposed value in the overlay + * └─ overlay (proposed clean) + * └─ SuggestingBlockEdit ── merge baseline + overlay + * └─ markContentDiff ── compute marked HTML on each render + * └─ BlockEdit ── render with deletions/additions visible + * + * The block-editor store stays at the baseline value the entire time the + * suggestion is open; only the rendered `attributes` prop carries the + * marked diff, gated on `! isBlockSelected` so RichText's caret is not + * disrupted while the user is typing into the block. Accept/Reject runs + * against the clean overlay value via the existing `attribute-set` path — + * no marked-value persistence, no schema migration. See #77867 for the + * tradeoff vs. edit-time interception. + */ + /** * External dependencies */ @@ -31,13 +55,19 @@ const BLOCK_EDITOR_STORE_NAME = 'core/block-editor'; */ const RICH_TEXT_ATTRIBUTE_KEYS = new Set( [ 'content' ] ); +/** + * True for plain strings and for objects that stringify to a meaningful HTML + * form (the rich-text package's `RichTextData` is the case we care about). + * Duck-typed against `toString` rather than `instanceof RichTextData` so we + * don't take a hard dependency on the rich-text package's internal class. + * + * @param {*} value Candidate attribute value. + * @return {boolean} True when `String( value )` will produce useful HTML. + */ function isStringLike( value ) { if ( typeof value === 'string' ) { return true; } - // `RichTextData` is an object that stringifies to its HTML form; check - // for a usable `toString` rather than instanceof so we don't take a - // hard dependency on the rich-text package's internal class. return ( value !== null && value !== undefined && @@ -129,6 +159,19 @@ function stripMarksFromIncoming( nextAttributes ) { */ const DEEP_MERGE_KEYS = new Set( [ 'style', 'metadata' ] ); +/** + * Apply an overlay attribute set on top of the block's real attributes for + * rendering. Keys in `DEEP_MERGE_KEYS` (currently `style` and `metadata`) + * are one-level-merged so a partial overlay payload — e.g. tweaking + * `style.color` while leaving `style.typography` alone — preserves the + * untouched fields. Every other key is replaced wholesale, matching + * `setAttributes` semantics for primitive and array values. + * + * @param {Object} base Block's real attributes from the block-editor store. + * @param {Object|null} overlay Pending overlay attributes; `null` is a no-op. + * @return {Object} Merged attributes for rendering. Returns `base` by reference + * when there is no overlay to apply, so React's prop-identity bail-out fires. + */ function mergeOverlayAttributes( base, overlay ) { if ( ! overlay ) { return base; @@ -206,6 +249,11 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const wrappedSetAttributes = useCallback( ( nextAttributes ) => { + // First overlay write for this block snapshots the current + // attributes as the baseline; subsequent writes only record + // overlay deltas. This lets the diff renderer below compare + // "what the block looked like when the suggestion started" + // against "what the suggester is proposing now". if ( ! entryExists ) { captureBaseline( clientId, name, attributesRef.current ); } @@ -222,6 +270,10 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const mergedAttributes = useMemo( () => { const merged = mergeOverlayAttributes( attributes, overlayAttributes ); + // While the block is selected, hand back the plain proposed value so + // the user's caret doesn't fight RichText's value-prop reconciliation + // on every keystroke. The marks reappear as soon as focus moves away + // — see the `isSelected` `useSelect` above for the full rationale. if ( isSelected ) { return merged; } diff --git a/test/e2e/specs/editor/various/suggestion-mode.spec.js b/test/e2e/specs/editor/various/suggestion-mode.spec.js index aa05985c3d8132..12c99b11e604be 100644 --- a/test/e2e/specs/editor/various/suggestion-mode.spec.js +++ b/test/e2e/specs/editor/various/suggestion-mode.spec.js @@ -1,3 +1,19 @@ +/** + * E2E coverage for Suggest mode (#77867). The diff-rendering scenarios below + * are the "golden paths" referenced in the PR description: + * + * - `add — golden path` : type a word, see it wrapped in + * ``. + * - `delete — golden path` : Backspace a word, see it survive in + * ``. + * - `style — golden path` : Cmd/Ctrl+B a word, see it render bold AND + * wrapped in the addition format, with the + * pre-bold version surfacing as a paired ``. + * + * Other tests in this file cover snackbar announcements, auto-save, and + * routing of block-switcher mutations through the store interceptor. + */ + /** * WordPress dependencies */ @@ -88,7 +104,7 @@ test.describe( 'Suggestion mode', () => { await expect( paragraph ).toHaveClass( /is-suggestion-pending/ ); } ); - test( 'shows appended text wrapped in after the block deselects', async ( { + test( 'add — golden path: shows appended text wrapped in after the block deselects', async ( { editor, page, } ) => { @@ -131,7 +147,7 @@ test.describe( 'Suggestion mode', () => { await expect( paragraph ).toContainText( 'Hello' ); } ); - test( 'shows deleted text wrapped in after the block deselects', async ( { + test( 'delete — golden path: shows deleted text wrapped in after the block deselects', async ( { editor, page, pageUtils, @@ -170,7 +186,7 @@ test.describe( 'Suggestion mode', () => { ).toBeVisible(); } ); - test( 'shows a newly bolded word wrapped in with preserved', async ( { + test( 'style — golden path: shows a newly bolded word wrapped in with preserved', async ( { editor, page, pageUtils, From 905841f7cb5f7cf7bb17c13f6a88d67a6beb3f15 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 6 May 2026 10:49:30 -0700 Subject: [PATCH 06/19] Suggestions: Move inline-diff styles into the block-editor content bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .has-suggestion-deletion / .has-suggestion-addition rules target RichText runs that render inside the editor canvas iframe. Previously they lived in the editor-package stylesheet (wp-edit-blocks), which is not loaded into the iframe — only wp-block-editor-content (compiled from block-editor/src/content.scss) is. As a result the inline marks landed on the DOM but no rule matched, so deletions/additions rendered unstyled. Move the inline-format rules to a new partial under block-editor/src/components/block-list/content-suggestion.scss and include it from block-editor/src/content.scss. The class names describe a text visual state, so block-editor styling them does not violate package layering: the partial only knows how to render the marks, not how the format arrived. Sidebar/diff/header rules stay in the editor package since they target chrome surrounding the iframe rather than RichText runs within it. Add `--suggestion-author-color` as an opt-in CSS custom property on the inline rules with the prior red/green pair as the fallback, so the follow-up commit that pipes the suggester's avatar color through `markContentDiff` can light it up without re-touching this file. --- .../block-list/content-suggestion.scss | 37 +++++++++++++++++++ packages/block-editor/src/content.scss | 1 + .../src/components/suggestion-mode/style.scss | 22 ----------- 3 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 packages/block-editor/src/components/block-list/content-suggestion.scss diff --git a/packages/block-editor/src/components/block-list/content-suggestion.scss b/packages/block-editor/src/components/block-list/content-suggestion.scss new file mode 100644 index 00000000000000..eb1cd24d1e7423 --- /dev/null +++ b/packages/block-editor/src/components/block-list/content-suggestion.scss @@ -0,0 +1,37 @@ +@use "@wordpress/base-styles/colors" as *; +@use "@wordpress/base-styles/variables" as *; + +// Visual treatment for RichText runs marked as a pending suggestion. The +// `.has-suggestion-deletion` / `.has-suggestion-addition` classes are +// applied programmatically by the editor-package overlay HOC; this +// stylesheet only knows about the resulting class names. Lives in +// `block-editor` because the canvas iframe loads `wp-block-editor-content` +// (compiled from this package) but does NOT load the editor-package +// chrome stylesheets — keeping these rules here is what makes them apply +// inside the iframe. + +// Default suggestion colors, used when the suggester's avatar color +// can't be resolved (e.g., anonymous edits or pre-collab sessions). +// `markContentDiff` writes the suggester's avatar color into +// `--suggestion-author-color` on each / run, so individual +// suggestions tint to their author's color the same way live cursors do +// — Google Docs-style. Rules below consume the variable with the +// red/green pair as the fallback. +$suggestion-deletion-color: #cc1818; +$suggestion-addition-color: #007017; +$suggestion-author-color: var(--suggestion-author-color, #{$suggestion-addition-color}); + +.has-suggestion-deletion { + color: var(--suggestion-author-color, #{$suggestion-deletion-color}); + text-decoration: line-through; + text-decoration-color: var(--suggestion-author-color, #{$suggestion-deletion-color}); +} + +.has-suggestion-addition { + color: $suggestion-author-color; + text-decoration: underline; + text-decoration-color: $suggestion-author-color; + box-shadow: + -1px 0 0 0 $suggestion-author-color, + 1px 0 0 0 $suggestion-author-color; +} diff --git a/packages/block-editor/src/content.scss b/packages/block-editor/src/content.scss index 4f70f2df4271a6..913477aaa45a6b 100644 --- a/packages/block-editor/src/content.scss +++ b/packages/block-editor/src/content.scss @@ -2,6 +2,7 @@ @use "@wordpress/base-styles/mixins" as *; @use "./components/block-icon/content.scss" as *; @use "./components/block-list/content.scss" as *; +@use "./components/block-list/content-suggestion.scss" as *; @use "./components/block-list-appender/content.scss" as *; @use "./components/block-content-overlay/content.scss" as *; @use "./components/block-draggable/content.scss" as *; diff --git a/packages/editor/src/components/suggestion-mode/style.scss b/packages/editor/src/components/suggestion-mode/style.scss index 35ba3d2db508b3..198ea4b0941a25 100644 --- a/packages/editor/src/components/suggestion-mode/style.scss +++ b/packages/editor/src/components/suggestion-mode/style.scss @@ -60,28 +60,6 @@ $suggestion-color: #007017; } } -// Inline diff formats applied to RichText runs while a suggestion is -// pending. The marked value lives in the suggest-mode overlay and is fed -// to the block's RichText components, so the marks render in the editor -// canvas itself — not just the sidebar summary. The persisted post is -// untouched (the store interceptor reverts the live block while the -// suggestion is open; Accept strips the formats before the value lands). -.has-suggestion-deletion { - color: var(--wp-block-synced-color, #cc1818); - text-decoration: line-through; - text-decoration-color: var(--wp-block-synced-color, #cc1818); -} - -.has-suggestion-addition { - color: var(--wp-admin-theme-color, $suggestion-color); - background: rgba(0, 112, 23, 0.08); - text-decoration: underline; - text-decoration-color: var(--wp-admin-theme-color, $suggestion-color); - box-shadow: - -1px 0 0 var(--wp-admin-theme-color, $suggestion-color), - 1px 0 0 var(--wp-admin-theme-color, $suggestion-color); -} - // Inline accept/reject buttons live in the note header, right-aligned next // to the author info. Force both icons to render at the same box so the // checkmark (a thin glyph) visually matches the close glyph next to it, From ef72cfdd94ed6cf0ef9b980714bc0cfc087e3773 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 6 May 2026 10:53:36 -0700 Subject: [PATCH 07/19] Suggestions: Carry suggester avatar color on inline diff marks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardcoded red / theme-green make every suggester's marks look the same, so reviewers can't tell two authors apart in the canvas. Pipe an optional `authorColor` through `markContentDiff` and emit it as an inline `style="--suggestion-author-color: …"` on each / run. The block-editor CSS partial already consumes the variable with the previous red/green pair as the fallback, so anonymous / pre-collab edits stay visually unchanged — single-suggester sessions see no diff on the wire. The author color rides on the wrapper element rather than on a parent (there is no block-level wrapper available — marks live inside RichText). `stripSuggestionMarks` already removes the wrapper on accept / round-trip, so the inline `style` attribute disappears with it; no schema impact. Add unit coverage for the new parameter (color propagation, null/ undefined fallthrough, and the strip round-trip) so future changes to the wrapper format don't silently drop the per-author tinting. --- .../suggestion-mode/inline-formats.js | 57 +++++++++++++++---- .../suggestion-mode/test/inline-formats.js | 40 +++++++++++++ 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js index 81db1f45d2c032..6a6ab01f777974 100644 --- a/packages/editor/src/components/suggestion-mode/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -71,16 +71,37 @@ registerSuggestionFormats(); const DELETION_CLASS = 'has-suggestion-deletion'; const ADDITION_CLASS = 'has-suggestion-addition'; +/** + * Build the optional `style` attribute that paints the suggester's avatar + * color into the inline mark. Returns an empty string when no color is + * passed so anonymous / pre-collab edits keep the existing red/green CSS + * fallbacks. Inline rather than a class because each suggester needs a + * distinct value, and there's no block-level wrapper to hang a custom + * property on — marks live inside RichText. + * + * @param {string|null|undefined} color Hex color from `getAvatarBorderColor`. + * @return {string} Style attribute fragment, leading-space included, or ''. + */ +function authorStyleAttr( color ) { + if ( ! color ) { + return ''; + } + return ` style="--suggestion-author-color: ${ color }"`; +} + /** * Wrap a chunk of HTML in the suggested-deletion ``. Used when an * incoming run was present in the baseline but absent from the proposed * value, so reviewers see what the suggester wants to remove. * - * @param {string} value Run content (already-serialized HTML or plain text). + * @param {string} value Run content (already-serialized HTML or plain text). + * @param {string|null|undefined} color Optional suggester avatar color. * @return {string} Wrapped run. */ -function wrapDeletion( value ) { - return `${ value }`; +function wrapDeletion( value, color ) { + return `${ value }`; } /** @@ -88,11 +109,14 @@ function wrapDeletion( value ) { * incoming run is present in the proposed value but absent from the * baseline, so reviewers see what the suggester wants to add. * - * @param {string} value Run content (already-serialized HTML or plain text). + * @param {string} value Run content (already-serialized HTML or plain text). + * @param {string|null|undefined} color Optional suggester avatar color. * @return {string} Wrapped run. */ -function wrapAddition( value ) { - return `${ value }`; +function wrapAddition( value, color ) { + return `${ value }`; } /** @@ -117,11 +141,22 @@ function wrapAddition( value ) { * (e.g. baseline === proposed during an attribute-only suggestion) costs * nothing. * - * @param {string|null|undefined} baseline Original attribute value. - * @param {string|null|undefined} proposed Suggested attribute value. + * `authorColor` is an optional hex string (`#188038`, etc.) that the HOC + * resolves from the suggester's user id via `getAvatarBorderColor`. When + * provided it rides along on each ``/`` as an inline + * `style="--suggestion-author-color: …"` so reviewers can tell two + * suggesters' marks apart at a glance — Google Docs-style. The CSS + * partial in `block-editor` consumes the variable with the existing + * red/green pair as the fallback, so omitting `authorColor` leaves + * single-suggester sessions visually unchanged. + * + * @param {string|null|undefined} baseline Original attribute value. + * @param {string|null|undefined} proposed Suggested attribute value. + * @param {string|null} [authorColor] Optional suggester avatar color + * applied per ``/`` run. * @return {string} Marked HTML representing the diff. */ -export function markContentDiff( baseline, proposed ) { +export function markContentDiff( baseline, proposed, authorColor = null ) { const beforeHtml = baseline === null || baseline === undefined ? '' : String( baseline ); const afterHtml = @@ -137,9 +172,9 @@ export function markContentDiff( baseline, proposed ) { if ( seg.type === 'equal' ) { result += seg.value; } else if ( seg.type === 'insert' ) { - result += wrapAddition( seg.value ); + result += wrapAddition( seg.value, authorColor ); } else { - result += wrapDeletion( seg.value ); + result += wrapDeletion( seg.value, authorColor ); } } return result; diff --git a/packages/editor/src/components/suggestion-mode/test/inline-formats.js b/packages/editor/src/components/suggestion-mode/test/inline-formats.js index d91eee07bbe523..f67dc01a27dad3 100644 --- a/packages/editor/src/components/suggestion-mode/test/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/test/inline-formats.js @@ -162,6 +162,34 @@ describe( 'markContentDiff', () => { 'Old' ); } ); + + it( 'paints the suggester avatar color onto each del/ins run', () => { + // `authorColor` rides along as an inline custom property on every + // mark so two suggesters' marks read as different colors. The CSS + // partial in block-editor consumes the variable; the helper just + // has to write it onto the tag. + expect( + markContentDiff( 'Hello world', 'Hello there', '#b26200' ) + ).toBe( + 'Hello ' + + 'world' + + 'there' + ); + } ); + + it( 'omits the inline style when authorColor is null or undefined', () => { + // Anonymous / pre-collab edits keep the existing red/green CSS + // fallback, so the marks on the wire stay byte-identical to the + // pre-author-color shape. + expect( markContentDiff( 'Hello', 'Hi', null ) ).toBe( + 'Hello' + + 'Hi' + ); + expect( markContentDiff( 'Hello', 'Hi', undefined ) ).toBe( + 'Hello' + + 'Hi' + ); + } ); } ); describe( 'stripSuggestionMarks', () => { @@ -210,4 +238,16 @@ describe( 'stripSuggestionMarks', () => { expect( stripSuggestionMarks( null ) ).toBeNull(); expect( stripSuggestionMarks( undefined ) ).toBeUndefined(); } ); + + it( 'discards the per-author inline style attribute on round-trip', () => { + // The author color rides on the wrapper element, so removing + // (deletion) or unwrapping (addition) the wrapper drops the inline + // `style` attribute with it — proposed value never carries leftover + // `--suggestion-author-color` on persistence. + const marked = + 'Hello ' + + 'world' + + 'there'; + expect( stripSuggestionMarks( marked ) ).toBe( 'Hello there' ); + } ); } ); From b67ed11a501b2f488b46f45e8f83bc94c2bfb1d5 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 6 May 2026 10:56:35 -0700 Subject: [PATCH 08/19] Suggestions: Tint inline diff marks with the suggester's avatar color MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the current user id from `coreStore.getCurrentUser` and map it through `getAvatarBorderColor` (the same palette `editor-collab-sidebar` uses for live cursors). Forward the resulting hex through `applyDiffMarks` into `markContentDiff` so each / emitted in the canvas carries `style="--suggestion-author-color: …"`. The block-editor CSS partial already consumes the variable with the red/green pair as the fallback, so anonymous edits keep the existing default. Folded into the existing `useSelect` so the HOC stays at one extra store-subscription per block in suggest mode. `authorColor` flows through `useMemo`'s dep list so the marked HTML re-renders the moment a suggester logs in (or out) without forcing the whole tree. Add a unit test for the propagation through `applyDiffMarks` so the contract is enforced at the helper boundary, independent of how the HOC resolves the color. --- .../test/with-suggestion-overlay.js | 16 +++++ .../with-suggestion-overlay.js | 63 +++++++++++++------ 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index 7f8c15fd3435c8..f9596e73f3d66a 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -331,6 +331,22 @@ describe( 'applyDiffMarks', () => { const baseline = { align: 'left' }; expect( applyDiffMarks( merged, baseline ) ).toBe( merged ); } ); + + it( 'propagates the suggester avatar color into each marked run', () => { + // HOC resolves the suggester via `getAvatarBorderColor` and passes + // the hex color through. The marks must carry it inline so two + // suggesters' edits read as different colors in the canvas. + const result = applyDiffMarks( + { content: 'Hello world' }, + { content: 'Hello' }, + '#b26200' + ); + expect( result.content ).toBe( + 'Hello' + + ' ' + + 'world' + ); + } ); } ); describe( 'stripMarksFromIncoming', () => { diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index d7b3e674ef942c..635e440f980515 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -34,6 +34,7 @@ import { createHigherOrderComponent } from '@wordpress/compose'; import { useSelect } from '@wordpress/data'; import { useCallback, useMemo, useRef } from '@wordpress/element'; import { addFilter } from '@wordpress/hooks'; +import { store as coreStore } from '@wordpress/core-data'; /** * Internal dependencies @@ -41,6 +42,7 @@ import { addFilter } from '@wordpress/hooks'; import { useSuggestionOverlay } from './overlay-context'; import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; import { markContentDiff, stripSuggestionMarks } from './inline-formats'; +import { getAvatarBorderColor } from '../collab-sidebar/utils'; const BLOCK_EDITOR_STORE_NAME = 'core/block-editor'; @@ -82,13 +84,19 @@ function isStringLike( value ) { * the baseline (no change to mark) or whose baseline is missing (the * suggestion has nothing to diff against). * - * @param {Object} merged Output of `mergeOverlayAttributes`. - * @param {Object} baseline Baseline attributes captured when the suggestion - * began. + * `authorColor` is forwarded to `markContentDiff` so each ``/`` + * carries the suggester's avatar color as an inline custom property; the + * canvas CSS partial consumes the variable and falls back to the + * red/green default when this is null. + * + * @param {Object} merged Output of `mergeOverlayAttributes`. + * @param {Object} baseline Baseline attributes captured when the + * suggestion began. + * @param {string|null} [authorColor] Optional suggester avatar color. * @return {Object} `merged` with rich-text attributes replaced by marked * HTML, or `merged` unchanged when nothing was eligible. */ -function applyDiffMarks( merged, baseline ) { +function applyDiffMarks( merged, baseline, authorColor = null ) { if ( ! merged || ! baseline ) { return merged; } @@ -110,7 +118,11 @@ function applyDiffMarks( merged, baseline ) { if ( ! result ) { result = { ...merged }; } - result[ key ] = markContentDiff( originalStr, proposedStr ); + result[ key ] = markContentDiff( + originalStr, + proposedStr, + authorColor + ); } return result ?? merged; } @@ -223,19 +235,28 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const overlayAttributes = overlayEntry?.overlayAttributes ?? null; const baselineAttributes = overlayEntry?.baselineAttributes ?? null; - // Whether this block is the currently selected one. While selected, we - // skip applying inline diff marks so RichText's value-prop reconciliation - // doesn't fight the user's caret on every keystroke. Marks reappear as - // soon as the user clicks/tabs away. Defaults to `true` so any - // environment without the block-editor store registered (unit tests of - // this HOC) skips marking too — production always has the store. - const isSelected = useSelect( + // Whether this block is the currently selected one (skip marking while + // the user is typing into it) and the suggester's avatar color (paints + // the inline marks so two suggesters' edits read as different colors). + // Folded into a single `useSelect` so the HOC stays at one extra + // store-subscription per block in suggest mode. `isSelected` defaults + // to `true` so any environment without the block-editor store + // registered (unit tests of this HOC) skips marking too — production + // always has both stores. `authorColor` defaults to `null` for + // anonymous / pre-collab edits, in which case the canvas CSS falls + // through to the red/green pair. + const { isSelected, authorColor } = useSelect( ( select ) => { const blockEditor = select( BLOCK_EDITOR_STORE_NAME ); - if ( ! blockEditor?.isBlockSelected ) { - return true; - } - return blockEditor.isBlockSelected( clientId ); + const core = select( coreStore ); + const userId = core?.getCurrentUser?.()?.id ?? null; + return { + isSelected: blockEditor?.isBlockSelected + ? blockEditor.isBlockSelected( clientId ) + : true, + authorColor: + userId !== null ? getAvatarBorderColor( userId ) : null, + }; }, [ clientId ] ); @@ -277,8 +298,14 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { if ( isSelected ) { return merged; } - return applyDiffMarks( merged, baselineAttributes ); - }, [ attributes, overlayAttributes, baselineAttributes, isSelected ] ); + return applyDiffMarks( merged, baselineAttributes, authorColor ); + }, [ + attributes, + overlayAttributes, + baselineAttributes, + isSelected, + authorColor, + ] ); return ( Date: Tue, 5 May 2026 21:54:01 -0600 Subject: [PATCH 09/19] RTC: Fix compaction unit test (#77986) Co-authored-by: alecgeatches Co-authored-by: maxschmeling Co-authored-by: ramonjd --- .../collaboration/wpHttpPollingSyncServer.php | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/phpunit/tests/collaboration/wpHttpPollingSyncServer.php b/phpunit/tests/collaboration/wpHttpPollingSyncServer.php index 31726d688232a2..7e599ed15d636a 100644 --- a/phpunit/tests/collaboration/wpHttpPollingSyncServer.php +++ b/phpunit/tests/collaboration/wpHttpPollingSyncServer.php @@ -915,7 +915,7 @@ public function test_sync_should_compact_is_false_for_non_compactor() { $this->assertFalse( $data['rooms'][0]['should_compact'] ); } - public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists() { + public function test_sync_stale_compaction_is_stored_as_update_when_newer_compaction_exists() { wp_set_current_user( self::$editor_id ); $room = $this->get_post_room(); @@ -945,9 +945,12 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists ) ); - // Client 3 sends a stale compaction at cursor 0. The server should find - // client 2's compaction in the updates after cursor 0 and silently discard - // this one. + // Client 3 sends a stale compaction at cursor 0 (mirroring two offline + // clients that reconnect from the same baseline cursor). The server + // cannot run remove_updates_before_cursor because client 2 has already + // advanced the frontier, but the bytes must still be stored as a + // regular update so client 3's operations can propagate to other + // clients via Yjs state-as-update merging. $stale_compaction = array( 'type' => 'compaction', 'data' => 'c3RhbGU=', @@ -960,16 +963,29 @@ public function test_sync_stale_compaction_succeeds_when_newer_compaction_exists $this->assertSame( 200, $response->get_status() ); - // Verify the newer compaction is preserved and the stale one was not stored. - $response = $this->dispatch_sync( + // Verify the newer compaction is preserved AND the stale compaction's + // bytes were persisted (now as type=update so subsequent compactions + // don't trip the has_newer_compaction check). + $response = $this->dispatch_sync( array( $this->build_room( $room, 4, 0, array( 'user' => 'c4' ) ), ) ); - $update_data = wp_list_pluck( $response->get_data()['rooms'][0]['updates'], 'data' ); + $updates = $response->get_data()['rooms'][0]['updates']; + $update_data = wp_list_pluck( $updates, 'data' ); $this->assertContains( 'Y29tcGFjdGVk', $update_data, 'The newer compaction should be preserved.' ); - $this->assertNotContains( 'c3RhbGU=', $update_data, 'The stale compaction should not be stored.' ); + $this->assertContains( 'c3RhbGU=', $update_data, 'The stale compaction bytes should be stored so client 3\'s operations propagate.' ); + + $stale_entry = null; + foreach ( $updates as $entry ) { + if ( 'c3RhbGU=' === $entry['data'] ) { + $stale_entry = $entry; + break; + } + } + $this->assertNotNull( $stale_entry, 'The stale compaction entry should be present in the room.' ); + $this->assertSame( 'update', $stale_entry['type'], 'The stale compaction should be stored as type=update, not type=compaction.' ); } /* From 5343fb1303f60a8486ae09b7e0467f64ffd3d7f1 Mon Sep 17 00:00:00 2001 From: Alec Geatches Date: Tue, 5 May 2026 16:11:54 -0600 Subject: [PATCH 10/19] RTC: Fix divergence when two offline users reconnect (#77980) * Keep overlapping compaction updates to avoid YDoc divergence * Add backport file --------- Co-authored-by: alecgeatches Co-authored-by: maxschmeling --- backport-changelog/7.0/11716.md | 3 +++ .../class-wp-http-polling-sync-server.php | 11 ++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 backport-changelog/7.0/11716.md diff --git a/backport-changelog/7.0/11716.md b/backport-changelog/7.0/11716.md new file mode 100644 index 00000000000000..2d5be7c7f232e6 --- /dev/null +++ b/backport-changelog/7.0/11716.md @@ -0,0 +1,3 @@ +https://github.com/WordPress/wordpress-develop/pull/11716 + +* https://github.com/WordPress/gutenberg/pull/77980 diff --git a/lib/compat/wordpress-7.0/class-wp-http-polling-sync-server.php b/lib/compat/wordpress-7.0/class-wp-http-polling-sync-server.php index ac3680ebb32f24..2d1a530ca7d085 100644 --- a/lib/compat/wordpress-7.0/class-wp-http-polling-sync-server.php +++ b/lib/compat/wordpress-7.0/class-wp-http-polling-sync-server.php @@ -500,9 +500,14 @@ private function process_sync_update( string $room, int $client_id, int $cursor, return $this->add_update( $room, $client_id, $type, $data ); } - // Reaching this point means there's a newer compaction, so we can - // silently ignore this one. - return true; + /* + * A newer compaction already advanced the cursor, but we + * can not safely drop an update. The incoming bytes still encode + * operations other clients may not have seen, so store them as a + * regular update. Y.applyUpdateV2 merges state-as-update blobs + * idempotently, so overlap with the existing compaction is safe. + */ + return $this->add_update( $room, $client_id, self::UPDATE_TYPE_UPDATE, $data ); case self::UPDATE_TYPE_SYNC_STEP1: case self::UPDATE_TYPE_SYNC_STEP2: From 6a423cee3d971cd340c51670de394dbb39e56793 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 11:25:32 -0700 Subject: [PATCH 11/19] Suggestions: Drop redundant bare import of inline-formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The named re-export of `SUGGESTED_DELETION_FORMAT`, `SUGGESTED_ADDITION_FORMAT`, and `registerSuggestionFormats` already pulls the module in, evaluating its top-level format registration. The companion `import './inline-formats';` above it added nothing — and because the file is not listed in the editor package's `sideEffects` allowlist, esbuild emits an `[ignored-bare-import]` warning every dev build. Removing the bare import keeps the registration behavior unchanged (the named re-export still triggers it, as does `with-suggestion-overlay.js` consuming `markContentDiff`/`stripSuggestionMarks`) and clears the warning. --- packages/editor/src/components/suggestion-mode/index.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index 961f13a76f3197..dd05b4ae644ebc 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -1,9 +1,3 @@ -// Side-effect import: registers the inline RichText format types -// (`gutenberg/suggested-deletion`, `gutenberg/suggested-addition`) consumed -// by `with-suggestion-overlay.js` to render proposed adds and deletes inside -// the editor canvas. See #77867. -import './inline-formats'; - export { SUGGESTED_DELETION_FORMAT, SUGGESTED_ADDITION_FORMAT, From ed7110cd6abf30b1ddfa5133b92358d1c9fedd3c Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 11:46:51 -0700 Subject: [PATCH 12/19] Suggestions: Coalesce consecutive diff segments into one wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wordDiff` tokenizes on word/whitespace boundaries, so a multi-word addition or deletion arrives as 2N-1 separate `insert`/`delete` segments. The previous `markContentDiff` wrapped each segment in its own ``/``, which combined with the per-element border CSS painted a pipe around every word — the result looked like a bracket grid rather than a single underlined addition. Accumulate consecutive same-type segments into one run and emit a single wrapper per run. Boundary CSS now only paints at the actual edges of each change. Updates two unit tests in `test/inline-formats.js` and two in `test/with-suggestion-overlay.js` that pinned the pre-coalesce per-segment shape. --- .../suggestion-mode/inline-formats.js | 31 ++++++++++++++++--- .../suggestion-mode/test/inline-formats.js | 19 ++++++------ .../test/with-suggestion-overlay.js | 7 ++--- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js index 6a6ab01f777974..a514e955d73652 100644 --- a/packages/editor/src/components/suggestion-mode/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -166,17 +166,38 @@ export function markContentDiff( baseline, proposed, authorColor = null ) { return afterHtml; } + // `wordDiff` tokenizes into alternating word/whitespace runs, so a + // contiguous addition like "foo bar baz" arrives as five separate + // `insert` segments. Wrapping each one in its own `` paints the + // per-segment `box-shadow` boundary CSS as a pipe around every word. + // Coalesce consecutive same-type segments so a single addition (or + // deletion) renders as one `` (or ``) with brackets only at + // the real boundaries of the change. const segments = wordDiff( beforeHtml, afterHtml ); let result = ''; + let runType = null; + let runValue = ''; + const flush = () => { + if ( runType === 'insert' ) { + result += wrapAddition( runValue, authorColor ); + } else if ( runType === 'delete' ) { + result += wrapDeletion( runValue, authorColor ); + } else if ( runType === 'equal' ) { + result += runValue; + } + runType = null; + runValue = ''; + }; for ( const seg of segments ) { - if ( seg.type === 'equal' ) { - result += seg.value; - } else if ( seg.type === 'insert' ) { - result += wrapAddition( seg.value, authorColor ); + if ( seg.type === runType ) { + runValue += seg.value; } else { - result += wrapDeletion( seg.value, authorColor ); + flush(); + runType = seg.type; + runValue = seg.value; } } + flush(); return result; } diff --git a/packages/editor/src/components/suggestion-mode/test/inline-formats.js b/packages/editor/src/components/suggestion-mode/test/inline-formats.js index f67dc01a27dad3..9257727de6f953 100644 --- a/packages/editor/src/components/suggestion-mode/test/inline-formats.js +++ b/packages/editor/src/components/suggestion-mode/test/inline-formats.js @@ -107,23 +107,22 @@ describe( 'markContentDiff', () => { } ); it( 'wraps appended text in ', () => { - // Pure end-of-string addition: "Hello" → "Hello world". The added - // space and word each surface as their own insert segments because - // `wordDiff` tokenizes on whitespace runs. + // Pure end-of-string addition: "Hello" → "Hello world". `wordDiff` + // emits separate insert segments per word/whitespace run, and + // `markContentDiff` coalesces consecutive same-type segments so the + // whole appended run lands inside one `` — otherwise the + // per-element boundary CSS draws a pipe around every word. expect( markContentDiff( 'Hello', 'Hello world' ) ).toBe( - 'Hello' + - ' ' + - 'world' + 'Hello' + ' world' ); } ); it( 'wraps removed text in ', () => { // Pure end-of-string deletion: "Hello world" → "Hello". Mirrors the - // addition case but in the delete direction. + // addition case but in the delete direction; the run is coalesced + // into a single ``. expect( markContentDiff( 'Hello world', 'Hello' ) ).toBe( - 'Hello' + - ' ' + - 'world' + 'Hello' + ' world' ); } ); diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index f9596e73f3d66a..3b38ffb75152d0 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -315,9 +315,7 @@ describe( 'applyDiffMarks', () => { { content: 'Hello', level: 2 } ); expect( result.content ).toBe( - 'Hello' + - ' ' + - 'world' + 'Hello' + ' world' ); // Other attributes pass through untouched. expect( result.level ).toBe( 2 ); @@ -343,8 +341,7 @@ describe( 'applyDiffMarks', () => { ); expect( result.content ).toBe( 'Hello' + - ' ' + - 'world' + ' world' ); } ); } ); From 2ce68371e5b79bf414ff75edaf1ef8d2e17e24ec Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 11:54:01 -0700 Subject: [PATCH 13/19] Suggestions: Show inline diff marks once edits settle, not only on blur MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay HOC suppresses diff marks while the block is selected to keep RichText's value-prop reconciliation from disrupting the caret during continuous typing. But discrete edits — a range Delete, a paste, a single Backspace — also stayed unmarked until focus moved off the block, leaving suggesters with no immediate visual feedback that the suggestion had been captured. Replace the block-selected gate with a ~100 ms idle debounce on `overlayAttributes` change. A single overlay write followed by silence flips the gate within a frame, so marks render almost immediately for discrete actions while continuous typing keeps the timer reset and avoids the original caret-fight scenario. Implemented as `setIsOverlayIdle(false)` during render keyed on a ref change, so the new `false` value is consumed by the same render that observed the overlay change — no one-frame flash of marked output between the change and the timeout cleanup. Adds two unit tests: marks render after the idle window, and stay suppressed while overlay writes churn inside it. --- .../test/with-suggestion-overlay.js | 91 +++++++++++++++++++ .../with-suggestion-overlay.js | 48 ++++++++-- 2 files changed, 133 insertions(+), 6 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index 3b38ffb75152d0..6059d9f1cbacaf 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -176,6 +176,97 @@ describe( 'withSuggestionOverlay', () => { } ); } ); + it( 'surfaces diff marks once the overlay change settles', () => { + // A discrete edit (e.g. range Delete) generates one overlay write + // then silence — after the ~100 ms idle window, the HOC swaps the + // clean proposed value for the marked HTML so reviewers see the + // strikethrough/insertion without leaving the block. + jest.useFakeTimers(); + try { + const setAttributes = jest.fn(); + renderWithProviders( + , + { intent: 'suggest' } + ); + + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + + // Immediately after the click, the gate still suppresses marks. + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + 'proposed' + ); + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + / { + jest.advanceTimersByTime( 100 ); + } ); + + // After the idle window, the rendered content carries both the + // deletion (`Hello`) and the addition (`proposed`) wrappers. + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /Hello<\/del>/ + ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /proposed<\/ins>/ + ); + } finally { + jest.useRealTimers(); + } + } ); + + it( 'keeps marks suppressed while overlay writes churn within the idle window', () => { + // Continuous typing flushes one setAttributes per keystroke. Each + // write resets the debounce timer, so marks stay hidden until the + // user pauses — RichText doesn't see a marked value mid-burst. + // Each click triggers a fresh setAttributes call, which the + // reducer turns into a new `overlayAttributes` reference even when + // the proposed value is unchanged, so two clicks model a typing + // burst that re-arms the debounce. + jest.useFakeTimers(); + try { + const setAttributes = jest.fn(); + renderWithProviders( + , + { intent: 'suggest' } + ); + + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + act( () => { + jest.advanceTimersByTime( 80 ); + } ); + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + act( () => { + jest.advanceTimersByTime( 80 ); + } ); + + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + / { + jest.advanceTimersByTime( 100 ); + } ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /Hello<\/del>/ + ); + } finally { + jest.useRealTimers(); + } + } ); + it( 're-captures baseline when overlay is cleared then re-edited', () => { // Regression: after Submit/Discard clears the overlay entry, a // later edit must create a new baseline + overlay rather than diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index 635e440f980515..e426e23444e3e3 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -32,7 +32,13 @@ import clsx from 'clsx'; */ import { createHigherOrderComponent } from '@wordpress/compose'; import { useSelect } from '@wordpress/data'; -import { useCallback, useMemo, useRef } from '@wordpress/element'; +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from '@wordpress/element'; import { addFilter } from '@wordpress/hooks'; import { store as coreStore } from '@wordpress/core-data'; @@ -289,13 +295,42 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { [ clientId, name, captureBaseline, setOverlayAttributes, entryExists ] ); + // Idle-debounce mark rendering: when `overlayAttributes` is unchanged + // across a ~100 ms window, treat the user as no longer typing and + // surface the marks. A discrete edit (range Delete, single Backspace, + // paste) generates one overlay write followed by silence, so marks + // appear within ~100 ms — imperceptibly close to "immediately" without + // fighting RichText's value-prop reconciliation mid-burst. Continuous + // typing re-arms the timer on every keystroke, so marks stay hidden + // until the user pauses. + const [ isOverlayIdle, setIsOverlayIdle ] = useState( false ); + const lastOverlayRef = useRef( overlayAttributes ); + if ( lastOverlayRef.current !== overlayAttributes ) { + // Reset the idle flag synchronously during render. This is React's + // "storing information from previous renders" pattern — calling + // `setIsOverlayIdle` here causes React to retry the render with the + // new state value before committing, so the current render computes + // the gate against the fresh `false` without a one-frame flash of + // marked output between an overlay change and the effect below. + lastOverlayRef.current = overlayAttributes; + setIsOverlayIdle( false ); + } + useEffect( () => { + if ( ! overlayAttributes ) { + return; + } + const handle = window.setTimeout( () => setIsOverlayIdle( true ), 100 ); + return () => window.clearTimeout( handle ); + }, [ overlayAttributes ] ); + const mergedAttributes = useMemo( () => { const merged = mergeOverlayAttributes( attributes, overlayAttributes ); - // While the block is selected, hand back the plain proposed value so - // the user's caret doesn't fight RichText's value-prop reconciliation - // on every keystroke. The marks reappear as soon as focus moves away - // — see the `isSelected` `useSelect` above for the full rationale. - if ( isSelected ) { + // Skip marking only while the block is selected AND the user is + // actively writing into the overlay (`! isOverlayIdle`). The block + // being merely selected isn't enough — a user who has stopped typing + // expects to see their change reflected. Once focus moves away, the + // `isSelected` check goes false and marks render immediately. + if ( isSelected && ! isOverlayIdle ) { return merged; } return applyDiffMarks( merged, baselineAttributes, authorColor ); @@ -304,6 +339,7 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { overlayAttributes, baselineAttributes, isSelected, + isOverlayIdle, authorColor, ] ); From 5e0a9f6e85cb53f680adc39b78ab5e12c374453b Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 12:03:05 -0700 Subject: [PATCH 14/19] Suggestions: Hydrate overlay from persisted suggestion comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline diff overlay lives in React state on `SuggestionOverlayProvider`, seeded only by direct edits in Suggest intent. That left two gaps: suggesters lost their pending marks on reload (auto-save persisted the comment, but the overlay starts empty on mount), and reviewers viewing the same post in any non-Suggest intent never saw the inline strike-through/insertion at all — only the sidebar summary. Add a `SuggestionOverlayHydrator` mounted alongside the existing suggest-mode siblings (interceptor, autosave) inside the provider. It re-uses the `useNoteThreads` query from the collab sidebar so no extra REST traffic is needed, parses each unresolved comment's `_wp_suggestion` payload via the existing `parseSuggestionPayload`, turns each `attribute-set` op into a baseline/overlay pair, and dispatches a new `SEED_FROM_COMMENT` reducer action. The seeded entry is tagged with `hydratedFromCommentId` so the HOC can tell a hydrator-sourced entry from a live edit. Two behaviors follow from that: - The outer `withSuggestionOverlay` wrap now activates whenever `isSuggestMode || hasOverlayEntry`, so blocks with a hydrated entry render with marks even outside Suggest intent. - Inside `SuggestingBlockEdit`, `wrappedSetAttributes` falls through to the real `setAttributes` when not in Suggest intent. Reviewers' keystrokes therefore land on the real block instead of being trapped in the suggester's overlay. The hydrator skips entries that already have live overlay writes (`Object.keys(overlayAttributes).length > 0` and not flagged as hydrator-sourced) so a refresh of the comment list mid-typing doesn't clobber the suggester's unsaved state. Adds three unit tests covering the reducer, the syncedOpsKey preservation, and the reviewer write-through scenario. --- .../editor/src/components/provider/index.js | 2 + .../components/suggestion-mode/hydrator.js | 134 ++++++++++++++++++ .../src/components/suggestion-mode/index.js | 1 + .../suggestion-mode/overlay-context.js | 43 ++++++ .../suggestion-mode/test/overlay-context.js | 53 +++++++ .../test/with-suggestion-overlay.js | 77 ++++++++++ .../with-suggestion-overlay.js | 71 ++++++++-- 7 files changed, 366 insertions(+), 15 deletions(-) create mode 100644 packages/editor/src/components/suggestion-mode/hydrator.js diff --git a/packages/editor/src/components/provider/index.js b/packages/editor/src/components/provider/index.js index 74eb706f840408..648c4c853a89d9 100644 --- a/packages/editor/src/components/provider/index.js +++ b/packages/editor/src/components/provider/index.js @@ -50,6 +50,7 @@ import { SuggestionOverlayProvider, SuggestionAutoSave, SuggestionStoreInterceptor, + SuggestionOverlayHydrator, registerSuggestionOverlayFilter, } from '../suggestion-mode'; @@ -480,6 +481,7 @@ export const ExperimentalEditorProvider = withRegistryProvider( + { window?.__experimentalMediaEditorModal && ( ) } diff --git a/packages/editor/src/components/suggestion-mode/hydrator.js b/packages/editor/src/components/suggestion-mode/hydrator.js new file mode 100644 index 00000000000000..13c5446b5d361f --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/hydrator.js @@ -0,0 +1,134 @@ +/** + * Seeds the suggestion overlay from persisted `_wp_suggestion` comment + * payloads on editor mount and whenever the comment list updates. Two + * scenarios depend on this: + * + * - A suggester reloads the page after auto-save fired. The in-memory + * overlay (see `overlay-context.js`) starts empty, but the persisted + * note carries the proposed values — without re-seeding, the inline + * diff marks disappear until the suggestion is accepted or rejected. + * - A reviewer (post author, admin) opens the same post in any intent. + * They never wrote the suggestion themselves, so their local overlay is + * empty too; without seeding, they only see the sidebar summary, never + * the inline strike-through/insertion on the canvas. + * + * The hydrator reuses the existing `useNoteThreads` hook from the collab + * sidebar so the entity-records query (and its cache) is shared — no extra + * REST traffic, no risk of the two consumers diverging on what counts as a + * note thread. + * + * Live editing wins over hydration: if an entry already exists for a block + * and was *not* sourced from the hydrator, the seed is skipped. That way a + * suggester typing into a block whose previous suggestion was persisted + * doesn't have their unsaved overlay clobbered by a refresh of the comment + * list. + */ + +/** + * WordPress dependencies + */ +import { useEffect } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { useSuggestionOverlay } from './overlay-context'; +import { parseSuggestionPayload } from './provider'; +import { useNoteThreads } from '../collab-sidebar/hooks'; +import { store as editorStore } from '../../store'; + +/** + * Derive baseline + overlay attribute pairs from a parsed payload's + * `attribute-set` operations. Structural ops (`block-remove`, + * `block-insert-after`, `block-move`) are already rendered via the block's + * own `metadata.suggestion` marker on the live canvas, so the hydrator + * skips them — including them here would re-do work that the structural + * BlockListBlock filter already handles. + * + * @param {{ operations: Array<{type: string, attribute?: string, before?: *, after?: *}> }|null} payload + * @return {{ baselineAttributes: Object, overlayAttributes: Object }|null} Pair + * suitable for `SEED_FROM_COMMENT`, or null when the payload carries no + * attribute-set ops. + */ +function attributePairsFromPayload( payload ) { + if ( ! payload || ! Array.isArray( payload.operations ) ) { + return null; + } + const baselineAttributes = {}; + const overlayAttributes = {}; + let count = 0; + for ( const op of payload.operations ) { + if ( + op?.type !== 'attribute-set' || + typeof op.attribute !== 'string' + ) { + continue; + } + baselineAttributes[ op.attribute ] = op.before; + overlayAttributes[ op.attribute ] = op.after; + count++; + } + if ( count === 0 ) { + return null; + } + return { baselineAttributes, overlayAttributes }; +} + +/** + * Mounted once inside `SuggestionOverlayProvider`. Watches the post's note + * threads and seeds an overlay entry for any unresolved (`status: 'hold'`) + * suggestion whose payload carries an `attribute-set` operation. Re-runs + * whenever the thread list or block tree changes so a newly-loaded + * suggestion (or a hot-reloaded block tree) ends up reflected. + */ +export default function SuggestionOverlayHydrator() { + const postId = useSelect( + ( select ) => select( editorStore ).getCurrentPostId(), + [] + ); + const { unresolvedNotes } = useNoteThreads( postId ); + const { entries, seedFromComment } = useSuggestionOverlay(); + + useEffect( () => { + if ( ! unresolvedNotes || unresolvedNotes.length === 0 ) { + return; + } + for ( const thread of unresolvedNotes ) { + const clientId = thread.blockClientId; + if ( ! clientId ) { + continue; + } + const payload = parseSuggestionPayload( + thread.meta?._wp_suggestion + ); + if ( ! payload ) { + continue; + } + const pairs = attributePairsFromPayload( payload ); + if ( ! pairs ) { + continue; + } + const existing = entries[ clientId ]; + // Don't clobber a live overlay that wasn't itself sourced from + // the hydrator — the suggester may be mid-edit and the in-memory + // state is more current than the persisted comment. + if ( + existing && + existing.hydratedFromCommentId !== thread.id && + Object.keys( existing.overlayAttributes ?? {} ).length > 0 + ) { + continue; + } + seedFromComment( + clientId, + payload.blockName ?? null, + thread.id, + pairs.baselineAttributes, + pairs.overlayAttributes + ); + } + }, [ unresolvedNotes, entries, seedFromComment ] ); + + return null; +} diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index dd05b4ae644ebc..8139d30262b4ba 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -14,6 +14,7 @@ export { } from './with-suggestion-overlay'; export { default as SuggestionAutoSave } from './auto-save'; export { default as SuggestionStoreInterceptor } from './store-interceptor'; +export { default as SuggestionOverlayHydrator } from './hydrator'; export { useSuggestionsProvider, operationsFromOverlay, diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.js index 43d560fa671236..08290af719f95c 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/overlay-context.js @@ -83,6 +83,7 @@ const OverlayContext = createContext( { clearOverlay: () => {}, setCommentId: () => {}, setSyncedOpsKey: () => {}, + seedFromComment: () => {}, hasOverlay: () => false, requestInterceptorBypass: () => {}, consumeInterceptorBypass: () => false, @@ -161,6 +162,27 @@ export function overlayReducer( state, action ) { }, }; } + case 'SEED_FROM_COMMENT': { + // Replace an entry verbatim with the values derived from a + // persisted suggestion comment. `hydratedFromCommentId` flags the + // entry so consumers can tell hydrator-sourced state from live + // editing (the HOC routes writes through to the real block when + // the only reason an entry exists is hydration). `syncedOpsKey` + // is preserved from any prior entry so a fresh seed doesn't + // re-trigger auto-save on top of an already-persisted payload. + const existing = state[ action.clientId ]; + return { + ...state, + [ action.clientId ]: { + blockName: action.blockName, + baselineAttributes: action.baselineAttributes, + overlayAttributes: action.overlayAttributes, + commentId: action.commentId, + syncedOpsKey: existing?.syncedOpsKey ?? null, + hydratedFromCommentId: action.commentId, + }, + }; + } case 'PRUNE_ORPHANS': { // Action carries a serializable array; the reducer materializes a // Set internally for the lookup. Keeps actions Redux-DevTools- @@ -236,6 +258,25 @@ export function SuggestionOverlayProvider( { children } ) { [] ); + const seedFromComment = useCallback( + ( + clientId, + blockName, + commentId, + baselineAttributes, + overlayAttributes + ) => + dispatch( { + type: 'SEED_FROM_COMMENT', + clientId, + blockName, + commentId, + baselineAttributes, + overlayAttributes, + } ), + [] + ); + const hasEntries = Object.keys( entries ).length > 0; const hasOverlay = useCallback( @@ -315,6 +356,7 @@ export function SuggestionOverlayProvider( { children } ) { clearOverlay, setCommentId, setSyncedOpsKey, + seedFromComment, hasOverlay, requestInterceptorBypass, consumeInterceptorBypass, @@ -326,6 +368,7 @@ export function SuggestionOverlayProvider( { children } ) { clearOverlay, setCommentId, setSyncedOpsKey, + seedFromComment, hasOverlay, requestInterceptorBypass, consumeInterceptorBypass, diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.js b/packages/editor/src/components/suggestion-mode/test/overlay-context.js index d0a49e8a697dd4..b991b1040c8524 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.js @@ -215,4 +215,57 @@ describe( 'overlayReducer', () => { expect( afterClear[ 'block-a' ] ).toBeUndefined(); expect( afterClear[ 'block-b' ] ).toEqual( state[ 'block-b' ] ); } ); + + it( 'seeds a full entry from a persisted suggestion comment', () => { + // The hydrator path bypasses CAPTURE_BASELINE / SET_OVERLAY_ATTRIBUTES + // in favor of one atomic write so a hydrated entry is identifiable + // (via `hydratedFromCommentId`) and never collides with the existing + // "CAPTURE_BASELINE is a no-op when an entry exists" rule. + const seeded = overlayReducer( INITIAL, { + type: 'SEED_FROM_COMMENT', + clientId: 'block-a', + blockName: 'core/paragraph', + commentId: 42, + baselineAttributes: { content: 'before' }, + overlayAttributes: { content: 'after' }, + } ); + + expect( seeded[ 'block-a' ] ).toEqual( { + blockName: 'core/paragraph', + baselineAttributes: { content: 'before' }, + overlayAttributes: { content: 'after' }, + commentId: 42, + syncedOpsKey: null, + hydratedFromCommentId: 42, + } ); + } ); + + it( 'preserves an existing syncedOpsKey when re-seeding the same entry', () => { + // After auto-save sets the sync fingerprint, a re-hydration on the + // next mount must not zero it out — otherwise auto-save would re-fire + // for an already-persisted payload on every reload. + const withSynced = { + 'block-a': { + blockName: 'core/paragraph', + baselineAttributes: { content: 'before' }, + overlayAttributes: { content: 'after' }, + commentId: 42, + syncedOpsKey: 'op-hash', + hydratedFromCommentId: 42, + }, + }; + const reseeded = overlayReducer( withSynced, { + type: 'SEED_FROM_COMMENT', + clientId: 'block-a', + blockName: 'core/paragraph', + commentId: 42, + baselineAttributes: { content: 'before' }, + overlayAttributes: { content: 'after-2' }, + } ); + + expect( reseeded[ 'block-a' ].syncedOpsKey ).toBe( 'op-hash' ); + expect( reseeded[ 'block-a' ].overlayAttributes ).toEqual( { + content: 'after-2', + } ); + } ); } ); diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index 6059d9f1cbacaf..b9144a89df197f 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -267,6 +267,83 @@ describe( 'withSuggestionOverlay', () => { } } ); + it( 'wraps for marks but writes through to the real block when a hydrated entry exists outside Suggest intent', () => { + // Reviewer scenario: the hydrator seeded an overlay entry from a + // persisted suggestion. The block must render the marked diff + // (via the wrapping HOC) but any keystrokes the reviewer types into + // the canvas must land on the real block, not get captured into + // the suggester's overlay. + jest.useFakeTimers(); + try { + // Trigger the hydration via a button the test can click, so the + // `seedFromComment` callback is only consumed inside React's + // event handler rather than reassigned from inside a render. + function SeedButton() { + const { seedFromComment } = useSuggestionOverlay(); + return ( + + ); + } + + const setAttributes = jest.fn(); + renderWithProviders( + <> + + + , + { intent: 'edit' } + ); + + // Initially there's no entry, so the HOC is a pass-through in + // Edit intent — no marks, real setAttributes wired up. + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + /<(del|ins)/ + ); + + // Hydrate from a persisted comment. + fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); + act( () => { + jest.advanceTimersByTime( 100 ); + } ); + + // Now the wrap is active and the marks render even outside + // Suggest intent. + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /before<\/del>/ + ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /after<\/ins>/ + ); + + // Reviewer types — write goes through to the real block, not + // the overlay. + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + expect( setAttributes ).toHaveBeenCalledWith( { + content: 'proposed', + } ); + } finally { + jest.useRealTimers(); + } + } ); + it( 're-captures baseline when overlay is cleared then re-edited', () => { // Regression: after Submit/Discard clears the overlay entry, a // later edit must create a new baseline + overlay rather than diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index e426e23444e3e3..3f1a6bb1d4f5d9 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -214,20 +214,30 @@ function mergeOverlayAttributes( base, overlay ) { } /** - * Inner renderer that owns the suggestion overlay hooks. Only mounted when - * the editor is in `suggest` intent, so the overlay's context lookup, - * refs, and memoized merge don't run on every `BlockEdit` render for every - * block across the entire editor when suggestions are inactive. This split - * matters for large documents — in Edit/View intent the outer wrapper - * executes a single `useSelect` and renders the original `BlockEdit` - * untouched. + * Inner renderer that owns the suggestion overlay hooks. Mounted when the + * editor is in `suggest` intent or when the hydrator has seeded a read-only + * overlay entry for this block (reviewer / post-reload). Splitting the + * outer pass-through HOC from this inner component keeps the overlay's + * context lookup, refs, and memoized merge out of the render path on + * blocks the suggestion system has nothing to say about — a noticeable win + * on large documents. * - * @param {Object} args Arguments. - * @param {import('react').ComponentType} args.BlockEdit Wrapped edit component. - * @param {Object} args.props Props to forward to `BlockEdit`. + * @param {Object} args Arguments. + * @param {import('react').ComponentType} args.BlockEdit Wrapped edit component. + * @param {Object} args.props Props to forward to `BlockEdit`. + * @param {boolean} args.isSuggestMode True when the editor is in Suggest + * intent; routes writes into the + * overlay. Otherwise the wrap is + * render-only and writes go straight + * to the real block. */ -function SuggestingBlockEdit( { BlockEdit, props } ) { - const { clientId, name, attributes } = props; +function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { + const { + clientId, + name, + attributes, + setAttributes: realSetAttributes, + } = props; const { entries, captureBaseline, setOverlayAttributes } = useSuggestionOverlay(); @@ -276,6 +286,15 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const wrappedSetAttributes = useCallback( ( nextAttributes ) => { + // Reviewer / non-suggest intent (the block is only being wrapped + // because the hydrator seeded an entry from a persisted + // suggestion comment): writes go straight to the real block. + // The reviewer's edit is not part of the original suggester's + // proposal, so it shouldn't be captured into the overlay. + if ( ! isSuggestMode ) { + realSetAttributes( nextAttributes ); + return; + } // First overlay write for this block snapshots the current // attributes as the baseline; subsequent writes only record // overlay deltas. This lets the diff renderer below compare @@ -292,7 +311,15 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { stripMarksFromIncoming( nextAttributes ) ); }, - [ clientId, name, captureBaseline, setOverlayAttributes, entryExists ] + [ + clientId, + name, + captureBaseline, + setOverlayAttributes, + entryExists, + isSuggestMode, + realSetAttributes, + ] ); // Idle-debounce mark rendering: when `overlayAttributes` is unchanged @@ -365,19 +392,33 @@ function SuggestingBlockEdit( { BlockEdit, props } ) { const withSuggestionOverlay = createHigherOrderComponent( ( BlockEdit ) => function BlockEditWithSuggestionOverlay( props ) { + const { clientId } = props; const isSuggestMode = useSelect( ( select ) => select( EDITOR_STORE_NAME ).getEditorIntent() === SUGGEST_INTENT, [] ); + // Wrap blocks that have an overlay entry too, not just those + // being edited in Suggest intent. The hydrator seeds entries + // from persisted suggestion comments so a reviewer (or the + // suggester after a reload) can see inline diff marks for any + // pending suggestion without having to re-enter Suggest intent. + // `wrappedSetAttributes` routes the reviewer's writes through + // to the real block, so this wrap is render-only for them. + const { entries } = useSuggestionOverlay(); + const hasOverlayEntry = !! entries[ clientId ]; - if ( ! isSuggestMode ) { + if ( ! isSuggestMode && ! hasOverlayEntry ) { return ; } return ( - + ); }, 'withSuggestionOverlay' From d76aa7f098a528a87c47e939ea22f2c484b6193b Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 14:44:49 -0700 Subject: [PATCH 15/19] Suggestions: Skip hydrator re-seed when persisted payload is unchanged The hydrator re-runs whenever the comment entity record list updates. Without a value-level guard, re-seeding the same payload produced a new overlay state reference on every render and the effect looped, pegging the renderer. Short-circuit when both baseline and overlay attribute shallow-compare equal to the existing entry's, and add unit tests covering the seed paths. --- .../components/suggestion-mode/hydrator.js | 52 +++ .../suggestion-mode/test/hydrator.js | 312 ++++++++++++++++++ 2 files changed, 364 insertions(+) create mode 100644 packages/editor/src/components/suggestion-mode/test/hydrator.js diff --git a/packages/editor/src/components/suggestion-mode/hydrator.js b/packages/editor/src/components/suggestion-mode/hydrator.js index 13c5446b5d361f..ac290b0d491566 100644 --- a/packages/editor/src/components/suggestion-mode/hydrator.js +++ b/packages/editor/src/components/suggestion-mode/hydrator.js @@ -38,6 +38,40 @@ import { parseSuggestionPayload } from './provider'; import { useNoteThreads } from '../collab-sidebar/hooks'; import { store as editorStore } from '../../store'; +/** + * Shallow equality over two flat objects of primitive-ish values. The + * hydrator uses this to avoid re-dispatching `SEED_FROM_COMMENT` on every + * render when the persisted payload hasn't actually moved — the reducer + * would otherwise mint a new state reference each time and the effect would + * loop. Strings, numbers, booleans, null, and undefined compare by value; + * non-primitive values (object attribute payloads like `style`) fall back to + * reference equality. That matches the suggestion payload shape today — + * `attribute-set` ops carry primitive or string-serialized values. + * + * @param {Object} a First object. + * @param {Object} b Second object. + * @return {boolean} True when both objects have the same keys and matching values. + */ +function shallowEqual( a, b ) { + if ( a === b ) { + return true; + } + if ( ! a || ! b ) { + return false; + } + const ak = Object.keys( a ); + const bk = Object.keys( b ); + if ( ak.length !== bk.length ) { + return false; + } + for ( const k of ak ) { + if ( a[ k ] !== b[ k ] ) { + return false; + } + } + return true; +} + /** * Derive baseline + overlay attribute pairs from a parsed payload's * `attribute-set` operations. Structural ops (`block-remove`, @@ -120,6 +154,24 @@ export default function SuggestionOverlayHydrator() { ) { continue; } + // Already hydrated from this comment and the persisted values + // haven't changed — no-op rather than re-dispatching. The reducer + // would otherwise produce a new state reference on every render, + // looping the effect indefinitely. + if ( + existing && + existing.hydratedFromCommentId === thread.id && + shallowEqual( + existing.baselineAttributes, + pairs.baselineAttributes + ) && + shallowEqual( + existing.overlayAttributes, + pairs.overlayAttributes + ) + ) { + continue; + } seedFromComment( clientId, payload.blockName ?? null, diff --git a/packages/editor/src/components/suggestion-mode/test/hydrator.js b/packages/editor/src/components/suggestion-mode/test/hydrator.js new file mode 100644 index 00000000000000..622775e56fc7e7 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/hydrator.js @@ -0,0 +1,312 @@ +/** + * Tests for `hydrator.js`. The component watches the post's note threads + * and seeds the suggestion overlay from each unresolved comment that + * carries an `attribute-set` payload. Coverage focuses on the dispatch + * decisions; the rendering of the marks themselves is exercised in + * `test/with-suggestion-overlay.js`. + * + * `useNoteThreads` is mocked so we can drive the thread list directly + * without standing up the entity-records query that the real hook uses. + */ + +/** + * External dependencies + */ +import { render, act } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { createRegistry, RegistryProvider } from '@wordpress/data'; +import { store as noticesStore } from '@wordpress/notices'; + +/** + * Internal dependencies + */ +import SuggestionOverlayHydrator from '../hydrator'; +import { + SuggestionOverlayProvider, + useSuggestionOverlay, +} from '../overlay-context'; +import { store as editorStore } from '../../../store'; + +jest.mock( '../../collab-sidebar/hooks', () => ( { + useNoteThreads: jest.fn(), +} ) ); + +// eslint-disable-next-line import/order +const { useNoteThreads } = require( '../../collab-sidebar/hooks' ); + +function makePayload( operations, blockName = 'core/paragraph' ) { + return JSON.stringify( { + schemaVersion: 2, + blockName, + baseRevision: null, + operations, + } ); +} + +// Renders the hydrator inside a provider plus a probe that captures +// the live entries map so the test can assert against it. +function renderWithProbe( { intent = 'edit' } = {} ) { + const registry = createRegistry(); + registry.register( editorStore ); + registry.dispatch( editorStore ).setEditedPost( 'post', 42 ); + registry.dispatch( editorStore ).setEditorIntent( intent ); + + let latestEntries = null; + function Probe() { + const { entries } = useSuggestionOverlay(); + latestEntries = entries; + return null; + } + + const utils = render( + + + + + + + ); + + return { + ...utils, + getEntries: () => latestEntries, + }; +} + +describe( 'SuggestionOverlayHydrator', () => { + beforeEach( () => { + useNoteThreads.mockReset(); + } ); + + it( 'seeds an entry from an unresolved attribute-set payload', () => { + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 101, + blockClientId: 'block-a', + status: 'hold', + meta: { + _wp_suggestion: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + before: 'Hello', + after: 'Hello world', + }, + ] ), + }, + }, + ], + } ); + + const { getEntries } = renderWithProbe(); + const entries = getEntries(); + expect( entries[ 'block-a' ] ).toEqual( { + blockName: 'core/paragraph', + baselineAttributes: { content: 'Hello' }, + overlayAttributes: { content: 'Hello world' }, + commentId: 101, + syncedOpsKey: null, + hydratedFromCommentId: 101, + } ); + } ); + + it( 'aggregates multiple attribute-set ops on the same block', () => { + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 102, + blockClientId: 'block-a', + status: 'hold', + meta: { + _wp_suggestion: makePayload( + [ + { + type: 'attribute-set', + attribute: 'content', + before: 'before', + after: 'after', + }, + { + type: 'attribute-set', + attribute: 'level', + before: 2, + after: 3, + }, + ], + 'core/heading' + ), + }, + }, + ], + } ); + + const { getEntries } = renderWithProbe(); + expect( getEntries()[ 'block-a' ] ).toMatchObject( { + blockName: 'core/heading', + baselineAttributes: { content: 'before', level: 2 }, + overlayAttributes: { content: 'after', level: 3 }, + } ); + } ); + + it( 'skips threads with no resolvable blockClientId', () => { + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 103, + blockClientId: null, + status: 'hold', + meta: { + _wp_suggestion: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ] ), + }, + }, + ], + } ); + + const { getEntries } = renderWithProbe(); + expect( getEntries() ).toEqual( {} ); + } ); + + it( 'skips threads whose payload fails to parse', () => { + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 104, + blockClientId: 'block-a', + status: 'hold', + meta: { _wp_suggestion: 'not json' }, + }, + ], + } ); + + const { getEntries } = renderWithProbe(); + expect( getEntries() ).toEqual( {} ); + } ); + + it( 'skips structural-only payloads — those are rendered via metadata.suggestion on the block, not via the overlay', () => { + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 105, + blockClientId: 'block-a', + status: 'hold', + meta: { + _wp_suggestion: makePayload( [ + { + type: 'block-remove', + clientId: 'block-a', + blockName: 'core/paragraph', + }, + ] ), + }, + }, + ], + } ); + + const { getEntries } = renderWithProbe(); + expect( getEntries() ).toEqual( {} ); + } ); + + it( 'does not clobber a live overlay write that was not hydrator-sourced', () => { + // The hydrator must refuse to overwrite an entry that was put in + // place by a live edit, because the suggester may be mid-edit and + // their unsaved overlay is more current than the persisted comment. + // Test in two steps so the live state is settled in the reducer + // before the hydrator mounts and reads `entries` from its render + // closure — otherwise the unit-test render would batch both + // dispatches into the same commit and the hydrator's first effect + // would see an empty entries map. + useNoteThreads.mockReturnValue( { + notes: [], + unresolvedNotes: [ + { + id: 106, + blockClientId: 'block-a', + status: 'hold', + meta: { + _wp_suggestion: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + before: 'persisted before', + after: 'persisted after', + }, + ] ), + }, + }, + ], + } ); + + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( editorStore ); + registry.dispatch( editorStore ).setEditedPost( 'post', 42 ); + + let overlayApi = null; + let entries = null; + function Probe() { + overlayApi = useSuggestionOverlay(); + entries = overlayApi.entries; + return null; + } + + // Step 1: mount only the provider + probe. Capture the overlay API + // so we can dispatch a live write directly. + const { rerender } = render( + + + + + + ); + + act( () => { + overlayApi.captureBaseline( 'block-a', 'core/paragraph', { + content: 'live baseline', + } ); + overlayApi.setOverlayAttributes( 'block-a', { + content: 'live in-progress', + } ); + } ); + + // Step 2: re-render with the hydrator mounted. Its first effect + // will see the live entry and skip the seed. + rerender( + + + + + + + ); + + // Live overlay wins; no hydratedFromCommentId stamp. + expect( entries[ 'block-a' ].overlayAttributes ).toEqual( { + content: 'live in-progress', + } ); + expect( entries[ 'block-a' ].hydratedFromCommentId ).toBeUndefined(); + } ); + + it( 'is a no-op when there are no unresolved threads', () => { + // The empty-list branch must not dispatch — orphan-prune / re-render + // cycles would otherwise churn the reducer unnecessarily. + useNoteThreads.mockReturnValue( { notes: [], unresolvedNotes: [] } ); + const { getEntries } = renderWithProbe(); + expect( getEntries() ).toEqual( {} ); + } ); +} ); From 8baa32d143efd1ba581d151ad91616ff74bc5f53 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 12 May 2026 14:47:04 -0700 Subject: [PATCH 16/19] Suggestions: Don't mask reviewer writes when overlay is hydrated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a real-time collaboration session, the hydrator seeds an overlay entry on every connected client from each persisted suggestion comment. Reviewers route their writes through to the real block, but the merge step still let the suggester's recorded `after` win on overlapping rich-text keys — so the reviewer's typing was masked in the canvas and the diff marks made it look as if their edits had become part of the suggester's proposal. Skip the merge for hydrated reviewer entries once the real content diverges from the suggester's recorded baseline; the existing accept-time conflict prompt covers the case where someone tries to accept a now-stale suggestion. --- .../test/with-suggestion-overlay.js | 108 +++++++++++++++++- .../with-suggestion-overlay.js | 67 +++++++++++ 2 files changed, 174 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index b9144a89df197f..5d4fc794556c81 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -299,13 +299,18 @@ describe( 'withSuggestionOverlay', () => { } const setAttributes = jest.fn(); + // Real block content stays at the suggester's recorded baseline + // (`before`) — that's the production state: until the suggestion + // is accepted, the live block-editor store still holds the + // baseline value, and only the overlay carries the proposed + // `after`. renderWithProviders( <> , @@ -344,6 +349,107 @@ describe( 'withSuggestionOverlay', () => { } } ); + it( 'drops marks for a hydrated entry when real content has diverged from the suggester baseline', () => { + // RTC regression: once a reviewer's real block content moves away + // from the suggester's recorded `before` (their own keystrokes, or + // a concurrent editor's CRDT-synced change), the overlay merge would + // otherwise overwrite the reviewer's text with the suggester's stale + // `after` and the diff marks would visually attribute the reviewer's + // edits to the suggester. The guard skips the merge in that case. + jest.useFakeTimers(); + try { + function SeedButton() { + const { seedFromComment } = useSuggestionOverlay(); + return ( + + ); + } + + const setAttributes = jest.fn(); + // Real block content has already diverged from the suggester's + // recorded baseline (`before`) — e.g. the reviewer just typed. + renderWithProviders( + <> + + + , + { intent: 'edit' } + ); + + fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); + act( () => { + jest.advanceTimersByTime( 100 ); + } ); + + // Real content wins. No del/ins wrappers — the divergence + // guard skipped the overlay merge and the diff rendering. + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + 'reviewer typed' + ); + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + / { + // The divergence guard is reviewer-only — the suggester's overlay + // is the source of truth and must keep rendering with diff marks + // regardless of whether the real block content (kept at baseline by + // the overlay) matches the recorded baseline. This pins the guard + // to `! isSuggestMode` so a future refactor can't widen it. + jest.useFakeTimers(); + try { + const setAttributes = jest.fn(); + renderWithProviders( + , + { intent: 'suggest' } + ); + + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + act( () => { + jest.advanceTimersByTime( 100 ); + } ); + + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /Hello<\/del>/ + ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /proposed<\/ins>/ + ); + } finally { + jest.useRealTimers(); + } + } ); + it( 're-captures baseline when overlay is cleared then re-edited', () => { // Regression: after Submit/Discard clears the overlay entry, a // later edit must create a new baseline + overlay rather than diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index 3f1a6bb1d4f5d9..5fc4d1746377d6 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -84,6 +84,48 @@ function isStringLike( value ) { ); } +/** + * Compare the rich-text attributes of two attribute objects for stringwise + * equality. Used to detect divergence between the suggester's recorded + * baseline and the reviewer's current real-block content: when they no + * longer match, the hydrated overlay is no longer applicable and the merge + * step must be skipped. Missing values on both sides count as equal so a + * block with no `content` attribute doesn't trigger divergence on every + * render. + * + * @param {Object|null} a First attribute set. + * @param {Object|null} b Second attribute set. + * @return {boolean} True when every key in `RICH_TEXT_ATTRIBUTE_KEYS` + * stringifies to the same value in both, including the both-missing case. + */ +function richTextAttributesMatch( a, b ) { + if ( ! a || ! b ) { + return false; + } + for ( const key of RICH_TEXT_ATTRIBUTE_KEYS ) { + const aHas = Object.prototype.hasOwnProperty.call( a, key ); + const bHas = Object.prototype.hasOwnProperty.call( b, key ); + if ( ! aHas && ! bHas ) { + continue; + } + if ( aHas !== bHas ) { + return false; + } + const av = a[ key ]; + const bv = b[ key ]; + if ( av === bv ) { + continue; + } + if ( ! isStringLike( av ) || ! isStringLike( bv ) ) { + return false; + } + if ( String( av ) !== String( bv ) ) { + return false; + } + } + return true; +} + /** * Walk the merged attribute set and replace each rich-text value with its * baseline-vs-proposed marked diff. Skips attributes whose value matches @@ -250,6 +292,19 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { const overlayEntry = entries[ clientId ]; const overlayAttributes = overlayEntry?.overlayAttributes ?? null; const baselineAttributes = overlayEntry?.baselineAttributes ?? null; + // Hydrator-seeded entries on reviewers (not the suggester) need a + // divergence guard. The overlay's recorded baseline is the suggester's + // `before` value. Once the real block content moves away from that + // baseline — either because the reviewer is typing locally or because + // CRDT synced inbound changes from a concurrent editor — the merge + // step would overwrite the reviewer's text with the suggester's stale + // `after` and the diff renderer would visually attribute the divergence + // to the suggester. The reviewer would see their own writes vanish and + // the suggester's name attached to text they never wrote. Skip the + // merge in that case; the existing `hasAttributeConflict` flow in + // `provider.js` will prompt on accept. + const isHydratedReviewerView = + ! isSuggestMode && !! overlayEntry?.hydratedFromCommentId; // Whether this block is the currently selected one (skip marking while // the user is typing into it) and the suggester's avatar color (paints @@ -351,6 +406,17 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { }, [ overlayAttributes ] ); const mergedAttributes = useMemo( () => { + // Reviewer view of a hydrated entry: only merge + mark while the + // real block's rich-text content still matches the suggester's + // recorded baseline. Once they diverge, render the real content + // unchanged so the reviewer's writes (or RTC-synced changes) stay + // visible and aren't visually attributed to the suggester. + if ( + isHydratedReviewerView && + ! richTextAttributesMatch( attributes, baselineAttributes ) + ) { + return attributes; + } const merged = mergeOverlayAttributes( attributes, overlayAttributes ); // Skip marking only while the block is selected AND the user is // actively writing into the overlay (`! isOverlayIdle`). The block @@ -368,6 +434,7 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { isSelected, isOverlayIdle, authorColor, + isHydratedReviewerView, ] ); return ( From 3f6c37d0fda8c7929d6f3dade5c892f26ca05298 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 13 May 2026 09:08:12 -0700 Subject: [PATCH 17/19] Suggestions: Drop vertical pipe shadows from inline addition runs The `.has-suggestion-addition` rule applied a 1px box-shadow on the left and right of every inserted run, which rendered as visible vertical bars around suggested text. The underline plus the author color already carry the "inserted" treatment (matching the Google Docs suggesting-mode style noted in the surrounding comments), so the box-shadow was redundant and visually noisy. --- .../src/components/block-list/content-suggestion.scss | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/block-editor/src/components/block-list/content-suggestion.scss b/packages/block-editor/src/components/block-list/content-suggestion.scss index eb1cd24d1e7423..8c7fad1b8d2b34 100644 --- a/packages/block-editor/src/components/block-list/content-suggestion.scss +++ b/packages/block-editor/src/components/block-list/content-suggestion.scss @@ -31,7 +31,4 @@ $suggestion-author-color: var(--suggestion-author-color, #{$suggestion-addition- color: $suggestion-author-color; text-decoration: underline; text-decoration-color: $suggestion-author-color; - box-shadow: - -1px 0 0 0 $suggestion-author-color, - 1px 0 0 0 $suggestion-author-color; } From efe0e236bc5304c78b7f282ea0748e4fb4b88275 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 13 May 2026 09:08:25 -0700 Subject: [PATCH 18/19] Suggestions: Suppress inline diff marks while the block is selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the 100 ms idle-debounce gate with a strict `isSelected` guard so the overlay HOC never flips RichText's value prop between the clean proposed value and the marked HTML while the caret is in the block. The idle gate ('Show inline diff marks once edits settle, not only on blur') tried to surface marks within ~100 ms of a discrete edit without leaving the block, but rendering ``/`` wrappers while the caret is live disrupts the cursor on the next keystroke. A reproducible failure: typing 'page' after backspacing 'post.' yields '.egappost' in the suggestion instead of 'page' — each typed character lands at the boundary of a format element, gets stripped into the deletion or prepended to the addition, and the result is a reversed insertion mixed with surviving baseline text. Marks reappear as soon as focus moves off the block. Reviewers (who are not editing the suggester's block) continue to see marks on hydrated entries without selecting anything. Two unit tests that exercised the idle gate are replaced with one that pins the new behavior: while `isBlockSelected` is true, the rendered content stays unmarked. The hydrated-reviewer and suggester-divergence tests now register the block in the editor store so `isBlockSelected` returns false (no block selected), which is the production state for those scenarios. --- .../test/with-suggestion-overlay.js | 427 ++++++++---------- .../with-suggestion-overlay.js | 51 +-- 2 files changed, 201 insertions(+), 277 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index 5d4fc794556c81..530a6492afc364 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -176,95 +176,40 @@ describe( 'withSuggestionOverlay', () => { } ); } ); - it( 'surfaces diff marks once the overlay change settles', () => { - // A discrete edit (e.g. range Delete) generates one overlay write - // then silence — after the ~100 ms idle window, the HOC swaps the - // clean proposed value for the marked HTML so reviewers see the - // strikethrough/insertion without leaving the block. - jest.useFakeTimers(); - try { - const setAttributes = jest.fn(); - renderWithProviders( - , - { intent: 'suggest' } - ); - - fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - - // Immediately after the click, the gate still suppresses marks. - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - 'proposed' - ); - expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( - / { - jest.advanceTimersByTime( 100 ); - } ); - - // After the idle window, the rendered content carries both the - // deletion (`Hello`) and the addition (`proposed`) wrappers. - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /Hello<\/del>/ - ); - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /proposed<\/ins>/ - ); - } finally { - jest.useRealTimers(); - } - } ); - - it( 'keeps marks suppressed while overlay writes churn within the idle window', () => { - // Continuous typing flushes one setAttributes per keystroke. Each - // write resets the debounce timer, so marks stay hidden until the - // user pauses — RichText doesn't see a marked value mid-burst. - // Each click triggers a fresh setAttributes call, which the - // reducer turns into a new `overlayAttributes` reference even when - // the proposed value is unchanged, so two clicks model a typing - // burst that re-arms the debounce. - jest.useFakeTimers(); - try { - const setAttributes = jest.fn(); - renderWithProviders( - , - { intent: 'suggest' } - ); + it( 'suppresses diff marks while the block is selected so RichText reconciliation does not fight the caret', () => { + // Regression: a prior implementation flipped the value prop from + // the clean overlay value to the marked HTML 100 ms after the + // suggester paused typing. Because the marked render contains + // ``/`` wrappers, the suggester's caret could land + // inside or at the boundary of a format element; the next + // keystroke would then be stripped (lost into a ``) or + // prepended to the existing addition (producing reversed + // insertions like "egap" instead of "page"). Keeping the gate + // strictly on `isSelected` means RichText never sees the marked + // value while the caret is live, so editing stays well-formed. + const setAttributes = jest.fn(); + renderWithProviders( + , + { intent: 'suggest' } + ); - fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - act( () => { - jest.advanceTimersByTime( 80 ); - } ); - fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - act( () => { - jest.advanceTimersByTime( 80 ); - } ); - - expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( - / { - jest.advanceTimersByTime( 100 ); - } ); - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /Hello<\/del>/ - ); - } finally { - jest.useRealTimers(); - } + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + 'proposed' + ); + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + /<(del|ins)/ + ); } ); it( 'wraps for marks but writes through to the real block when a hydrated entry exists outside Suggest intent', () => { @@ -273,80 +218,84 @@ describe( 'withSuggestionOverlay', () => { // (via the wrapping HOC) but any keystrokes the reviewer types into // the canvas must land on the real block, not get captured into // the suggester's overlay. - jest.useFakeTimers(); - try { - // Trigger the hydration via a button the test can click, so the - // `seedFromComment` callback is only consumed inside React's - // event handler rather than reassigned from inside a render. - function SeedButton() { - const { seedFromComment } = useSuggestionOverlay(); - return ( - - ); - } - - const setAttributes = jest.fn(); - // Real block content stays at the suggester's recorded baseline - // (`before`) — that's the production state: until the suggestion - // is accepted, the live block-editor store still holds the - // baseline value, and only the overlay carries the proposed - // `after`. - renderWithProviders( - <> - - - , - { intent: 'edit' } + // + // `blocks` is passed so that the block-editor store is registered + // and `isBlockSelected('a')` returns false — the reviewer is just + // looking at the block, not editing it — which is what frees the + // HOC to render marks. + function SeedButton() { + const { seedFromComment } = useSuggestionOverlay(); + return ( + ); + } - // Initially there's no entry, so the HOC is a pass-through in - // Edit intent — no marks, real setAttributes wired up. - expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( - /<(del|ins)/ - ); + const setAttributes = jest.fn(); + // Real block content stays at the suggester's recorded baseline + // (`before`) — that's the production state: until the suggestion + // is accepted, the live block-editor store still holds the + // baseline value, and only the overlay carries the proposed + // `after`. + renderWithProviders( + <> + + + , + { + intent: 'edit', + blocks: [ + { + clientId: 'a', + name: 'core/paragraph', + attributes: { content: 'before' }, + innerBlocks: [], + }, + ], + } + ); - // Hydrate from a persisted comment. - fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); - act( () => { - jest.advanceTimersByTime( 100 ); - } ); + // Initially there's no entry, so the HOC is a pass-through in + // Edit intent — no marks, real setAttributes wired up. + expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( + /<(del|ins)/ + ); - // Now the wrap is active and the marks render even outside - // Suggest intent. - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /before<\/del>/ - ); - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /after<\/ins>/ - ); + // Hydrate from a persisted comment. + fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); - // Reviewer types — write goes through to the real block, not - // the overlay. - fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - expect( setAttributes ).toHaveBeenCalledWith( { - content: 'proposed', - } ); - } finally { - jest.useRealTimers(); - } + // Now the wrap is active and the marks render even outside + // Suggest intent. + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /before<\/del>/ + ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /after<\/ins>/ + ); + + // Reviewer types — write goes through to the real block, not + // the overlay. + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + expect( setAttributes ).toHaveBeenCalledWith( { + content: 'proposed', + } ); } ); it( 'drops marks for a hydrated entry when real content has diverged from the suggester baseline', () => { @@ -356,63 +305,65 @@ describe( 'withSuggestionOverlay', () => { // otherwise overwrite the reviewer's text with the suggester's stale // `after` and the diff marks would visually attribute the reviewer's // edits to the suggester. The guard skips the merge in that case. - jest.useFakeTimers(); - try { - function SeedButton() { - const { seedFromComment } = useSuggestionOverlay(); - return ( - - ); - } - - const setAttributes = jest.fn(); - // Real block content has already diverged from the suggester's - // recorded baseline (`before`) — e.g. the reviewer just typed. - renderWithProviders( - <> - - - , - { intent: 'edit' } + function SeedButton() { + const { seedFromComment } = useSuggestionOverlay(); + return ( + ); + } + + const setAttributes = jest.fn(); + // Real block content has already diverged from the suggester's + // recorded baseline (`before`) — e.g. the reviewer just typed. + renderWithProviders( + <> + + + , + { + intent: 'edit', + blocks: [ + { + clientId: 'a', + name: 'core/paragraph', + attributes: { content: 'reviewer typed' }, + innerBlocks: [], + }, + ], + } + ); - fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); - act( () => { - jest.advanceTimersByTime( 100 ); - } ); + fireEvent.click( screen.getByRole( 'button', { name: 'seed' } ) ); - // Real content wins. No del/ins wrappers — the divergence - // guard skipped the overlay merge and the diff rendering. - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - 'reviewer typed' - ); - expect( screen.getByTestId( 'content' ) ).not.toHaveTextContent( - / { @@ -421,33 +372,39 @@ describe( 'withSuggestionOverlay', () => { // regardless of whether the real block content (kept at baseline by // the overlay) matches the recorded baseline. This pins the guard // to `! isSuggestMode` so a future refactor can't widen it. - jest.useFakeTimers(); - try { - const setAttributes = jest.fn(); - renderWithProviders( - , - { intent: 'suggest' } - ); + // + // Registering the block in the editor store makes + // `isBlockSelected('a')` return false (no block selected), so the + // HOC's selection gate doesn't suppress marks here. + const setAttributes = jest.fn(); + renderWithProviders( + , + { + intent: 'suggest', + blocks: [ + { + clientId: 'a', + name: 'core/paragraph', + attributes: { content: 'Hello' }, + innerBlocks: [], + }, + ], + } + ); - fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - act( () => { - jest.advanceTimersByTime( 100 ); - } ); + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /Hello<\/del>/ - ); - expect( screen.getByTestId( 'content' ) ).toHaveTextContent( - /proposed<\/ins>/ - ); - } finally { - jest.useRealTimers(); - } + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /Hello<\/del>/ + ); + expect( screen.getByTestId( 'content' ) ).toHaveTextContent( + /proposed<\/ins>/ + ); } ); it( 're-captures baseline when overlay is cleared then re-edited', () => { diff --git a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js index 5fc4d1746377d6..238575734d8691 100644 --- a/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/with-suggestion-overlay.js @@ -32,13 +32,7 @@ import clsx from 'clsx'; */ import { createHigherOrderComponent } from '@wordpress/compose'; import { useSelect } from '@wordpress/data'; -import { - useCallback, - useEffect, - useMemo, - useRef, - useState, -} from '@wordpress/element'; +import { useCallback, useMemo, useRef } from '@wordpress/element'; import { addFilter } from '@wordpress/hooks'; import { store as coreStore } from '@wordpress/core-data'; @@ -377,34 +371,6 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { ] ); - // Idle-debounce mark rendering: when `overlayAttributes` is unchanged - // across a ~100 ms window, treat the user as no longer typing and - // surface the marks. A discrete edit (range Delete, single Backspace, - // paste) generates one overlay write followed by silence, so marks - // appear within ~100 ms — imperceptibly close to "immediately" without - // fighting RichText's value-prop reconciliation mid-burst. Continuous - // typing re-arms the timer on every keystroke, so marks stay hidden - // until the user pauses. - const [ isOverlayIdle, setIsOverlayIdle ] = useState( false ); - const lastOverlayRef = useRef( overlayAttributes ); - if ( lastOverlayRef.current !== overlayAttributes ) { - // Reset the idle flag synchronously during render. This is React's - // "storing information from previous renders" pattern — calling - // `setIsOverlayIdle` here causes React to retry the render with the - // new state value before committing, so the current render computes - // the gate against the fresh `false` without a one-frame flash of - // marked output between an overlay change and the effect below. - lastOverlayRef.current = overlayAttributes; - setIsOverlayIdle( false ); - } - useEffect( () => { - if ( ! overlayAttributes ) { - return; - } - const handle = window.setTimeout( () => setIsOverlayIdle( true ), 100 ); - return () => window.clearTimeout( handle ); - }, [ overlayAttributes ] ); - const mergedAttributes = useMemo( () => { // Reviewer view of a hydrated entry: only merge + mark while the // real block's rich-text content still matches the suggester's @@ -418,12 +384,14 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { return attributes; } const merged = mergeOverlayAttributes( attributes, overlayAttributes ); - // Skip marking only while the block is selected AND the user is - // actively writing into the overlay (`! isOverlayIdle`). The block - // being merely selected isn't enough — a user who has stopped typing - // expects to see their change reflected. Once focus moves away, the - // `isSelected` check goes false and marks render immediately. - if ( isSelected && ! isOverlayIdle ) { + // While the block is selected, hand back the plain proposed value + // so the user's caret doesn't fight RichText's value-prop + // reconciliation when marks are swapped in mid-edit. Round-tripping + // marked HTML through RichText can land the caret inside a `` + // or at the start of an ``, which then turns the next + // keystroke into a stripped/reversed insertion. Marks reappear as + // soon as focus moves off the block. + if ( isSelected ) { return merged; } return applyDiffMarks( merged, baselineAttributes, authorColor ); @@ -432,7 +400,6 @@ function SuggestingBlockEdit( { BlockEdit, props, isSuggestMode } ) { overlayAttributes, baselineAttributes, isSelected, - isOverlayIdle, authorColor, isHydratedReviewerView, ] ); From 445a59fe79f981b460b6770627f91deca254cff3 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 13 May 2026 13:50:42 -0700 Subject: [PATCH 19/19] Suggestions: Let overlay tests register the block-editor store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hydrated-reviewer scenarios need `isBlockSelected` to return false (no block is selected) so the HOC's selection gate lets the diff marks render. The renderWithProviders helper previously registered only the notices and editor stores; this adds an opt-in `blocks` parameter that registers the block-editor + preferences stores and seeds the live block tree. The orphan-prune effect runs against the live tree on every overlay mutation, so we only register the block-editor store when a test asks for it — synthetic clientIds with no matching block would otherwise be pruned the moment a baseline is captured. --- .../test/with-suggestion-overlay.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js index 530a6492afc364..d31705826909af 100644 --- a/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js +++ b/packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js @@ -23,6 +23,8 @@ import { render, screen, act, fireEvent } from '@testing-library/react'; */ import { createRegistry, RegistryProvider } from '@wordpress/data'; import { store as noticesStore } from '@wordpress/notices'; +import { store as blockEditorStore } from '@wordpress/block-editor'; +import { store as preferencesStore } from '@wordpress/preferences'; /** * Internal dependencies @@ -38,13 +40,26 @@ import { } from '../overlay-context'; import { store as editorStore } from '../../../store'; -function renderWithProviders( ui, { intent = 'edit' } = {} ) { +function renderWithProviders( ui, { intent = 'edit', blocks = null } = {} ) { const registry = createRegistry(); // `setEditorIntent` dispatches a snackbar via the notices store when // the intent actually changes, so the store needs to be registered even // in tests that only care about the overlay HOC. registry.register( noticesStore ); registry.register( editorStore ); + // `blockEditorStore` is only registered when the test passes `blocks`. + // Registering it unconditionally activates the overlay provider's + // orphan-prune effect, which would clobber overlay entries whose + // `clientId` doesn't correspond to a real block — most tests use a + // synthetic `clientId="a"` and never register a matching block. + // Tests that need `isBlockSelected` to return false (e.g. the + // hydrated-reviewer scenarios) opt in by passing `blocks` with a + // matching `clientId`. + if ( blocks ) { + registry.register( preferencesStore ); + registry.register( blockEditorStore ); + registry.dispatch( blockEditorStore ).resetBlocks( blocks ); + } registry.dispatch( editorStore ).setEditorIntent( intent ); const wrapper = ( { children } ) => (