Skip to content

Commit 42d77e8

Browse files
Suggestions: Adopt synced apply of a pending-remove instead of re-tagging
When another client accepts a pending-remove suggestion, the marker-clear plus the removeBlock arrive on this client through sync, typically batched into a single block-editor update. The interceptor's removal-detection branch was treating the disappearance as a fresh user delete — re-inserting the block and tagging it pending-remove again, which then bounced back through sync and undid the apply on the accepting client a moment after they clicked. Recognize the apply landing by checking the previous-tick tree snapshot for `metadata.suggestion.type === 'pending-remove'` on the disappearing block. When the marker is still there, drop the snapshot entry and let the removal stand. The PRUNE_ORPHANS effect in overlay-context cleans up the matching overlay entry on the next render.
1 parent a84fb40 commit 42d77e8

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

packages/editor/src/components/suggestion-mode/store-interceptor.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,27 @@ export default function SuggestionStoreInterceptor() {
634634
const removedIds = [];
635635
for ( const clientId of tree.blocksByClientId.keys() ) {
636636
if ( ! live.has( clientId ) ) {
637+
// "Apply landing": when another client accepts a
638+
// `pending-remove` suggestion, the resulting
639+
// `removeBlock` arrives here through sync —
640+
// typically batched with the marker-clearing
641+
// `updateBlockAttributes` into a single block-
642+
// editor update, which fires this subscriber
643+
// once. The previous-tick tree snapshot still
644+
// carries the pending marker, so we can recognize
645+
// the disappearance as the apply landing rather
646+
// than a fresh user delete. Adopt the removal:
647+
// drop the snapshot entry and skip the re-insert
648+
// + re-tag path. Without this, the apply bounces
649+
// back through sync and undoes the removal on the
650+
// accepting client a moment after they clicked.
651+
const trackedMarker =
652+
tree.blocksByClientId.get( clientId )?.attributes
653+
?.metadata?.suggestion?.type;
654+
if ( trackedMarker === 'pending-remove' ) {
655+
snapshot.delete( clientId );
656+
continue;
657+
}
637658
removedIds.push( clientId );
638659
}
639660
}

packages/editor/src/components/suggestion-mode/test/store-interceptor.js

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,57 @@ describe( 'SuggestionStoreInterceptor (integration)', () => {
427427
);
428428
} );
429429

430+
it( 'adopts a removal that arrives bundled with its marker-clear (apply of pending-remove via sync)', async () => {
431+
// Regression: when another client clicks "Apply" on a
432+
// pending-remove suggestion, the resulting marker-clear and
433+
// removeBlock land here through sync as a single batched
434+
// block-editor update. Without the apply-landing check the
435+
// removal-detection branch would re-insert the block and tag
436+
// it as a fresh pending-remove — which then bounces back
437+
// through sync, undoing the apply on the accepting client a
438+
// moment after they clicked.
439+
const a = createBlock( TEST_BLOCK_NAME, { content: 'A' } );
440+
const b = createBlock( TEST_BLOCK_NAME, { content: 'B' } );
441+
const { registry, getOverlay } = setup( {
442+
initialBlocks: [ a, b ],
443+
} );
444+
445+
// First, this client creates the pending-remove suggestion
446+
// (just like the suggester would).
447+
await act( async () => {
448+
registry.dispatch( blockEditorStore ).removeBlock( b.clientId );
449+
} );
450+
await flushSubscribers();
451+
452+
expect(
453+
registry.select( blockEditorStore ).getBlockAttributes( b.clientId )
454+
?.metadata?.suggestion?.type
455+
).toBe( 'pending-remove' );
456+
457+
// Simulate the apply landing: marker-clear + removeBlock
458+
// batched into a single store update (matching how YJS
459+
// applies multi-op transactions on a peer client).
460+
await act( async () => {
461+
registry.batch( () => {
462+
registry
463+
.dispatch( blockEditorStore )
464+
.updateBlockAttributes( b.clientId, { metadata: {} } );
465+
registry.dispatch( blockEditorStore ).removeBlock( b.clientId );
466+
} );
467+
} );
468+
await flushSubscribers();
469+
470+
// The block must stay removed — the interceptor adopts the
471+
// apply landing rather than re-inserting and re-tagging.
472+
const liveBlocks = registry.select( blockEditorStore ).getBlocks();
473+
expect( liveBlocks ).toHaveLength( 1 );
474+
expect( liveBlocks[ 0 ].clientId ).toBe( a.clientId );
475+
476+
// The orphan overlay entry is cleaned up by the PRUNE_ORPHANS
477+
// effect once the block leaves the live tree.
478+
expect( getOverlay().entries[ b.clientId ] ).toBeUndefined();
479+
} );
480+
430481
it( 're-inserts only the top-level removed block when a parent and its child are removed together', async () => {
431482
// `removeBlock` on a parent removes the whole subtree atomically.
432483
// We must re-insert just the parent — its child rides along.

0 commit comments

Comments
 (0)