Repository navigation
Suggest mode 3/9: block-level capture (attribute and structural) - #80429
adamsilverstein wants to merge 71 commits into
Conversation
Adds the store-level interceptor that catches block mutations dispatched straight to the block-editor store (the block-switcher variation picker, programmatic writes), routing attribute drift into the overlay and reverting the live block; captures structural edits (insert/remove/move) by tagging the live block with a metadata.suggestion marker and recording the op; and the overlay HOC that renders the pending-suggestion visual treatment (the green attribute bracket and the strikethrough/dim/move-ghost overlays). The revert guard and undo guard keep the interceptor's own writes off the undo stack and out of its own subscribe loop, and the server hides un-accepted structural insertions at render time. Inline text and formatting markers are layered on top of this HOC in a later step.
|
Size Change: +7.4 kB (+0.09%) Total Size: 7.93 MB 📦 View Changed
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in ec6ef6b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/33450977039 user-customized templates cannot be overridden by plugins in
|
Trunk switched @wordpress/dependency-group to 'never' mode, so the import group headers in these files now fail lint.
SuggestingBlockEdit and the block class-name HOC, which runs for every block in every intent, read the whole overlay context, so each overlay write re-rendered every block. Both now subscribe to their own entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
…nged The interceptor subscribes to every store, so selection, notice and entity changes each ran four or five full-tree walks. Any attribute or structure change replaces the getBlocks() tree, so a fire that leaves it unchanged has nothing to capture. Passes still run while an interceptor bypass is pending, since the walk is what consumes a no-op bypass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
An edit inside a block with controlled inner blocks (a synced pattern, a template part) leaves the root getBlocks() tree unchanged, so the interceptor's unchanged-tree fast path could skip it and let the edit through uncaptured. getBlockTreeVersion() also watches every controlled subtree. The per-block pending-insert check now answers from a set built once per tree version, and only in Suggest intent. Walking each block's parents on every store update was the largest suggest-mode cost in a typing trace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
| $suggestion-deletion-color: #cc1818; | ||
| $suggestion-addition-color: #007017; |
There was a problem hiding this comment.
For almost all of the hard-coded values in this file (colors, radiuses, typography), can we use appropriate design system tokens instead?
There was a problem hiding this comment.
Good call, done in 9939f8c.
Claude handled the token sweep, notes below:
Every color, radius, spacing and type value in
content-suggestion.scssnow comes from a--wpds-*token. The tokens are available inside the canvas iframe sincewp-themeis the first dependency ofwp-edit-blocks. The tone defaults are the strong success and error surface strokes, text sizes map toxsandmd, the label weight moved from 500 to theemphasistoken (600) since 500 is not in the scale, and stroke widths use thesmandxsborder tokens.Two things stayed as they were on purpose: the bracket corner geometry keeps
$grid-unit-*because no token role describes it (the tokens doc asks not to force-fit a slot), and the twoopacityvalues have no token family.
| // fail for TEXT. Mixing 40% black brings every palette entry above the 4.5:1 | ||
| // text threshold on light backgrounds. Use these wherever the author color |
There was a problem hiding this comment.
What guarantees do we have that this shows on a light background such that the contrast expectations are upheld? (e.g. themed editor backgrounds)
There was a problem hiding this comment.
Good question, let me see how we have approached this elsewhere.
There was a problem hiding this comment.
Short answer: there wasn't one, and the old "text-safe" mix made things worse on a dark canvas. Fixed in 9939f8c.
Claude dug into this, ran the numbers, and here is where it landed:
The previous variant mixed the author color 60% with black. That works on white but pulls the text toward the background on a dark theme, and it turned out not to clear AA on white either for the orange and cyan palette entries (3.7:1 and 4.1:1).
The text colors now mix the author color 45% with
currentColorinstead. In thecolorproperty that resolves to the inherited text color, so the tint is pulled toward whatever the theme already contrasts against its background: darker on light themes, lighter on dark ones, and per block, so a Cover with light text over a dark image gets a light tint. At that share every palette entry and both defaults clear 4.5:1 on white and on dark backgrounds down to #333. Worst cases:
Color on white on #000 on #333 Orange #FBBF24(weakest on light)5.45 16.5 9.95 Purple #6F42C1(weakest on dark)11.6 10.1 6.05 Default green #00803010.3 10.7 6.45 The move tab is the one place that cannot lean on the inherited color, since its label sits on the tab's own fill. It mixes toward the neutral foreground token and pairs it with the light "on strong" foreground, then inverts under
.is-dark-theme, the same body class the button appender, placeholders and captions use for themed backgrounds. That flag is canvas-level, so a dark Group inside a light theme is the one residual gap, shared with the rest of that chrome.Testing on the dark theme also exposed a pre-existing bug, visible in the before shot: the inline markers are
<mark>elements, which browsers paint black by default, and the nested annotation<mark>was painting the run black regardless of theme. Both now inherit the text color.Outlines, underlines and the ghost border keep the raw author color as before. Orange and cyan sit under 3:1 on white there, but that is the shared avatar palette from notes and live cursors, so better handled where the palette lives.
The screenshots are from the e2e env with emptytheme and the darktheme test theme. Does that cover what you had in mind?
There was a problem hiding this comment.
Yeah, visibility-wise it certainly looks a lot better in the dark example 👍 Mixing with the text color this way has a design effect in making it look a lot more desaturated; not entirely clear if we're okay with that. I wondered about the use of mixing at all, but I suppose we can't safely just use some static variation of red if the theme's background itself is red, where I assume the idea with using text color is relying on it having some contrast with the background? In an ideal world we could just use the error tone design tokens, but that would rely on the having a theme context for the content area based on the theme's background and foreground (text) colors, which I don't know that we can derive.
… styles Replace the hard-coded colors, radii and typography in the suggestion canvas treatment with wpds design tokens, and mix author colors with the inherited text color instead of black so text stays readable on dark themes and dark blocks. At a 45% share every avatar palette entry clears 4.5:1 on white and on dark backgrounds down to #333, which the previous 60% black mix did not for the orange and cyan entries. Also inherit the text color on the suggestion mark and the nested annotation mark: the browser default paints a mark black, which made inline markers invisible on a dark canvas, and give the move tab a fill that mixes toward the neutral tokens so its label contrasts with the fill on light and dark themes alike. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017e7htmwLnMYkUvjbBfkgvR
The test imported the block-editor store without the expected-error marker every sibling test carries for that untyped package, and registered a container block with no attributes, which the blocks package types as required. Both failed the type-check CI job. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnVknLuyvKfR9DsM4H3SrX
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved capture, structural-edit, undo, persistence, and accessibility issues can lose edits or misrepresent pending suggestions.
Review effort: Balanced
Findings: 6
Open (11)
Invalidate controller cache when membership changes · New Prevent baseline blocks from entering pending insertions · New Avoid restoring removals under pending insertions · New Only arm adoption tokens for existing history records · New Preserve multi-selection attribute updates in Suggest mode · New Evaluate setAttributes updaters against latest overlay attributes · New Keep text opaque while dimming decorative elements · New Adopt descendants added to pending insertions · New Include overlay suggestions in undo availability · New Persist stable origin references across block ID regeneration · New Preserve style replacement semantics for reset suggestions · New
What changed in this PR
Adds block-level suggestion capture to the experimental Suggest mode, building on its storage and provider layer.
Changes:
- Captures attribute and structural edits with revert and undo guards.
- Adds pending-state styling and move-origin ghosts.
- Hides pending insertions from rendered output and adds tests.
| File | Description |
|---|---|
| tools/eslint/suppressions.json | Adds ref-rule suppressions. |
| phpunit/tests/strip-pending-structural-suggestions-test.php | Tests structural rendering rules. |
| packages/editor/src/components/suggestion-mode/with-suggestion-overlay.tsx | Captures attributes and decorates blocks. |
| packages/editor/src/components/suggestion-mode/use-move-ghosts.tsx | Shares the move-ghost index. |
| packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.jsdom.test.tsx | Tests capture and decoration. |
| packages/editor/src/components/suggestion-mode/test/suggestion-undo-guard.jsdom.test.ts | Tests suggestion undo ordering. |
| packages/editor/src/components/suggestion-mode/test/suggestion-move-ghost.jsdom.test.tsx | Tests ghost presentation. |
| packages/editor/src/components/suggestion-mode/test/move-ghost-index.ts | Tests ghost placement. |
| packages/editor/src/components/suggestion-mode/test/block-tree-version.jsdom.test.ts | Tests tree-change detection. |
| packages/editor/src/components/suggestion-mode/suggestion-undo-guard.ts | Integrates suggestion withdrawal with undo. |
| packages/editor/src/components/suggestion-mode/suggestion-move-ghost.tsx | Renders move-origin placeholders. |
| packages/editor/src/components/suggestion-mode/store-interceptor.ts | Captures direct block-store mutations. |
| packages/editor/src/components/suggestion-mode/move-ghost-index.ts | Resolves ghost anchors. |
| packages/editor/src/components/suggestion-mode/index.ts | Exports capture components. |
| packages/editor/src/components/suggestion-mode/block-tree-version.ts | Tracks document tree changes. |
| packages/editor/src/components/provider/index.jsx | Mounts experiment-gated capture components. |
| packages/editor/src/components/attribute-suggestions/test/revert-guard.ts | Tests revert-token matching. |
| packages/editor/src/components/attribute-suggestions/revert-guard.ts | Recognizes interceptor revert echoes. |
| packages/block-editor/src/content.scss | Loads suggestion canvas styles. |
| packages/block-editor/src/components/block-list/content-suggestion.scss | Styles pending suggestions and ghosts. |
| lib/compat/wordpress-7.1/block-suggestions.php | Suppresses pending insertions during rendering. |
| const changed = entry.controlledIds.some( | ||
| ( clientId, index ) => | ||
| blockEditor.getBlocks( clientId ) !== entry.trees[ index ] | ||
| ); |
| if ( ownMarker === 'pending-insert' || leftInsertion ) { | ||
| if ( | ||
| ownMarker !== 'pending-insert' && | ||
| isPartOfPendingInsertion( blockEditor, move.clientId ) | ||
| ) { |
| const trackedAttributes = | ||
| tree.blocksByClientId.get( clientId )?.attributes; | ||
| const trackedMarker = | ||
| trackedAttributes?.metadata?.suggestion?.type; | ||
| if ( | ||
| trackedMarker === 'pending-insert' || | ||
| isAppliedRemoval( coreSelect, trackedAttributes ) | ||
| ) { |
| armUndoRedoAdoption(); | ||
| return originalUndo( ...args ); | ||
| }; | ||
|
|
||
| coreActions.redo = ( ...args ) => { | ||
| armUndoRedoAdoption(); | ||
| return originalRedo( ...args ); |
| if ( ! entryExists ) { | ||
| captureBaseline( clientId, name, attributesRef.current ); | ||
| } | ||
| setOverlayAttributes( clientId, nextAttributes ); |
| position: relative; | ||
| margin: var(--wpds-dimension-gap-md) 0; | ||
| padding: var(--wpds-dimension-padding-md); | ||
| opacity: 0.6; |
| // built up in parents-first iteration order, so its | ||
| // presence wouldn't distinguish "pre-existing" from | ||
| // "already-processed-new-block" parents. | ||
| const block = blockEditor.getBlock?.( clientId ); |
| coreActions.undo = ( ...args ) => { | ||
| if ( withdrawNewestSuggestion() ) { | ||
| return Promise.resolve(); | ||
| } |
| fromAnchorClientId: marker.fromAnchorClientId ?? null, | ||
| fromParentClientId: marker.fromParentClientId ?? '', | ||
| fromIndex: marker.fromIndex ?? 0, |
A block that became controlled after the tree version was cached kept the root tree's identity, so the empty controlled list never noticed edits inside it and they escaped capture. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
The overlay wrapper diverged from the real setter in three ways: it merged `style` one level deep, so a reset kept the cleared fields on screen; it spread updater functions into an empty object, dropping Table cell edits; and it recorded a multi-selection change on the first block only. Replace `style` wholesale, evaluate updaters against the latest overlay-merged attributes, and hand multi-selection updates to the real setter so the interceptor captures one suggestion per block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
Undo or Redo with nothing to replay still armed an adoption token, and the next direct block mutation within its lifetime skipped capture and became real content. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
Three structural paths treated the inside of a pending insertion like published content: - Deleting a child of a suggested Group re-inserted it as a pending removal, although it has no published state to keep. - A block added inside a suggested Group got its own insertion marker and note, so accepting the Group could leave the child hidden. - Moving an existing block into a suggested Group recorded an ordinary move, so the server hid it with the Group and rejecting the Group deleted it. Let removals inside an insertion stand, adopt new descendants as part of the insertion, and move existing blocks back out with a snackbar explaining why. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
The ghost's 0.6 opacity also dimmed its label and excerpt, pulling the contrast-mixed author color below 4.5:1 (2.79:1 for the amber palette entry). Dim only the decorative block icon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
An attribute suggestion lives in the overlay and leaves no core-data history, so on a freshly loaded post the Undo button had no click handler and never reached the guard that withdraws the suggestion. The guard now records when it has a suggestion to withdraw, and the button honors it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4A1BxYdV83dMSQq1bUAFU
A block carrying a `pending-attributes` marker renders its live attributes on the front end; the proposal in `metadata.suggestion.after` never reaches readers. Pin that contract on the render_block filter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf



Part of #73411
What's in this PR
Adds the store-level interceptor that catches block mutations dispatched
straight to the block-editor store (the block-switcher variation picker,
programmatic writes), routing attribute drift into the overlay and reverting
the live block; captures structural edits (insert/remove/move) by tagging
the live block with a metadata.suggestion marker and recording the op; and
the overlay HOC that renders the pending-suggestion visual treatment (the
green attribute bracket and the strikethrough/dim/move-ghost overlays). The
revert guard and undo guard keep the interceptor's own writes off the undo
stack and out of its own subscribe loop, and the server hides un-accepted
structural insertions at render time. Inline text and formatting markers
are layered on top of this HOC in a later step.
Screenshot
Structural suggestions keep the block on the canvas and tag it instead of applying the change. A suggested move lands the block at its proposed position and leaves a ghost at the origin; a suggested removal keeps the block with a struck-through treatment:
Testing
This is one layer of the stack. To exercise the whole feature, #78994 bundles every layer into one branch and builds it in Playground:
👉 https://playground.wordpress.net/gutenberg.html?pr=78994
Enable Gutenberg > Experiments > Collaboration > Suggestion Mode, then follow the walkthrough in #73411, which also explains how to review the stack layer by layer.
Suggest mode stack
This rebuilds the manually-stacked Suggest mode work (#73411) as a GitHub Stack of 9 small, independently reviewable PRs, each building on the one below it:
Each follow up fix now sits in the layer that owns the code it changes, rather than piling onto the top of the stack. The whole feature can be exercised end-to-end via the combined testing branch #78994 (Playground). Behind the "Suggestion Mode" experiment (Gutenberg > Experiments).
AI Use
Code and description both written with 🤖 Claude Code. I will review and test.