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: 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..8c7fad1b8d2b34 --- /dev/null +++ b/packages/block-editor/src/components/block-list/content-suggestion.scss @@ -0,0 +1,34 @@ +@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; +} 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/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..ac290b0d491566 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/hydrator.js @@ -0,0 +1,186 @@ +/** + * 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'; + +/** + * 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`, + * `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; + } + // 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, + 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 19eaec10652496..8139d30262b4ba 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -1,3 +1,8 @@ +export { + SUGGESTED_DELETION_FORMAT, + SUGGESTED_ADDITION_FORMAT, + registerSuggestionFormats, +} from './inline-formats'; export { SuggestionOverlayProvider, useSuggestionOverlay, @@ -9,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/inline-formats.js b/packages/editor/src/components/suggestion-mode/inline-formats.js new file mode 100644 index 00000000000000..a514e955d73652 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/inline-formats.js @@ -0,0 +1,255 @@ +/** + * WordPress dependencies + */ +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. + * + * Both formats are registered without an `edit` UI so they never appear in + * 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. + */ +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(); + +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|null|undefined} color Optional suggester avatar color. + * @return {string} Wrapped run. + */ +function wrapDeletion( value, color ) { + 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). + * @param {string|null|undefined} color Optional suggester avatar color. + * @return {string} Wrapped run. + */ +function wrapAddition( value, color ) { + 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 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 + * nothing. + * + * `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, authorColor = null ) { + const beforeHtml = + baseline === null || baseline === undefined ? '' : String( baseline ); + const afterHtml = + proposed === null || proposed === undefined ? '' : String( proposed ); + + if ( beforeHtml === afterHtml ) { + 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 === runType ) { + runValue += seg.value; + } else { + flush(); + runType = seg.type; + runValue = seg.value; + } + } + flush(); + 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/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/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( {} ); + } ); +} ); 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..9257727de6f953 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/inline-formats.js @@ -0,0 +1,252 @@ +/** + * 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, + markContentDiff, + stripSuggestionMarks, +} 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' + ); + } ); +} ); + +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". `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' + ); + } ); + + it( 'wraps removed text in ', () => { + // Pure end-of-string deletion: "Hello world" → "Hello". Mirrors the + // addition case but in the delete direction; the run is coalesced + // into a single ``. + 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' + ); + } ); + + 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', () => { + 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(); + } ); + + 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' ); + } ); +} ); 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 afacd69920caf1..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 @@ -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 */ @@ -8,12 +23,16 @@ 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 */ import withSuggestionOverlay, { mergeOverlayAttributes, + applyDiffMarks, + stripMarksFromIncoming, } from '../with-suggestion-overlay'; import { SuggestionOverlayProvider, @@ -21,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 } ) => ( @@ -159,6 +191,237 @@ describe( 'withSuggestionOverlay', () => { } ); } ); + 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' } + ); + + // `renderWithProviders` doesn't register `blockEditorStore`, so + // `isBlockSelected` falls through to the safe default of `true` + // — the same condition the HOC sees while a real user has the + // caret in the block. + fireEvent.click( screen.getByRole( 'button', { name: 'edit' } ) ); + + 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', () => { + // 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. + // + // `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 ( + + ); + } + + 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: [], + }, + ], + } + ); + + // 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' } ) ); + + // 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', () => { + // 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. + 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' } ) ); + + // 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. + // + // 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' } ) ); + + 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', () => { // Regression: after Submit/Discard clears the overlay entry, a // later edit must create a new baseline + overlay rather than @@ -273,3 +536,90 @@ 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 ); + } ); + + 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', () => { + 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..238575734d8691 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 */ @@ -10,12 +34,176 @@ 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 */ 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'; + +/** + * 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' ] ); + +/** + * 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; + } + return ( + value !== null && + value !== undefined && + typeof value.toString === 'function' && + value.toString !== Object.prototype.toString + ); +} + +/** + * 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 + * the baseline (no change to mark) or whose baseline is missing (the + * suggestion has nothing to diff against). + * + * `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, authorColor = null ) { + 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, + authorColor + ); + } + 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 @@ -25,6 +213,19 @@ import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; */ 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; @@ -49,20 +250,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(); @@ -72,29 +283,126 @@ 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; + // 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 + // 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 ); + 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 ] + ); // 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 ) => { + // 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 + // "what the block looked like when the suggestion started" + // against "what the suggester is proposing now". 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 ] + [ + clientId, + name, + captureBaseline, + setOverlayAttributes, + entryExists, + isSuggestMode, + realSetAttributes, + ] ); - const mergedAttributes = useMemo( - () => mergeOverlayAttributes( attributes, overlayAttributes ), - [ attributes, 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 ); + // 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 ); + }, [ + attributes, + overlayAttributes, + baselineAttributes, + isSelected, + authorColor, + isHydratedReviewerView, + ] ); return ( 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' @@ -198,5 +520,5 @@ export function registerSuggestionOverlayFilter() { ); } -export { mergeOverlayAttributes }; +export { mergeOverlayAttributes, applyDiffMarks, stripMarksFromIncoming }; export default withSuggestionOverlay; 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.' ); } /* diff --git a/test/e2e/specs/editor/various/suggestion-mode.spec.js b/test/e2e/specs/editor/various/suggestion-mode.spec.js index 14e0fff32ed876..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,6 +104,133 @@ test.describe( 'Suggestion mode', () => { await expect( paragraph ).toHaveClass( /is-suggestion-pending/ ); } ); + test( 'add — golden path: 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( 'delete — golden path: 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( 'style — golden path: 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,