diff --git a/packages/editor/src/components/collab-sidebar/note.js b/packages/editor/src/components/collab-sidebar/note.js index ab39c63e61aaf9..dbfc7d54fa2405 100644 --- a/packages/editor/src/components/collab-sidebar/note.js +++ b/packages/editor/src/components/collab-sidebar/note.js @@ -20,6 +20,7 @@ import { moreVertical, published } from '@wordpress/icons'; */ import { NoteCard } from './note-card'; import { NoteForm } from './note-form'; +import SuggestionActions from './suggestion-actions'; import { unlock } from '../../lock-unlock'; const { Menu } = unlock( componentsPrivateApis ); @@ -66,7 +67,11 @@ export function Note( { const [ actionState, setActionState ] = useState( null ); const actionButtonRef = useRef( null ); - const canResolve = note.parent === 0; + // Suggestion threads expose their own Accept/Reject affordance in the + // header; the generic "Resolve" button would duplicate that action with + // a confusingly similar checkmark icon, so hide it for suggestion notes. + const hasSuggestionPayload = !! note?.meta?._wp_suggestion; + const canResolve = note.parent === 0 && ! hasSuggestionPayload; const isResolutionNote = note.type === 'note' && note.meta && @@ -165,9 +170,10 @@ export function Note( { ); } - const actions = isSelected ? ( + const showActions = isSelected; + const actions = showActions ? ( <> - { canResolve && onResolve && ( + { isSelected && canResolve && onResolve && ( + + + + ) } + { showStaleDialog && ( + { + setShowStaleDialog( false ); + runApply(); + } } + onCancel={ () => setShowStaleDialog( false ) } + confirmButtonText={ __( 'Apply anyway' ) } + > + { __( + 'The post has changed since this suggestion was made. Applying it may produce unexpected results. Continue?' + ) } + + ) } + + ); +} diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index 4af71a0e614c1f..f204900159a6d9 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -12,7 +12,10 @@ export { default as SuggestionStoreInterceptor } from './store-interceptor'; export { useSuggestionsProvider, operationsFromOverlay, + applyOperations, + parseSuggestionPayload, payloadByteLength, PAYLOAD_MAX_BYTES, SCHEMA_VERSION, } from './provider'; +export { default as SuggestionDiff, wordDiff } from './suggestion-diff'; diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.js index e69d9276550fdc..ffc393a53d7282 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/overlay-context.js @@ -38,6 +38,7 @@ import { useEffect, useMemo, useReducer, + useRef, } from '@wordpress/element'; import { useRegistry, useSelect } from '@wordpress/data'; @@ -81,6 +82,8 @@ const OverlayContext = createContext( { setOverlayAttributes: () => {}, clearOverlay: () => {}, hasOverlay: () => false, + requestInterceptorBypass: () => {}, + consumeInterceptorBypass: () => false, } ); /** @@ -203,6 +206,30 @@ export function SuggestionOverlayProvider( { children } ) { [ entries ] ); + // Tracks clientIds whose next block-attribute mutation should bypass the + // store interceptor. The accept-suggestion flow uses this to land applied + // attributes on the live block — without it, the interceptor would treat + // the apply as just another user edit and revert it into the overlay. + // A ref-set rather than reducer state because the value is consumed + // inside `registry.subscribe` (which doesn't react to React state) and + // must clear synchronously when the dispatch is processed. + const bypassClientIdsRef = useRef( new Set() ); + + const requestInterceptorBypass = useCallback( ( clientId ) => { + if ( clientId ) { + bypassClientIdsRef.current.add( clientId ); + } + }, [] ); + + const consumeInterceptorBypass = useCallback( ( clientId ) => { + const set = bypassClientIdsRef.current; + if ( ! set.has( clientId ) ) { + return false; + } + set.delete( clientId ); + return true; + }, [] ); + // Prune overlay entries whose block was removed from the editor. This // prevents stale baselines from persisting after a block is deleted. // The block-count subscription only runs when there are entries to @@ -245,6 +272,8 @@ export function SuggestionOverlayProvider( { children } ) { setOverlayAttributes, clearOverlay, hasOverlay, + requestInterceptorBypass, + consumeInterceptorBypass, } ), [ entries, @@ -252,6 +281,8 @@ export function SuggestionOverlayProvider( { children } ) { setOverlayAttributes, clearOverlay, hasOverlay, + requestInterceptorBypass, + consumeInterceptorBypass, ] ); diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index 75bf6bb4491939..cee28a41a0bd65 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -12,6 +12,7 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { EDITOR_STORE_NAME } from './constants'; +import { useSuggestionOverlay } from './overlay-context'; /** * @typedef {Object} SuggestionOperation @@ -107,7 +108,17 @@ function isAttributeEqual( a, b ) { if ( a === null || a === undefined || b === null || b === undefined ) { return false; } - if ( typeof a !== 'object' || typeof b !== 'object' ) { + // One side is a primitive (typically a string from a JSON-deserialized + // suggestion payload) and the other is a wrapper object (typically a + // `RichTextData` instance from the live block-editor store). Compare + // their string representations so the same logical content reads as + // equal across the serialization boundary. + const aIsObject = typeof a === 'object'; + const bIsObject = typeof b === 'object'; + if ( aIsObject !== bIsObject ) { + return String( a ) === String( b ); + } + if ( ! aIsObject ) { return false; } const aIsArray = Array.isArray( a ); @@ -143,13 +154,53 @@ function isAttributeEqual( a, b ) { } /** - * Comment-meta backed suggestions provider. Phase 2 implements only - * `createSuggestion`. Apply and reject are stubbed and will be implemented - * alongside the diff preview in Phase 3. The provider shape is stable so - * Phase 3 / a future Yjs-backed provider can swap in without touching the - * UI. + * Apply a suggestion payload's operations to a block's current attributes + * to produce the new attributes. Pure function — no side effects. + * + * @param {Object} currentAttributes Block's current attributes. + * @param {SuggestionOperation[]} operations Operations from the payload. + * @return {Object} Merged attributes with suggestions applied. + */ +export function applyOperations( currentAttributes, operations ) { + const result = { ...currentAttributes }; + for ( const op of operations ) { + if ( op.type === 'attribute-set' ) { + result[ op.attribute ] = op.after; + } + } + return result; +} + +/** + * Parse a `_wp_suggestion` meta value into a typed payload. + * + * @param {string|undefined} raw The raw JSON string from comment meta. + * @return {SuggestionPayload|null} Parsed payload, or null if invalid. + */ +export function parseSuggestionPayload( raw ) { + if ( ! raw ) { + return null; + } + try { + const parsed = JSON.parse( raw ); + if ( + typeof parsed === 'object' && + parsed !== null && + Array.isArray( parsed.operations ) + ) { + return parsed; + } + return null; + } catch { + return null; + } +} + +/** + * Comment-meta backed suggestions provider. The provider shape is stable so + * a future Yjs-backed provider can swap in without touching the UI. * - * Storage: a new `note` comment with the suggestion payload serialized to + * Storage: a `note` comment with the suggestion payload serialized to * the `_wp_suggestion` comment meta. Linkage to a block reuses the existing * `metadata.noteId` block attribute. * @@ -183,6 +234,7 @@ export function useSuggestionsProvider() { const { updateBlockAttributes } = useDispatch( blockEditorStore ); const { getBlockAttributes: selectBlockAttributes } = useSelect( blockEditorStore ); + const { requestInterceptorBypass, clearOverlay } = useSuggestionOverlay(); const createSuggestion = useCallback( async ( { clientId, blockName, operations } ) => { @@ -266,12 +318,161 @@ export function useSuggestionsProvider() { ] ); - const applySuggestion = useCallback( async () => { - throw new Error( 'applySuggestion is not implemented in Phase 2.' ); - }, [] ); - const rejectSuggestion = useCallback( async () => { - throw new Error( 'rejectSuggestion is not implemented in Phase 2.' ); - }, [] ); + /** + * Apply a suggestion to the live block, then persist the lifecycle + * status to the comment meta. On a server failure the block is rolled + * back so the UI is never left in a half-applied state. + * + * @param {Object} args Apply arguments. + * @param {number|string} args.commentId Comment id holding the + * suggestion (`_wp_suggestion` + * meta). + * @param {string} args.clientId Block client id of the apply + * target. + * @param {SuggestionPayload} args.payload Parsed payload (from + * `parseSuggestionPayload`). + * @return {Promise} + */ + const applySuggestion = useCallback( + async ( { commentId, clientId, payload } ) => { + if ( ! payload || ! Array.isArray( payload.operations ) ) { + createNotice( 'error', __( 'Invalid suggestion payload.' ), { + type: 'snackbar', + isDismissible: true, + } ); + return; + } + + if ( + payload.baseRevision && + postModified && + payload.baseRevision !== postModified + ) { + createNotice( + 'warning', + __( + 'Post content has changed since this suggestion. Review carefully.' + ), + { type: 'snackbar', isDismissible: true } + ); + } + + const currentAttributes = selectBlockAttributes( clientId ); + const newAttributes = applyOperations( + currentAttributes, + payload.operations + ); + + // Build a rollback payload that covers exactly the keys this + // apply touched. `updateBlockAttributes` is a partial merge — + // passing `currentAttributes` alone would leave keys that the + // apply newly added stuck on the block (set to their `after` + // value), since they have no entry in the original attributes + // to override them. Listing each touched key with its original + // value (or `undefined` when the key was added by this apply) + // restores the block cleanly. + const rollbackPayload = {}; + for ( const op of payload.operations ) { + if ( op.type !== 'attribute-set' ) { + continue; + } + rollbackPayload[ op.attribute ] = + Object.prototype.hasOwnProperty.call( + currentAttributes ?? {}, + op.attribute + ) + ? currentAttributes[ op.attribute ] + : undefined; + } + + try { + // Bypass the suggest-mode interceptor for this dispatch so + // the applied attributes actually land on the live block + // instead of being reverted into the overlay. Clearing the + // overlay entry resets the per-block suggestion tracking, + // so any subsequent user edit captures a fresh baseline + // from the post-apply attributes. Outside Suggest mode the + // interceptor isn't running and these calls are no-ops. + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + updateBlockAttributes( clientId, newAttributes ); + + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'applied' }, + }, + { throwOnError: true } + ); + + createNotice( 'snackbar', __( 'Suggestion applied.' ), { + type: 'snackbar', + isDismissible: true, + } ); + } catch ( error ) { + // Roll back the block change so the UI isn't left in a + // half-applied state if the server rejected the update. + requestInterceptorBypass( clientId ); + updateBlockAttributes( clientId, rollbackPayload ); + createNotice( + 'error', + error?.message || __( 'Failed to save suggestion status.' ), + { type: 'snackbar', isDismissible: true } + ); + } + }, + [ + postModified, + saveEntityRecord, + updateBlockAttributes, + selectBlockAttributes, + createNotice, + requestInterceptorBypass, + clearOverlay, + ] + ); + + /** + * Reject a suggestion by setting the comment's lifecycle status. The + * comment itself stays as a thread (status `approved`) so the + * conversation persists as evidence that the suggestion was reviewed. + * + * @param {Object} args Reject arguments. + * @param {number|string} args.commentId Comment id of the rejected + * suggestion. + * @return {Promise} + */ + const rejectSuggestion = useCallback( + async ( { commentId } ) => { + try { + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'rejected' }, + }, + { throwOnError: true } + ); + + createNotice( 'snackbar', __( 'Suggestion rejected.' ), { + type: 'snackbar', + isDismissible: true, + } ); + } catch ( error ) { + createNotice( + 'error', + error?.message || __( 'Failed to reject suggestion.' ), + { type: 'snackbar', isDismissible: true } + ); + } + }, + [ saveEntityRecord, createNotice ] + ); return { createSuggestion, applySuggestion, rejectSuggestion }; } diff --git a/packages/editor/src/components/suggestion-mode/store-interceptor.js b/packages/editor/src/components/suggestion-mode/store-interceptor.js index 8f83bd2e0d73d9..459fc1551133a7 100644 --- a/packages/editor/src/components/suggestion-mode/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/store-interceptor.js @@ -36,15 +36,39 @@ */ import { useRegistry, useSelect } from '@wordpress/data'; import { useEffect, useRef } from '@wordpress/element'; +import { store as coreStore } from '@wordpress/core-data'; /** * Internal dependencies */ import { useSuggestionOverlay } from './overlay-context'; import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; +import { parseSuggestionPayload } from './provider'; const BLOCK_EDITOR_STORE_NAME = 'core/block-editor'; +/** + * Read note IDs from a block's `metadata.noteId`. Originally a scalar; the + * multi-note threads change (issue #75147) widens it to an array. Inlined + * here so this module doesn't depend on a helper that lands in a later + * stack PR — it accepts either shape and returns a normalized array. + * + * @param {Object|undefined} metadata Block metadata. + * @return {Array} Normalized note ids (empty when none). + */ +function readNoteIds( metadata ) { + const value = metadata?.noteId; + if ( Array.isArray( value ) ) { + return value.filter( + ( id ) => id !== null && id !== undefined && id !== '' + ); + } + if ( value === null || value === undefined || value === '' ) { + return []; + } + return [ value ]; +} + /** * Keys under `metadata` that are programmatic linkages set by editor-internal * code (the suggestion provider writes `metadata.noteId` after creating a @@ -72,7 +96,16 @@ function shallowAttributeEquals( a, b ) { if ( a === null || a === undefined || b === null || b === undefined ) { return false; } - if ( typeof a !== 'object' || typeof b !== 'object' ) { + const aIsObject = typeof a === 'object'; + const bIsObject = typeof b === 'object'; + if ( aIsObject !== bIsObject ) { + // One side is a wrapper (e.g. RichTextData) and the other is a + // primitive (e.g. a string from a JSON-decoded suggestion payload). + // Compare their string projections so the two sides of that + // serialization boundary read as equal when their content matches. + return String( a ) === String( b ); + } + if ( ! aIsObject ) { return false; } const aIsArray = Array.isArray( a ); @@ -96,6 +129,14 @@ function shallowAttributeEquals( a, b ) { if ( aKeys.length !== bKeys.length ) { return false; } + // Wrapper objects like `RichTextData` hold their content in private + // class fields, so `Object.keys()` returns an empty array regardless + // of the text or formatting they wrap. Without a string fallback the + // loop below is vacuously equal and two different RichTextData values + // look identical to the interceptor. + if ( aKeys.length === 0 ) { + return String( a ) === String( b ); + } for ( const key of aKeys ) { if ( ! Object.prototype.hasOwnProperty.call( b, key ) ) { return false; @@ -227,6 +268,74 @@ function stripSystemMetadata( changed ) { return { ...changed, metadata: stripped }; } +/** + * Detect whether a delta represents the acceptance of a known suggestion that + * has been propagated to this client — most often, another peer clicked + * "Apply suggestion" and the resulting block-attribute change arrived through + * the sync layer. For each note linked to the block via `metadata.noteId`, + * the comment's `_wp_suggestion` payload is consulted; when every changed + * attribute lands on an `after` value declared by one of those payloads, the + * change is treated as an apply and the interceptor adopts `current` as the + * new baseline rather than reverting. + * + * Without this check the interceptor — running on the suggester's side — + * reverts the incoming applied attributes, then propagates that revert back + * through the sync layer, which undoes the apply on the accepter's screen + * a moment after they clicked. + * + * @param {Object|null} coreSelect Selectors for the core-data store, + * or `null` when the store isn't + * registered (e.g. unit tests). + * @param {Object} currentAttributes Block attributes after the mutation. + * @param {Object} delta Output of `diffAttributes`. + * @return {boolean} True when every changed key matches a suggestion's `after`. + */ +function isAcceptedSuggestionChange( coreSelect, currentAttributes, delta ) { + if ( ! coreSelect?.getEntityRecord ) { + return false; + } + const noteIds = readNoteIds( currentAttributes?.metadata ); + if ( noteIds.length === 0 ) { + return false; + } + const changedKeys = Object.keys( delta.changed ); + if ( changedKeys.length === 0 ) { + return false; + } + + const matched = new Set(); + for ( const noteId of noteIds ) { + const comment = coreSelect.getEntityRecord( 'root', 'comment', noteId ); + const payload = parseSuggestionPayload( comment?.meta?._wp_suggestion ); + if ( ! payload ) { + continue; + } + for ( const op of payload.operations ) { + if ( op.type !== 'attribute-set' ) { + continue; + } + if ( + ! Object.prototype.hasOwnProperty.call( + delta.changed, + op.attribute + ) + ) { + continue; + } + if ( + shallowAttributeEquals( + op.after, + currentAttributes?.[ op.attribute ] + ) + ) { + matched.add( op.attribute ); + } + } + } + + return changedKeys.every( ( key ) => matched.has( key ) ); +} + /** * Invisible component that catches block-attribute mutations dispatched * directly to the block-editor store while the editor is in Suggest intent. @@ -249,8 +358,12 @@ function stripSystemMetadata( changed ) { * @return {null} Renders nothing. */ export default function SuggestionStoreInterceptor() { - const { entries, captureBaseline, setOverlayAttributes } = - useSuggestionOverlay(); + const { + entries, + captureBaseline, + setOverlayAttributes, + consumeInterceptorBypass, + } = useSuggestionOverlay(); const registry = useRegistry(); const isSuggestMode = useSelect( @@ -270,6 +383,9 @@ export default function SuggestionStoreInterceptor() { const setOverlayAttributesRef = useRef( setOverlayAttributes ); setOverlayAttributesRef.current = setOverlayAttributes; + const consumeInterceptorBypassRef = useRef( consumeInterceptorBypass ); + consumeInterceptorBypassRef.current = consumeInterceptorBypass; + useEffect( () => { if ( ! isSuggestMode ) { return undefined; @@ -283,6 +399,10 @@ export default function SuggestionStoreInterceptor() { return undefined; } + // `coreStore` may be unregistered in unit tests; the helper that + // reads suggestion comments handles a `null` selector defensively. + const coreSelect = registry.select( coreStore ); + // Snapshot of every block's attributes at the moment Suggest mode // activated. New blocks added during the session are slotted in as // they appear; mutations on existing blocks are reverted + overlaid. @@ -312,6 +432,15 @@ export default function SuggestionStoreInterceptor() { let previous = snapshot.get( clientId ); const current = blockEditor.getBlockAttributes( clientId ); + // Apply-suggestion flow: the provider opts the next mutation + // out of interception so the applied attributes land on the + // live block. Adopt `current` as the new baseline so the + // next user edit diffs against the post-apply state. + if ( consumeInterceptorBypassRef.current?.( clientId ) ) { + snapshot.set( clientId, current ); + continue; + } + if ( previous === undefined ) { // New block (inserted after Suggest activated). Track it // but don't intercept. @@ -344,6 +473,19 @@ export default function SuggestionStoreInterceptor() { continue; } + // Remote sync of a suggestion accepted on another client: + // when every changed attribute matches the `after` value of + // a suggestion attached to this block, adopt the change as + // the new baseline. Without this branch the interceptor + // reverts the apply, and the revert round-trips back through + // the sync layer, undoing the apply on the accepter's screen. + if ( + isAcceptedSuggestionChange( coreSelect, current, delta ) + ) { + snapshot.set( clientId, current ); + continue; + } + // Capture a baseline if one isn't already set. The HOC's // own captureBaseline only fires for `setAttributes` calls; // for store-level mutations we have to seed one here. @@ -402,4 +544,5 @@ export { shallowAttributeEquals, adoptSystemMetadata, stripSystemMetadata, + isAcceptedSuggestionChange, }; diff --git a/packages/editor/src/components/suggestion-mode/suggestion-diff.js b/packages/editor/src/components/suggestion-mode/suggestion-diff.js new file mode 100644 index 00000000000000..87aadb9654e9bb --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/suggestion-diff.js @@ -0,0 +1,230 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { __experimentalText as WCText } from '@wordpress/components'; +import { Stack, VisuallyHidden } from '@wordpress/ui'; +import { useMemo } from '@wordpress/element'; + +/** + * Upper bound for word-level LCS input length (characters). Beyond this, + * we fall back to an attribute-level before→after label to avoid the + * O(m·n) diff dominating the render. Two 2 KB strings produce a 4M-cell DP + * table, which is the practical ceiling for an interactive sidebar render. + */ +const MAX_DIFF_LENGTH = 2000; + +/** + * Compute a word-level diff between two strings, returning an array of + * segments tagged as `equal`, `insert`, or `delete`. + * + * @param {string} before Original text. + * @param {string} after Proposed text. + * @return {Array<{type: string, value: string}>} Diff segments. + */ +export function wordDiff( before, after ) { + const a = tokenize( before ); + const b = tokenize( after ); + const lcs = longestCommonSubsequence( a, b ); + + const result = []; + let ai = 0; + let bi = 0; + + for ( const token of lcs ) { + while ( ai < a.length && a[ ai ] !== token ) { + result.push( { type: 'delete', value: a[ ai ] } ); + ai++; + } + while ( bi < b.length && b[ bi ] !== token ) { + result.push( { type: 'insert', value: b[ bi ] } ); + bi++; + } + result.push( { type: 'equal', value: token } ); + ai++; + bi++; + } + + while ( ai < a.length ) { + result.push( { type: 'delete', value: a[ ai ] } ); + ai++; + } + while ( bi < b.length ) { + result.push( { type: 'insert', value: b[ bi ] } ); + bi++; + } + + return result; +} + +function tokenize( str ) { + if ( typeof str !== 'string' ) { + return []; + } + return str.match( /\S+|\s+/g ) || []; +} + +function longestCommonSubsequence( a, b ) { + const m = a.length; + const n = b.length; + const dp = Array.from( { length: m + 1 }, () => + new Array( n + 1 ).fill( 0 ) + ); + + for ( let i = 1; i <= m; i++ ) { + for ( let j = 1; j <= n; j++ ) { + dp[ i ][ j ] = + a[ i - 1 ] === b[ j - 1 ] + ? dp[ i - 1 ][ j - 1 ] + 1 + : Math.max( dp[ i - 1 ][ j ], dp[ i ][ j - 1 ] ); + } + } + + const result = []; + let i = m; + let j = n; + while ( i > 0 && j > 0 ) { + if ( a[ i - 1 ] === b[ j - 1 ] ) { + result.unshift( a[ i - 1 ] ); + i--; + j--; + } else if ( dp[ i - 1 ][ j ] > dp[ i ][ j - 1 ] ) { + i--; + } else { + j--; + } + } + return result; +} + +/** + * Renders a compact inline diff preview for a suggestion's operations. + * Text-valued attributes show word-level insertions (green underline) and + * deletions (red strikethrough). Non-text attributes show a before → after + * label. + * + * Operations are expected to come from `parseSuggestionPayload(...)`. Each + * operation has the shape `{ type: 'attribute-set', attribute, before, after }`. + * + * Edge cases: + * - When either side is too large (`>= MAX_DIFF_LENGTH` chars), the + * component falls back to an attribute-level label rather than a word + * diff to avoid the LCS dominating the render time. + * - When `before` or `after` is a wrapper object (e.g. `RichTextData`), + * `isTextValue` returns false and the component renders the + * attribute-level label. The provider has already serialized wrapper + * boundaries to strings before this layer in normal flow. + * + * @param {Object} props + * @param {import('./provider').SuggestionOperation[]} props.operations Suggestion + * operations, + * typically the + * `operations` + * array from a + * parsed payload. + */ +export default function SuggestionDiff( { operations } ) { + if ( ! operations || operations.length === 0 ) { + return null; + } + + return ( + + + { __( 'Suggested change' ) } + + { operations.map( ( op, index ) => { + const canWordDiff = + op.type === 'attribute-set' && + isTextValue( op.before ) && + isTextValue( op.after ) && + ( op.before?.length ?? 0 ) <= MAX_DIFF_LENGTH && + ( op.after?.length ?? 0 ) <= MAX_DIFF_LENGTH; + const key = `${ op.type }:${ op.attribute }:${ index }`; + return ( +
+ { canWordDiff ? ( + + ) : ( + + ) } +
+ ); + } ) } + + ); +} + +function isTextValue( value ) { + return value === null || value === undefined || typeof value === 'string'; +} + +function TextDiff( { before, after } ) { + // The LCS below is O(m·n) in time and space. Memoize so repeated + // sidebar renders don't repay the cost. + const segments = useMemo( + () => wordDiff( before, after ), + [ before, after ] + ); + return ( + + { segments.map( ( seg, i ) => { + if ( seg.type === 'delete' ) { + return ( + + + { __( 'Deleted:' ) } + + { seg.value } + + ); + } + if ( seg.type === 'insert' ) { + return ( + + + { __( 'Inserted:' ) } + + { seg.value } + + ); + } + return { seg.value }; + } ) } + + ); +} + +function AttributeDiff( { operation } ) { + const label = + typeof operation.before === 'string' + ? `${ operation.attribute }: ${ operation.before } → ${ operation.after }` + : `${ operation.attribute }: changed`; + return ( + + { label } + + ); +} diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js index 4540e4d2cf1519..f9d6ccbe9cdb5c 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -3,6 +3,8 @@ */ import { operationsFromOverlay, + applyOperations, + parseSuggestionPayload, payloadByteLength, PAYLOAD_MAX_BYTES, } from '../provider'; @@ -95,6 +97,40 @@ describe( 'operationsFromOverlay', () => { } ); } ); +describe( 'applyOperations', () => { + it( 'applies attribute-set operations to produce new attributes', () => { + const result = applyOperations( + { content: 'Hello', level: 2, align: 'left' }, + [ + { + type: 'attribute-set', + attribute: 'content', + before: 'Hello', + after: 'Hi', + }, + { + type: 'attribute-set', + attribute: 'level', + before: 2, + after: 3, + }, + ] + ); + expect( result ).toEqual( { + content: 'Hi', + level: 3, + align: 'left', + } ); + } ); + + it( 'returns a copy even when operations are empty', () => { + const attrs = { content: 'Same' }; + const result = applyOperations( attrs, [] ); + expect( result ).toEqual( attrs ); + expect( result ).not.toBe( attrs ); + } ); +} ); + describe( 'payloadByteLength', () => { it( 'measures ASCII payload byte length', () => { // {"a":"hello"} is 13 bytes. @@ -111,3 +147,35 @@ describe( 'payloadByteLength', () => { expect( typeof PAYLOAD_MAX_BYTES ).toBe( 'number' ); } ); } ); + +describe( 'parseSuggestionPayload', () => { + it( 'parses a valid JSON payload', () => { + const raw = JSON.stringify( { + schemaVersion: 1, + blockName: 'core/paragraph', + baseRevision: '2026-04-15T00:00:00', + operations: [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ], + } ); + const result = parseSuggestionPayload( raw ); + expect( result ).not.toBeNull(); + expect( result.operations ).toHaveLength( 1 ); + expect( result.blockName ).toBe( 'core/paragraph' ); + } ); + + it( 'returns null for missing, empty, or invalid input', () => { + expect( parseSuggestionPayload( undefined ) ).toBeNull(); + expect( parseSuggestionPayload( '' ) ).toBeNull(); + expect( parseSuggestionPayload( 'not json' ) ).toBeNull(); + expect( parseSuggestionPayload( '42' ) ).toBeNull(); + expect( + parseSuggestionPayload( JSON.stringify( { noOps: true } ) ) + ).toBeNull(); + } ); +} ); diff --git a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js index 2bf87a68088856..32972ce5790084 100644 --- a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js @@ -25,6 +25,7 @@ import SuggestionStoreInterceptor, { shallowAttributeEquals, adoptSystemMetadata, stripSystemMetadata, + isAcceptedSuggestionChange, } from '../store-interceptor'; import { SuggestionOverlayProvider, @@ -317,6 +318,228 @@ describe( 'SuggestionStoreInterceptor (integration)', () => { getOverlay().entries[ clientId ]?.overlayAttributes?.content ).toBe( 'Edited' ); } ); + + it( 'lands an apply-style mutation on the live block when bypass is requested', async () => { + // The accept/reject suggestion flow calls `updateBlockAttributes` + // directly on the block-editor store. Without an explicit bypass + // the interceptor would mistake the apply for a user edit and + // revert it — so "Accept suggestion" would silently no-op while + // in Suggest mode. The provider opts into a bypass for the apply + // dispatch and clears the overlay so the new attributes become + // the new baseline. + const { registry, clientId, getOverlay } = setup(); + + // User had a pending suggestion overlay on this block. + await act( async () => { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( clientId, { content: 'Suggested' } ); + } ); + await flushSubscribers(); + + // Sanity: the user-style edit was reverted into the overlay. + expect( + registry.select( blockEditorStore ).getBlockAttributes( clientId ) + ?.content + ).toBe( 'Hello' ); + + // Now run the apply path the way the provider will: ask the + // interceptor to bypass the next dispatch, drop the overlay, and + // write the applied attributes. + await act( async () => { + getOverlay().requestInterceptorBypass( clientId ); + getOverlay().clearOverlay( clientId ); + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( clientId, { content: 'Applied' } ); + } ); + await flushSubscribers(); + + const liveAttributes = registry + .select( blockEditorStore ) + .getBlockAttributes( clientId ); + + // The applied attributes land on the live block — they aren't + // reverted to baseline. + expect( liveAttributes?.content ).toBe( 'Applied' ); + // The overlay entry is gone, so future edits start from a fresh + // baseline that reflects the applied state. + expect( getOverlay().entries[ clientId ] ).toBeUndefined(); + + // A subsequent user-style edit is still intercepted normally and + // rebaselines against the post-apply attributes. + await act( async () => { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( clientId, { content: 'After apply' } ); + } ); + await flushSubscribers(); + + expect( + registry.select( blockEditorStore ).getBlockAttributes( clientId ) + ?.content + ).toBe( 'Applied' ); + expect( + getOverlay().entries[ clientId ]?.overlayAttributes?.content + ).toBe( 'After apply' ); + } ); +} ); + +describe( 'isAcceptedSuggestionChange', () => { + function makeCoreSelect( comments ) { + return { + getEntityRecord: ( _kind, _name, id ) => + comments[ String( id ) ] ?? null, + }; + } + + function makePayload( operations ) { + return { + meta: { + _wp_suggestion: JSON.stringify( { + schemaVersion: 1, + operations, + } ), + }, + }; + } + + it( 'returns false when the block has no linked notes', () => { + const coreSelect = makeCoreSelect( {} ); + const delta = { changed: { content: 'Edited' }, restore: {} }; + expect( + isAcceptedSuggestionChange( + coreSelect, + { content: 'Edited' }, + delta + ) + ).toBe( false ); + } ); + + it( 'returns false when the change does not match any suggestion `after`', () => { + const coreSelect = makeCoreSelect( { + 42: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + after: 'Suggested', + }, + ] ), + } ); + const current = { content: 'Edited', metadata: { noteId: [ 42 ] } }; + const delta = { changed: { content: 'Edited' }, restore: {} }; + expect( isAcceptedSuggestionChange( coreSelect, current, delta ) ).toBe( + false + ); + } ); + + it( 'returns true when the change matches a suggestion `after` value', () => { + const coreSelect = makeCoreSelect( { + 42: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + after: 'Suggested', + }, + ] ), + } ); + const current = { content: 'Suggested', metadata: { noteId: [ 42 ] } }; + const delta = { changed: { content: 'Suggested' }, restore: {} }; + expect( isAcceptedSuggestionChange( coreSelect, current, delta ) ).toBe( + true + ); + } ); + + it( 'matches across multiple notes and multiple changed keys', () => { + const coreSelect = makeCoreSelect( { + 42: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + after: 'Suggested', + }, + ] ), + 43: makePayload( [ + { type: 'attribute-set', attribute: 'level', after: 3 }, + ] ), + } ); + const current = { + content: 'Suggested', + level: 3, + metadata: { noteId: [ 42, 43 ] }, + }; + const delta = { + changed: { content: 'Suggested', level: 3 }, + restore: {}, + }; + expect( isAcceptedSuggestionChange( coreSelect, current, delta ) ).toBe( + true + ); + } ); + + it( 'returns false when only some changed keys are matched', () => { + const coreSelect = makeCoreSelect( { + 42: makePayload( [ + { + type: 'attribute-set', + attribute: 'content', + after: 'Suggested', + }, + ] ), + } ); + const current = { + content: 'Suggested', + level: 3, + metadata: { noteId: [ 42 ] }, + }; + const delta = { + changed: { content: 'Suggested', level: 3 }, + restore: {}, + }; + expect( isAcceptedSuggestionChange( coreSelect, current, delta ) ).toBe( + false + ); + } ); + + it( 'returns false when the core-data selectors are unavailable', () => { + const current = { content: 'Suggested', metadata: { noteId: [ 42 ] } }; + const delta = { changed: { content: 'Suggested' }, restore: {} }; + expect( isAcceptedSuggestionChange( null, current, delta ) ).toBe( + false + ); + } ); + + it( 'matches across the wrapper/string boundary (RichTextData vs JSON)', () => { + // Mimic a RichTextData wrapper: an object with no enumerable keys + // whose `String()` projection equals the JSON-decoded `after` value. + class FakeRichTextData { + constructor( text ) { + Object.defineProperty( this, '_text', { + value: text, + enumerable: false, + } ); + } + toString() { + return this._text; + } + } + const coreSelect = makeCoreSelect( { + 42: makePayload( [ + { type: 'attribute-set', attribute: 'content', after: 'Hello' }, + ] ), + } ); + const current = { + content: new FakeRichTextData( 'Hello' ), + metadata: { noteId: [ 42 ] }, + }; + const delta = { + changed: { content: new FakeRichTextData( 'Hello' ) }, + restore: {}, + }; + expect( isAcceptedSuggestionChange( coreSelect, current, delta ) ).toBe( + true + ); + } ); } ); describe( 'stripSystemMetadata', () => { diff --git a/packages/editor/src/components/suggestion-mode/test/suggestion-diff.js b/packages/editor/src/components/suggestion-mode/test/suggestion-diff.js new file mode 100644 index 00000000000000..1bcb499884a0ef --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/suggestion-diff.js @@ -0,0 +1,73 @@ +/** + * Internal dependencies + */ +import { wordDiff } from '../suggestion-diff'; + +describe( 'wordDiff', () => { + it( 'returns equal segments for identical strings', () => { + expect( wordDiff( 'hello world', 'hello world' ) ).toEqual( [ + { type: 'equal', value: 'hello' }, + { type: 'equal', value: ' ' }, + { type: 'equal', value: 'world' }, + ] ); + } ); + + it( 'detects insertions', () => { + const result = wordDiff( 'hello world', 'hello beautiful world' ); + const types = result.map( ( s ) => s.type ); + expect( types ).toContain( 'insert' ); + const inserted = result.filter( ( s ) => s.type === 'insert' ); + expect( inserted.map( ( s ) => s.value.trim() ) ).toContain( + 'beautiful' + ); + } ); + + it( 'detects deletions', () => { + const result = wordDiff( 'hello beautiful world', 'hello world' ); + const deleted = result.filter( ( s ) => s.type === 'delete' ); + expect( deleted.map( ( s ) => s.value.trim() ) ).toContain( + 'beautiful' + ); + } ); + + it( 'handles replacement as delete+insert', () => { + const result = wordDiff( 'the cat sat', 'the dog sat' ); + expect( result ).toEqual( + expect.arrayContaining( [ + expect.objectContaining( { type: 'delete', value: 'cat' } ), + expect.objectContaining( { type: 'insert', value: 'dog' } ), + ] ) + ); + } ); + + it( 'handles empty before (all insertions)', () => { + const result = wordDiff( '', 'new text' ); + expect( result.every( ( s ) => s.type === 'insert' ) ).toBe( true ); + } ); + + it( 'handles empty after (all deletions)', () => { + const result = wordDiff( 'old text', '' ); + expect( result.every( ( s ) => s.type === 'delete' ) ).toBe( true ); + } ); + + it( 'handles null/undefined gracefully', () => { + expect( wordDiff( null, 'hello' ) ).toEqual( [ + { type: 'insert', value: 'hello' }, + ] ); + expect( wordDiff( 'hello', null ) ).toEqual( [ + { type: 'delete', value: 'hello' }, + ] ); + } ); + + it( 'segments token-by-token at word boundaries', () => { + // Replacing 'b' with 'x' in the middle of 'a b c' produces + // delete+insert segments at the changed position. + const result = wordDiff( 'a b c', 'a x c' ); + expect( result ).toEqual( + expect.arrayContaining( [ + expect.objectContaining( { type: 'delete', value: 'b' } ), + expect.objectContaining( { type: 'insert', value: 'x' } ), + ] ) + ); + } ); +} ); diff --git a/tools/eslint/suppressions.json b/tools/eslint/suppressions.json index 47c01ccfeba492..dbc1382a0467bd 100644 --- a/tools/eslint/suppressions.json +++ b/tools/eslint/suppressions.json @@ -935,6 +935,11 @@ "count": 1 } }, + "packages/editor/src/components/collab-sidebar/suggestion-actions.js": { + "@wordpress/use-recommended-components": { + "count": 1 + } + }, "packages/editor/src/components/document-bar/index.js": { "@wordpress/use-recommended-components": { "count": 1 @@ -1110,7 +1115,12 @@ }, "packages/editor/src/components/suggestion-mode/store-interceptor.js": { "react-hooks/refs": { - "count": 3 + "count": 4 + } + }, + "packages/editor/src/components/suggestion-mode/suggestion-diff.js": { + "@wordpress/use-recommended-components": { + "count": 1 } }, "packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js": {