diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index d69f52916fbdb4..0af8afbf3927b7 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -389,7 +389,7 @@ export function useSuggestionsProvider() { const { saveEntityRecord } = useDispatch( coreStore ); const { createNotice } = useDispatch( noticesStore ); - const { updateBlockAttributes, removeBlock } = + const { updateBlockAttributes, removeBlock, moveBlockToPosition } = useDispatch( blockEditorStore ); const { getBlockAttributes: selectBlockAttributes, @@ -661,23 +661,27 @@ export function useSuggestionsProvider() { requestInterceptorBypass( targetClientId ); clearOverlay( targetClientId ); removeBlock( targetClientId ); - } else if ( structuralOp.type === 'block-insert-after' ) { - // The block is already in the live tree (the user - // inserted it during Suggest mode); apply commits - // the captured edits onto the live block AND clears - // the pending-insert marker so the block loses its - // dimmed treatment. + } else if ( + structuralOp.type === 'block-insert-after' || + structuralOp.type === 'block-move' + ) { + // The block is already at its proposed location + // (the user inserted or moved it during Suggest + // mode); apply commits the captured edits onto the + // live block AND clears the pending marker so the + // block loses its dimmed/outlined treatment. // // Attribute-set ops in the same payload represent - // edits the user made between insertion and auto- - // save. They never reach the live block on the - // suggester's side — the interceptor reverts them - // into the overlay — so collaborators (and the - // suggester after a reload) see the live block in - // the captured shape (typically empty content for - // a fresh paragraph). Apply must materialize those - // edits on the live block, otherwise the inserted - // block ends up empty after acceptance. + // edits the user made between the structural + // change and auto-save. They never reach the live + // block on the suggester's side — the interceptor + // reverts them into the overlay — so collaborators + // (and the suggester after a reload) see the live + // block in the captured shape (typically empty + // content for a fresh paragraph). Apply must + // materialize those edits on the live block, + // otherwise the inserted/moved block ends up in + // the wrong shape after acceptance. const currentAttributes = selectBlockAttributes( targetClientId ); const withOpsApplied = applyOperations( @@ -829,6 +833,9 @@ export function useSuggestionsProvider() { // - block-insert-after: dispatch removeBlock to undo the // suggested insertion. The marker on the live block goes // away with the block itself. + // - block-move: clear the marker, then dispatch + // moveBlockToPosition to put the block back at its + // pre-move parent + index. // - attribute-set (no structural op): no live-block change. const structuralOp = findStructuralOp( payload?.operations ); if ( structuralOp && clientId ) { @@ -836,6 +843,24 @@ export function useSuggestionsProvider() { requestInterceptorBypass( clientId ); clearOverlay( clientId ); removeBlock( clientId ); + } else if ( structuralOp.type === 'block-move' ) { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( clientId ); + updateBlockAttributes( clientId, clearAttrs ); + } + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + moveBlockToPosition( + clientId, + // `moveBlockToPosition` expects '' (not null) for + // the root. + structuralOp.fromParentClientId ?? '', + structuralOp.fromParentClientId ?? '', + structuralOp.fromIndex ?? 0 + ); } else { const clearAttrs = clearSuggestionMarkerAttributes( selectBlockAttributes( clientId ) @@ -878,6 +903,7 @@ export function useSuggestionsProvider() { selectBlockAttributes, updateBlockAttributes, removeBlock, + moveBlockToPosition, requestInterceptorBypass, clearOverlay, ] diff --git a/packages/editor/src/components/suggestion-mode/store-interceptor.js b/packages/editor/src/components/suggestion-mode/store-interceptor.js index 81c52e3d078314..07ca2b08b78715 100644 --- a/packages/editor/src/components/suggestion-mode/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/store-interceptor.js @@ -33,7 +33,9 @@ * prior parent + index, then tagged `metadata.suggestion = pending-remove`. * - New blocks → tagged `metadata.suggestion = pending-insert` (the block * stays in the tree; the marker drives the dimmed visual treatment). - * - Move detection ships in a follow-up. + * - Moved blocks → tagged `metadata.suggestion = pending-move` with the + * pre-move parent + anchor; an LCS-based heuristic isolates the moved + * block from siblings whose index just shifted as a side-effect. * * In every case the live block carries the marker and a corresponding * structural op is written to the overlay so auto-save persists it. @@ -432,6 +434,171 @@ function topLevelRemoved( removedIds, parentByClientId ) { } ); } +/** + * Length of the longest common subsequence of two arrays of clientIds (the + * elements of the result appear in the same relative order in both inputs). + * Used by the move-detection heuristic: blocks NOT in the LCS of their + * parent's old vs new sibling order are the ones that actually moved; + * blocks in the LCS just had their index shift as a side-effect. + * + * @param {string[]} a First array. + * @param {string[]} b Second array. + * @return {Set} The LCS as a set for O(1) membership checks. + */ +function lcsClientIds( a, b ) { + const m = a.length; + const n = b.length; + if ( m === 0 || n === 0 ) { + return new Set(); + } + 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 = new Set(); + let i = m; + let j = n; + while ( i > 0 && j > 0 ) { + if ( a[ i - 1 ] === b[ j - 1 ] ) { + result.add( a[ i - 1 ] ); + i--; + j--; + } else if ( dp[ i - 1 ][ j ] > dp[ i ][ j - 1 ] ) { + i--; + } else { + j--; + } + } + return result; +} + +/** + * Detect blocks that moved between two ticks of the live tree. A block has + * "moved" when its parent changed (cross-parent move) or, within the same + * parent, its position falls outside the LCS of the parent's old vs new + * sibling order. The LCS heuristic prevents tagging blocks whose index + * just shifted as a side-effect of another block moving past them. + * + * Blocks that are new (not in the previous-tick tree) or removed (in the + * previous-tick tree but not live) are handled by the insertion / removal + * branches; this function ignores both. + * + * @param {string[]} liveClientIds Live tree client ids. + * @param {Object} tree Previous-tick tree snapshot from + * `captureTreeSnapshot`. + * @param {Object} blockEditor Block-editor selectors. + * @return {Array} One entry per moved block, with from/to anchors. + */ +function detectMovedBlocks( liveClientIds, tree, blockEditor ) { + const movedRaw = []; + + const candidatesByNewParent = new Map(); + for ( const clientId of liveClientIds ) { + if ( ! tree.parentByClientId.has( clientId ) ) { + continue; // new block — handled elsewhere + } + const oldParent = tree.parentByClientId.get( clientId ); + const newParent = + blockEditor.getBlockRootClientId?.( clientId ) || null; + if ( oldParent !== newParent ) { + movedRaw.push( { clientId, oldParent, newParent } ); + continue; + } + if ( ! candidatesByNewParent.has( newParent ) ) { + candidatesByNewParent.set( newParent, [] ); + } + candidatesByNewParent.get( newParent ).push( clientId ); + } + + for ( const [ parent, candidates ] of candidatesByNewParent ) { + const oldSiblings = candidates + .slice() + .sort( + ( a, b ) => + ( tree.indexByClientId.get( a ) ?? 0 ) - + ( tree.indexByClientId.get( b ) ?? 0 ) + ); + const newSiblingOrder = + blockEditor.getBlockOrder?.( parent ?? undefined ) ?? []; + const newSiblings = candidates + .slice() + .sort( + ( a, b ) => + newSiblingOrder.indexOf( a ) - newSiblingOrder.indexOf( b ) + ); + const samePosition = + oldSiblings.length === newSiblings.length && + oldSiblings.every( ( id, i ) => id === newSiblings[ i ] ); + if ( samePosition ) { + continue; + } + const stable = lcsClientIds( oldSiblings, newSiblings ); + for ( const clientId of newSiblings ) { + if ( ! stable.has( clientId ) ) { + movedRaw.push( { + clientId, + oldParent: parent, + newParent: parent, + } ); + } + } + } + + if ( movedRaw.length === 0 ) { + return []; + } + + // Reconstruct the previous-tick sibling order per parent so we can + // compute fromAnchorClientId. Building this lazily keeps the no-move + // path zero-cost. + const oldOrderByParent = new Map(); + const oldOrderFor = ( oldParent ) => { + if ( oldOrderByParent.has( oldParent ) ) { + return oldOrderByParent.get( oldParent ); + } + const ids = []; + for ( const [ id, parent ] of tree.parentByClientId ) { + if ( parent === oldParent ) { + ids.push( id ); + } + } + ids.sort( + ( a, b ) => + ( tree.indexByClientId.get( a ) ?? 0 ) - + ( tree.indexByClientId.get( b ) ?? 0 ) + ); + oldOrderByParent.set( oldParent, ids ); + return ids; + }; + + return movedRaw.map( ( { clientId, oldParent, newParent } ) => { + const oldIndex = tree.indexByClientId.get( clientId ) ?? 0; + const oldSiblingOrder = oldOrderFor( oldParent ); + const fromAnchorClientId = + oldIndex > 0 ? oldSiblingOrder[ oldIndex - 1 ] : null; + const newSiblingOrder = + blockEditor.getBlockOrder?.( newParent ?? undefined ) ?? []; + const newIndex = newSiblingOrder.indexOf( clientId ); + const toAnchorClientId = + newIndex > 0 ? newSiblingOrder[ newIndex - 1 ] : null; + return { + clientId, + fromParentClientId: oldParent, + fromAnchorClientId, + fromIndex: oldIndex, + toParentClientId: newParent, + toAnchorClientId, + }; + } ); +} + /** * Invisible component that catches block-attribute mutations dispatched * directly to the block-editor store while the editor is in Suggest intent. @@ -684,6 +851,49 @@ export default function SuggestionStoreInterceptor() { // block; do NOT update it to `current` here. } + // Detect blocks that moved (different parent or out-of-place + // within the same parent). The block stays at its new + // position; tag it with `metadata.suggestion = pending-move` + // and capture the from/to anchors for the structural op. + // Apply leaves the block where it is; Reject dispatches + // `moveBlockToPosition` with the from-position. + const moves = detectMovedBlocks( liveClientIds, tree, blockEditor ); + for ( const move of moves ) { + const currentAttrs = blockEditor.getBlockAttributes?.( + move.clientId + ); + if ( ! currentAttrs ) { + continue; + } + const block = blockEditor.getBlock?.( move.clientId ); + if ( ! block ) { + continue; + } + isReverting = true; + try { + blockEditorDispatch.updateBlockAttributes( move.clientId, { + metadata: withSuggestionMarker( currentAttrs.metadata, { + type: 'pending-move', + fromAnchorClientId: move.fromAnchorClientId, + fromParentClientId: move.fromParentClientId, + fromIndex: move.fromIndex, + } ), + } ); + } finally { + isReverting = false; + } + setStructuralOpRef.current?.( move.clientId, block.name, { + type: 'block-move', + clientId: move.clientId, + blockName: block.name, + fromAnchorClientId: move.fromAnchorClientId, + fromParentClientId: move.fromParentClientId, + fromIndex: move.fromIndex, + toAnchorClientId: move.toAnchorClientId, + toParentClientId: move.toParentClientId, + } ); + } + // Detect blocks that disappeared from the live tree and route // them through the Suggest-mode "apply-and-tag" flow: re-insert // the subtree from the previous-tick snapshot at its previous @@ -837,4 +1047,6 @@ export { captureTreeSnapshot, topLevelRemoved, withSuggestionMarker, + lcsClientIds, + detectMovedBlocks, }; 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 4532a427abdd59..2222189154a471 100644 --- a/packages/editor/src/components/suggestion-mode/test/store-interceptor.js +++ b/packages/editor/src/components/suggestion-mode/test/store-interceptor.js @@ -28,6 +28,7 @@ import SuggestionStoreInterceptor, { isAcceptedSuggestionChange, topLevelRemoved, withSuggestionMarker, + lcsClientIds, } from '../store-interceptor'; import { SuggestionOverlayProvider, @@ -672,6 +673,58 @@ describe( 'SuggestionStoreInterceptor (integration)', () => { } ); } ); + it( 'tags a moved block with metadata.suggestion = pending-move and writes a block-move op', async () => { + // Move a block from index 1 to index 3 in a 4-block tree. The + // moved block carries a pending-move marker with from-position + // context; the side-effect siblings (whose indices shifted to + // fill the gap) are NOT tagged thanks to the LCS heuristic. + const a = createBlock( TEST_BLOCK_NAME, { content: 'A' } ); + const b = createBlock( TEST_BLOCK_NAME, { content: 'B' } ); + const c = createBlock( TEST_BLOCK_NAME, { content: 'C' } ); + const d = createBlock( TEST_BLOCK_NAME, { content: 'D' } ); + const { registry, getOverlay } = setup( { + initialBlocks: [ a, b, c, d ], + } ); + + await act( async () => { + registry + .dispatch( blockEditorStore ) + .moveBlockToPosition( b.clientId, '', '', 3 ); + } ); + await flushSubscribers(); + + const liveBlocks = registry.select( blockEditorStore ).getBlocks(); + expect( liveBlocks.map( ( bl ) => bl.attributes?.content ) ).toEqual( [ + 'A', + 'C', + 'D', + 'B', + ] ); + expect( + liveBlocks.find( ( bl ) => bl.clientId === b.clientId )?.attributes + ?.metadata?.suggestion?.type + ).toBe( 'pending-move' ); + + // Side-effect siblings: NOT tagged. + const sideEffectBlocks = liveBlocks.filter( + ( bl ) => bl.clientId !== b.clientId + ); + for ( const bl of sideEffectBlocks ) { + expect( bl.attributes?.metadata?.suggestion ).toBeUndefined(); + } + + expect( + getOverlay().entries[ b.clientId ]?.structuralOp + ).toMatchObject( { + type: 'block-move', + clientId: b.clientId, + fromAnchorClientId: a.clientId, + fromParentClientId: null, + toAnchorClientId: d.clientId, + toParentClientId: null, + } ); + } ); + it( 'tags only the top-level new block when an inserted subtree contains nested children', async () => { // A Group block with a child paragraph dispatches both as new in // the same tick. The interceptor must tag only the top-level @@ -970,3 +1023,26 @@ describe( 'withSuggestionMarker', () => { ).toEqual( { type: 'pending-remove' } ); } ); } ); + +describe( 'lcsClientIds', () => { + it( 'returns the full sequence when both sides match', () => { + const lcs = lcsClientIds( [ 'a', 'b', 'c' ], [ 'a', 'b', 'c' ] ); + expect( Array.from( lcs ).sort() ).toEqual( [ 'a', 'b', 'c' ] ); + } ); + + it( 'identifies the moved-block as the one absent from the LCS', () => { + // [A, B, C, D] → [A, C, D, B]: B is the moved block; A, C, D form + // the longest stable subsequence. + const lcs = lcsClientIds( + [ 'a', 'b', 'c', 'd' ], + [ 'a', 'c', 'd', 'b' ] + ); + expect( Array.from( lcs ).sort() ).toEqual( [ 'a', 'c', 'd' ] ); + expect( lcs.has( 'b' ) ).toBe( false ); + } ); + + it( 'returns an empty set when either side is empty', () => { + expect( lcsClientIds( [], [ 'a' ] ).size ).toBe( 0 ); + expect( lcsClientIds( [ 'a' ], [] ).size ).toBe( 0 ); + } ); +} );