Skip to content
84 changes: 69 additions & 15 deletions packages/editor/src/components/suggestion-mode/provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,16 @@ export function hasAttributeConflict( currentAttributes, operations ) {
if ( ! Array.isArray( operations ) ) {
return false;
}
// Inserted blocks have no pre-existing attributes — the overlay's
// baseline for a `block-insert-after` entry is `{}`, so every
// attribute-set op rides on `before: null`. Comparing that against the
// live (already-typed-into) block's attributes always reads as
// divergence, which falsely fires the staleness prompt on apply. The
// attribute-set ops describe the inserted block's content, not an
// overwrite of pre-existing data, so there is nothing to conflict with.
if ( findStructuralOp( operations )?.type === 'block-insert-after' ) {
return false;
}
for ( const op of operations ) {
if ( op.type !== 'attribute-set' ) {
continue;
Expand Down Expand Up @@ -627,12 +637,12 @@ export function useSuggestionsProvider() {
return;
}

// Structural ops (block-remove for now, block-insert-after and
// block-move in follow-up PRs) can't ride the
// updateBlockAttributes path: their apply mutates the tree
// rather than a single block's attributes. Branch out, run the
// matching block-editor action, and short-circuit before the
// attribute-set rollback machinery below.
// Structural ops (block-remove, block-insert-after; block-move
// ships in a follow-up) can't ride the updateBlockAttributes
// path: their apply mutates the tree rather than a single
// block's attributes. Branch out, run the matching block-
// editor action, and short-circuit before the attribute-set
// rollback machinery below.
const structuralOp = findStructuralOp( payload.operations );
if ( structuralOp ) {
try {
Expand All @@ -651,6 +661,40 @@ 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.
//
// 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.
const currentAttributes =
selectBlockAttributes( targetClientId );
const withOpsApplied = applyOperations(
currentAttributes,
payload.operations
);
const markerCleared =
clearSuggestionMarkerAttributes( withOpsApplied );
const finalAttributes = markerCleared
? { ...withOpsApplied, ...markerCleared }
: withOpsApplied;
requestInterceptorBypass( targetClientId );
updateBlockAttributes(
targetClientId,
finalAttributes
);
clearOverlay( targetClientId );
}

await saveEntityRecord(
Expand Down Expand Up @@ -780,19 +824,28 @@ export function useSuggestionsProvider() {
*/
const rejectSuggestion = useCallback(
async ( { commentId, clientId, payload } ) => {
// Clear the live block's suggestion marker for structural
// rejects. Attribute-set suggestions don't carry a marker, so
// nothing to clear.
// Reject behavior depends on the structural op type:
// - block-remove: drop the marker (block stays).
// - block-insert-after: dispatch removeBlock to undo the
// suggested insertion. The marker on the live block goes
// away with the block itself.
// - attribute-set (no structural op): no live-block change.
const structuralOp = findStructuralOp( payload?.operations );
if ( structuralOp && clientId ) {
const clearAttrs = clearSuggestionMarkerAttributes(
selectBlockAttributes( clientId )
);
if ( clearAttrs ) {
if ( structuralOp.type === 'block-insert-after' ) {
requestInterceptorBypass( clientId );
updateBlockAttributes( clientId, clearAttrs );
clearOverlay( clientId );
removeBlock( clientId );
} else {
const clearAttrs = clearSuggestionMarkerAttributes(
selectBlockAttributes( clientId )
);
if ( clearAttrs ) {
requestInterceptorBypass( clientId );
updateBlockAttributes( clientId, clearAttrs );
}
clearOverlay( clientId );
}
clearOverlay( clientId );
}

try {
Expand Down Expand Up @@ -824,6 +877,7 @@ export function useSuggestionsProvider() {
createNotice,
selectBlockAttributes,
updateBlockAttributes,
removeBlock,
requestInterceptorBypass,
clearOverlay,
]
Expand Down
130 changes: 107 additions & 23 deletions packages/editor/src/components/suggestion-mode/store-interceptor.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@
* keyboard handlers, external integrations) that wouldn't trigger a
* `useSelect`-based watcher.
*
* New blocks (no snapshot entry) are tracked but not intercepted — inserting
* a block in Suggest mode is currently a real edit, not a suggestion.
* Structural changes (#77434) flow through the same subscribe loop:
* - Removed blocks → re-inserted from the previous-tick snapshot at their
* 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.
*
* In every case the live block carries the marker and a corresponding
* structural op is written to the overlay so auto-save persists it.
*/
/**
* WordPress dependencies
Expand Down Expand Up @@ -542,9 +549,63 @@ export default function SuggestionStoreInterceptor() {
}

if ( previous === undefined ) {
// New block (inserted after Suggest activated). Track it
// but don't intercept.
// New block (inserted after Suggest mode activated):
// route through the apply-and-tag flow. The block stays
// in the live tree; auto-save persists a `block-insert-
// after` op against the previous-tick tree snapshot.
// Apply later just clears the marker (the block is
// already there); Reject runs `removeBlock` to undo.
//
// Skip descendants of another new block — a Group with
// nested children fires multiple new-block entries in a
// single tick, but only the top-level Group is the
// suggested insertion. The previous-tick tree is the
// reference because the active `snapshot` map is being
// built up in parents-first iteration order, so its
// presence wouldn't distinguish "pre-existing" from
// "already-processed-new-block" parents.
snapshot.set( clientId, current );
const parentClientId =
blockEditor.getBlockRootClientId?.( clientId ) || null;
const parentExisted =
parentClientId === null ||
tree.blocksByClientId.has( parentClientId );
if ( ! parentExisted ) {
continue;
}
const block = blockEditor.getBlock?.( clientId );
if ( ! block ) {
continue;
}
const siblingIds =
blockEditor.getBlockOrder?.(
parentClientId ?? undefined
) ?? [];
const indexInParent = siblingIds.indexOf( clientId );
const anchorClientId =
indexInParent > 0
? siblingIds[ indexInParent - 1 ]
: null;

isReverting = true;
try {
blockEditorDispatch.updateBlockAttributes( clientId, {
metadata: withSuggestionMarker( current?.metadata, {
type: 'pending-insert',
} ),
} );
} finally {
isReverting = false;
}

setStructuralOpRef.current?.( clientId, block.name, {
type: 'block-insert-after',
clientId,
blockName: block.name,
anchorClientId,
parentClientId,
block,
} );
continue;
}

Expand Down Expand Up @@ -634,24 +695,31 @@ export default function SuggestionStoreInterceptor() {
const removedIds = [];
for ( const clientId of tree.blocksByClientId.keys() ) {
if ( ! live.has( clientId ) ) {
// "Apply landing": when another client accepts a
// `pending-remove` suggestion, the resulting
// `removeBlock` arrives here through sync —
// typically batched with the marker-clearing
// `updateBlockAttributes` into a single block-
// editor update, which fires this subscriber
// once. The previous-tick tree snapshot still
// carries the pending marker, so we can recognize
// the disappearance as the apply landing rather
// than a fresh user delete. Adopt the removal:
// drop the snapshot entry and skip the re-insert
// + re-tag path. Without this, the apply bounces
// back through sync and undoes the removal on the
// accepting client a moment after they clicked.
// "Apply / reject landing": when another client
// accepts a `pending-remove` — or rejects a
// `pending-insert`, which also dispatches
// `removeBlock` to undo the insert — the
// resulting `removeBlock` arrives here through
// sync, typically batched with the marker-
// clearing `updateBlockAttributes` into a
// single block-editor update, which fires
// this subscriber once. The previous-tick
// tree snapshot still carries the pending
// marker, so we can recognize the
// disappearance as the suggestion landing
// rather than a fresh user delete. Adopt the
// removal: drop the snapshot entry and skip
// the re-insert + re-tag path. Without this,
// the apply / reject bounces back through
// sync and undoes the change on the accepting
// client a moment after they clicked.
const trackedMarker =
tree.blocksByClientId.get( clientId )?.attributes
?.metadata?.suggestion?.type;
if ( trackedMarker === 'pending-remove' ) {
if (
trackedMarker === 'pending-remove' ||
trackedMarker === 'pending-insert'
) {
snapshot.delete( clientId );
continue;
}
Expand Down Expand Up @@ -723,11 +791,27 @@ export default function SuggestionStoreInterceptor() {
} );
}

// Drop snapshot entries for any remaining (descendant)
// removed blocks — they came back as part of their parent's
// re-inserted subtree, but we still want a fresh re-seed.
// Re-seed the snapshot for every block that came back via
// the re-insert (top-level tops AND their descendants). The
// live attributes for tops include the pending-remove
// marker; `metadata.suggestion`'s membership in
// SYSTEM_METADATA_KEYS keeps the marker invisible to the
// next diff. Using `snapshot.delete` here would leave the
// re-inserted blocks looking new, so the next subscribe
// fire — triggered by a sync echo, an unrelated dispatch,
// or React batching draining a follow-up tick — would route
// them through the new-block branch and overwrite both the
// pending-remove marker and the persisted `block-remove`
// structural op with the `pending-insert` /
// `block-insert-after` pair.
for ( const clientId of removedIds ) {
snapshot.delete( clientId );
const liveAttrs =
blockEditor.getBlockAttributes?.( clientId );
if ( liveAttrs ) {
snapshot.set( clientId, liveAttrs );
} else {
snapshot.delete( clientId );
}
}
}

Expand Down
78 changes: 78 additions & 0 deletions packages/editor/src/components/suggestion-mode/test/provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,53 @@ describe( 'applyOperations', () => {
expect( result ).toEqual( attrs );
expect( result ).not.toBe( attrs );
} );

it( 'composes with clearSuggestionMarkerAttributes to produce the inserted-block apply payload', () => {
// When applying a block-insert-after suggestion the live block on
// the accepter's side is in its captured-at-insertion shape — the
// suggester's interceptor diverted any subsequent typing into the
// overlay rather than committing it. Apply must (1) materialize
// those overlay edits as attribute-set ops, AND (2) clear the
// pending-insert marker. Combining `applyOperations` with
// `clearSuggestionMarkerAttributes` yields the merged payload the
// provider hands to `updateBlockAttributes`.
const liveAttributes = {
content: '',
metadata: {
noteId: [ 42 ],
suggestion: { type: 'pending-insert' },
},
};
const operations = [
{
type: 'block-insert-after',
clientId: 'abc',
blockName: 'core/paragraph',
anchorClientId: null,
parentClientId: null,
block: { name: 'core/paragraph', attributes: { content: '' } },
},
{
type: 'attribute-set',
attribute: 'content',
before: '',
after: 'Hello world',
},
];

const withOpsApplied = applyOperations( liveAttributes, operations );
const markerCleared = clearSuggestionMarkerAttributes( withOpsApplied );
const finalAttributes = markerCleared
? { ...withOpsApplied, ...markerCleared }
: withOpsApplied;

// The typed content is committed to the live block...
expect( finalAttributes.content ).toBe( 'Hello world' );
// ...and the pending-insert marker is gone, while noteId
// (system metadata) is preserved.
expect( finalAttributes.metadata ).toEqual( { noteId: [ 42 ] } );
expect( finalAttributes.metadata.suggestion ).toBeUndefined();
} );
} );

describe( 'payloadByteLength', () => {
Expand Down Expand Up @@ -248,6 +295,37 @@ describe( 'hasAttributeConflict', () => {
hasAttributeConflict( { content: wrapperOther }, [ CONTENT_OP ] )
).toBe( true );
} );

it( 'returns false for a block-insert-after payload even when attribute-set ops appear divergent', () => {
// The overlay baseline for an inserted block is `{}` — every
// attribute-set op the auto-save loop persists carries
// `before: null` regardless of what the user typed. Comparing
// that against the live (already-typed-into) block on the
// accepting client must not fire the staleness prompt.
const operations = [
{
type: 'block-insert-after',
clientId: 'inserted',
blockName: 'core/paragraph',
anchorClientId: 'anchor',
parentClientId: null,
block: {
name: 'core/paragraph',
attributes: { content: 'Hi' },
innerBlocks: [],
},
},
{
type: 'attribute-set',
attribute: 'content',
before: null,
after: 'Hi',
},
];
expect( hasAttributeConflict( { content: 'Hi' }, operations ) ).toBe(
false
);
} );
} );

describe( 'parseSuggestionPayload', () => {
Expand Down
Loading
Loading