Repository navigation
Suggest mode 7/9: inline live wiring - #80433
adamsilverstein wants to merge 153 commits into
Conversation
Wires the inline suggestion engine into the live editor: the addition, deletion, and format keyboards capture typing / delete / cut / paste / format toggles on beforeinput and turn them into inline markers; the content reconciler covers the onChange seams (IME, autocorrect, drag, multi-line paste); the overlay HOC hands format and content edits off to those paths before the overlay runs; author colors tint markers per suggester; annotations decorate them on the canvas; and the note garbage collector prunes orphaned suggestion notes. Restores the inline apply/reject branches in the provider, mounts the singletons in the editor provider, adds the server-side inline marker strip, the architecture doc, and the full e2e suites (golden path, persistence, review, undo, overlay invariants).
|
Size Change: +14.6 kB (+0.18%) Total Size: 7.95 MB 📦 View Changed
|
Input events target the editing host. With the editableRoot block support (native cross-block selection, #79105) that host is the writing-flow wrapper whenever the selected block has editable siblings, so event.target no longer identifies the block's rich text. The suggest-mode keyboard interceptors then never engaged and typing fell through to the per-keystroke reconciler path, which drops every keystroke after the first (each plan re-validates against a content snapshot the previous write already moved) - a typed addition in any multi-paragraph document lost all but its first character. Resolve the affected element from the event's target range (or the live selection for clipboard events, which expose no target ranges) instead of event.target, in both isEventTargetSelectedRichText and readEventRange. Also harden the tinting e2e: assert the typed run lands inside the marker (an attribute-only assertion missed the text loss) and assert each marker resolves its author's exact palette color rather than merely differing, so a user-id collision on the 7-color palette cannot fail the test.
|
The Fixed in 1b71c34 by resolving the affected element from the event's target range (or the live selection for clipboard events) instead of All 64 suggestion-mode e2e tests pass locally. Cherry-picked to the combined testing branch (#78994) to keep it byte-equal with the stack tip. |
|
Flaky tests detected in 71e6c55. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/33117806892 refuses the drop and uploads nothing in
|
|
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. |
Trunk switched @wordpress/dependency-group to 'never' mode, so the import group headers in these files now fail lint.
Trunk replaced the appender button with a ghost block exposing role=document, so the button locator no longer resolved and the test timed out.
The provider mixed the payload schema, attribute diffing and conflict detection, op finders and the structural reject planning with the persistence and notice code around them, so none of it could be read or tested on its own. Move the pure functions into suggestion-mode/operations/ and have the structural apply and reject return a plan of block-editor steps the provider dispatches, so the list indent/outdent rebuild no longer needs a registry to reason about. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
The in-flight, resolved-this-session and withdrawn-anchor sets lived at module scope, so two editor instances on one page shared them and a decision in one could stop the other's note collector. Move them into decision-state.ts behind a WeakMap keyed by the data registry: every hook inside one editor still sees the same sets, and the module no longer holds state tests have to reset between cases. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Every write to the note comment (create, payload update, trash, decision status) was an inline saveEntityRecord call in the provider, next to the block-tree and notice code, so the storage shape leaked through the whole file. Gather them in suggestion-store.ts behind a small typed SuggestionStore, which is the one module a different backend would reimplement. The size cap moves with the writes and is raised as an error the callers already report. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
With the pure logic, the persistence and the bookkeeping moved out, what was left in provider.ts was two unrelated flows sharing one hook: the suggester's create/update/delete and the reviewer's apply/reject. Give each its own hook (use-suggestion-submission.ts, use-suggestion- decisions.ts) and keep useSuggestionsProvider as the composition with its return shape unchanged, so auto-save, the keyboards and the sidebar keep working untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Attribute suggestions need a durable home in post content like structural and inline ones already have. The marker gains a pending-attributes type and an after field any marker can carry, with the merge, sanitize and clear rules in one module so the HOC, interceptor and decisions agree on them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
The overlay entries map was the only place attribute proposals lived, and the structural and inline paths had already moved their records into content. Drop the entries store and keep what is genuinely per-session coordination: bypass tokens, handler slots, the write queue, deferred insertions, undo adoption, structural capture records and the title slot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
The HOC diverted edits into React memory, so a proposal vanished on reload and the author could not see it in Editing intent. Write it into the block's metadata.suggestion instead, as a persistent change the undo stack owns, and merge it for rendering in every intent so reviewers see what is proposed. Every other consumer is pointed at the session module here so the module graph resolves; their behavior moves off the overlay in the next commits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Direct dispatches (block switcher, multi-select) used to land in the overlay entries map. Write them to metadata.suggestion.after like the HOC does, and record structural ops on the session instead of the retired entries store. The marker write runs after the revert so a drift that touched metadata cannot clobber it, and a metadata clear landing on a block that never had metadata reads as no proposal rather than an empty one. One fewer ref read in the interceptor, so its eslint suppression count drops by one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
With proposals in content the marker is the record. Derive each marked block's operations from the marker and the live tree, keep only session bookkeeping in the hook, and write the note id back onto the marker so a reload resolves it without a second note. A marker another author wrote is left to that author, so a synced proposal never opens a note in our name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
…rkers An attribute note is now anchored to the block marker's proposal, so the collector trashes it when the proposal leaves content and the undo guard no longer needs to withdraw it by hand: Ctrl+Z pops the marker write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
…this post A marker's commentId and metadata.noteId are read from post content, which any author of the post can edit, so an id found there is only a hint. Auto-save now updates or trashes a note on that hint only when core-data shows it as a pending note on the current post; otherwise it opens a fresh note and leaves the named one alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Apply lands the proposed values and drops the proposal in one update, and puts both back if the decision fails to save; reject drops only the proposal. Inline and structural decisions leave a co-resident proposal where it is, and a rejected move keeps the attribute proposal that rode on it as its own pending-attributes marker. SuggestionOperation now lives in operations/payload.ts; the session only re-exports it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Give the post title its own session slot, label a pending attribute change in List View, and mount the session provider in place of the overlay one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
A decision on a structural note resolves the attribute ops it carried, so clearing the marker drops a ride-along proposal instead of orphaning it as a note-less pending-attributes marker. A proposal that returns after undo or a revert is a new suggestion (auto-save forgets the block's note id and fingerprint when its marker leaves), and redo restores an attribute note like an inline one. An attribute-only note follows the block into a structural retype so one note keeps both ops, and the undo guard keeps the older attribute proposal when it withdraws the structural one. Edit intent declines a write to a proposed attribute rather than landing it behind the canvas. Store-level proposals stamp the history sequence like HOC writes. A copied proposal is folded into a new insertion. Auto-save waits for an unresolved hinted note instead of opening a second one, skips a reloaded note that already holds the same ops, and only acts on notes the current user authored. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
# Conflicts: # packages/editor/src/components/collab-sidebar/hooks.js # packages/editor/src/components/collab-sidebar/note-thread.jsx # packages/editor/src/components/collab-sidebar/test/utils.jsdom.test.js # packages/editor/src/components/collab-sidebar/utils.js
# Conflicts: # packages/editor/src/components/suggestion-mode/provider.ts
Trunk folded the floating notes sidebar into the canvas margin, dropped the SIDEBARS list, and now expects calculateNotePositions to place the measured cards while a neighbour is still unmeasured. Compare the active area to ALL_NOTES_SIDEBAR directly, and skip an unmeasured card in the sweep instead of holding the whole board back - it still sits out the hit test until the next sweep places it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
…the overview Attribute suggestions now live on the block's metadata.suggestion marker instead of an in-memory overlay, and the provider sits behind a SuggestionStore with submission and decision hooks. Both landed in #80433, so the overview's tables, diagrams and open findings described a model the code no longer has. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USLfaxhwoVRBZ7QFP5XoeU
The block type was registered inside one test, so under CI's shuffled test order a later test could create the block first. An unregistered name makes createBlock fall back to core/missing, which is not registered in this harness either, and the fallback recursed until the stack overflowed. Register in beforeAll and unregister in afterAll. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
Indenting a list item clones the source list for the new nested list, so inside a suggested list the clone carried the list's pending-insert marker and note link. The note then had two anchors: its summary moved to the nested list, the parent insertion lost its note, and a second decision appeared that could not resolve (#73411). A block adopted without a note of its own, inside a pending insertion or as a list indent's carrier, now drops the inherited suggestion state.
The test registers the real core-data store, and the reconciler reads getCurrentUser(), whose resolver calls api-fetch against a jsdom with no server. On a shuffled CI shard the failed request rejected after the test ended and Vitest failed the run on the unhandled rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTRGFetxgiXPJC9a91DMTL
The test registers the real core-data store, and auto-save reads getCurrentUser(), whose resolver calls api-fetch against a jsdom with no server. A failed request can reject after its test ends, which fails a shuffled CI shard on the unhandled rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTRGFetxgiXPJC9a91DMTL
Part of #73411
What's in this PR
Wires the inline suggestion engine into the live editor: the addition,
deletion, and format keyboards capture typing / delete / cut / paste /
format toggles on beforeinput and turn them into inline markers; the
content reconciler covers the onChange seams (IME, autocorrect, drag,
multi-line paste); the overlay HOC hands format and content edits off to
those paths before the overlay runs; author colors tint markers per
suggester; annotations decorate them on the canvas; and the note garbage
collector prunes orphaned suggestion notes, sparing any note somebody has
replied to (fixes #81958, folded
in from #81997). Restores the inline apply/reject
branches in the provider, mounts the singletons in the editor provider, and
adds the server-side inline marker strip.
The architecture document moved out to #82047 and the end-to-end suites to
#82048, so this PR is production code and its unit tests only.
Screenshot
With the inline layer wired up, all three marker types render live in the canvas and each opens a note summarizing the proposal:
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.