Suggestions - show suggestions as inline previews#77869
Open
adamsilverstein wants to merge 20 commits into
Open
Suggestions - show suggestions as inline previews#77869adamsilverstein wants to merge 20 commits into
adamsilverstein wants to merge 20 commits into
Conversation
…and deletes Phase A of #77867. Registers two display-oriented format types that later phases will apply programmatically to the suggest-mode overlay so reviewers and suggesters see proposed deletions struck through and proposed additions highlighted inline within RichText, alongside the existing sidebar diff summary. - gutenberg/suggested-deletion (<del class="has-suggestion-deletion">) - gutenberg/suggested-addition (<ins class="has-suggestion-addition">) Both formats are non-interactive (no toolbar UI) and registered idempotently so the editor bootstrap and integration tests can both trigger registration safely. CSS uses --wp-admin-theme-color and a faint background so the marks read on light and dark themes. Phase B (input interception) and Phase C (apply/reject transforms) follow in subsequent PRs.
|
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. |
|
Size Change: +1.96 kB (+0.02%) Total Size: 7.92 MB 📦 View Changed
ℹ️ View Unchanged
|
adamsilverstein
changed the title
adamsilverstein
added
the
[Feature] Notes
adamsilverstein
added
the
[Status] In Progress
Phase A wired up the inline format types but no code applied them. This
adds the two helpers the overlay HOC needs to render proposed changes
inline:
- markContentDiff(baseline, proposed) returns marked HTML where deleted
runs are wrapped in <del class="has-suggestion-deletion"> and added
runs in <ins class="has-suggestion-addition">. Tokenization is the
existing word-level wordDiff so we share a definition of "what counts
as a change" with the sidebar diff.
- stripSuggestionMarks(marked) reverses the wrap so the overlay can
store the proposed value rather than the marked rendering — without
this, a re-edit through RichText would feed marked HTML back into the
overlay and the next render would double up the marks.
Eleven new unit tests cover the round-trip, the three golden-path
scenarios (text addition, text deletion, mid-string replacement), an
inline format change ("world" -> "<strong>world</strong>"), and the
null/undefined edge cases the overlay can pass when an attribute is
introduced for the first time.
Wires markContentDiff and stripSuggestionMarks into withSuggestionOverlay so RichText attributes like `content` render with the suggestion marks visible in the editor canvas, not just in the sidebar summary. - mergedAttributes now passes the proposed content through markContentDiff before handing it to BlockEdit, so deleted runs render with the red strikethrough and added runs render with the green underline + side- shadow defined in the existing CSS. - wrappedSetAttributes strips suggestion marks from incoming payloads so RichText round-tripping the marked HTML back through onChange doesn't pile additional marks onto the next render. - Marks are skipped while the block is selected so the user's caret is not fighting the value-prop reconciliation on every keystroke. They reappear as soon as focus moves elsewhere — a Docs-like end state that trades live feedback for a cursor that doesn't jump mid-word. - RICH_TEXT_ATTRIBUTE_KEYS is intentionally narrow (just `content`) so primitive attribute changes (align, level, fontSize) keep flowing through the existing block-level outline rather than leaking HTML into string slots. Seven new unit tests exercise the helpers directly; the existing HOC tests continue to pass because the marking is gated on a selected block and the test harness leaves block-editor unregistered (the useSelect fallback returns true, so marks are skipped).
Three new playwright tests exercise the golden-path scenarios the PR description promises: - A trailing text addition lands in the paragraph wrapped in <ins class="has-suggestion-addition"> after the block deselects, so reviewers see the new word with the addition color treatment. - A trailing text deletion (selected range backspaced) survives in the paragraph wrapped in <del class="has-suggestion-deletion"> so the reviewer can read what the suggester wants removed. - Bolding a word renders the suggestion as <ins class="has-suggestion-addition"><strong>word</strong></ins>, keeping both the new format AND the addition treatment, with the pre-bold version surfacing as a paired <del> so the reviewer can see what changed. Each test deselects the block via clearSelectedBlock() before asserting, because withSuggestionOverlay deliberately skips marking while a block is selected (avoiding RichText caret jumps during typing). The marks appear once focus moves elsewhere — Docs-like end state without the custom rendering pipeline.
Update the suggest-mode JSDoc and inline comments so reviewers can read the files top-down without cross-referencing the tracking issue. - inline-formats.js: rewrite the registration docblock to describe the overlay HOC consumer; update markContentDiff to drop the stale golden-path-of-this-PR phrasing in favor of the v1 scenarios in #77867. - index.js: barrel comment now points at the actual consumer rather than promising a future phase. - with-suggestion-overlay.js: add a top-of-file architecture note tracing data flow from BlockEdit through the overlay to markContentDiff and back; promote isStringLike's intent to JSDoc; document mergeOverlayAttributes explicitly; explain the capture-baseline-on-first-write pattern; surface the isSelected short-circuit at the call site. - test files: add header comments listing the contracts each suite covers (HOC pass-through, merge semantics, diff/strip round-trip). - e2e spec: add a header naming the three golden-path scenarios; rename the scenario tests to 'add | delete | style' so they map 1:1 to the PR description bullets. No behavior change.
This was referenced May 4, 2026
Member
Author
|
Plan to bring this PR in line with the canvas-treatment fixes that just landed on the structural-marker stack (#77979 and friends). Two independent improvements; both should land on this branch. 1. Relocate inline-format CSS so it reaches the canvas iframeProblem. This is the same bug the structural stack hit with
The class names describe a text visual state, so block-editor styling them does not violate package layering: it doesn't need to know how the format arrived, just how to render it. 2. Tint inline marks by suggester avatar colorProblem. Right now the deletion strikethrough is hard-coded red and the addition underline is hard-coded green-ish ( The pattern that landed in f7d03128: \$suggestion-author-color: var(--suggestion-author-color, #{\$suggestion-color});
.is-suggestion-pending-remove { color: \$suggestion-author-color; ... }The HOC writes Implementation sketch:
Order of operations
Happy to take this on as a follow-up commit on this branch once the relocation goes in — let me know if you'd like me to push the change directly. |
…ndle The .has-suggestion-deletion / .has-suggestion-addition rules target RichText runs that render inside the editor canvas iframe. Previously they lived in the editor-package stylesheet (wp-edit-blocks), which is not loaded into the iframe — only wp-block-editor-content (compiled from block-editor/src/content.scss) is. As a result the inline marks landed on the DOM but no rule matched, so deletions/additions rendered unstyled. Move the inline-format rules to a new partial under block-editor/src/components/block-list/content-suggestion.scss and include it from block-editor/src/content.scss. The class names describe a text visual state, so block-editor styling them does not violate package layering: the partial only knows how to render the marks, not how the format arrived. Sidebar/diff/header rules stay in the editor package since they target chrome surrounding the iframe rather than RichText runs within it. Add `--suggestion-author-color` as an opt-in CSS custom property on the inline rules with the prior red/green pair as the fallback, so the follow-up commit that pipes the suggester's avatar color through `markContentDiff` can light it up without re-touching this file.
Hardcoded red / theme-green make every suggester's marks look the same, so reviewers can't tell two authors apart in the canvas. Pipe an optional `authorColor` through `markContentDiff` and emit it as an inline `style="--suggestion-author-color: …"` on each <del> / <ins> run. The block-editor CSS partial already consumes the variable with the previous red/green pair as the fallback, so anonymous / pre-collab edits stay visually unchanged — single-suggester sessions see no diff on the wire. The author color rides on the wrapper element rather than on a parent (there is no block-level wrapper available — marks live inside RichText). `stripSuggestionMarks` already removes the wrapper on accept / round-trip, so the inline `style` attribute disappears with it; no schema impact. Add unit coverage for the new parameter (color propagation, null/ undefined fallthrough, and the strip round-trip) so future changes to the wrapper format don't silently drop the per-author tinting.
Resolve the current user id from `coreStore.getCurrentUser` and map it through `getAvatarBorderColor` (the same palette `editor-collab-sidebar` uses for live cursors). Forward the resulting hex through `applyDiffMarks` into `markContentDiff` so each <del>/<ins> emitted in the canvas carries `style="--suggestion-author-color: …"`. The block-editor CSS partial already consumes the variable with the red/green pair as the fallback, so anonymous edits keep the existing default. Folded into the existing `useSelect` so the HOC stays at one extra store-subscription per block in suggest mode. `authorColor` flows through `useMemo`'s dep list so the marked HTML re-renders the moment a suggester logs in (or out) without forcing the whole tree. Add a unit test for the propagation through `applyDiffMarks` so the contract is enforced at the helper boundary, independent of how the HOC resolves the color.
Member
Author
|
Pushed both items on this branch — single-file diffs each, ordered as proposed:
Coverage:
Foreign-edit author resolution (passing the suggester's id rather than the current user's once stored on the pending suggestion) and the multi-suggester e2e are mentioned in the plan but not in these commits — happy to follow up once the structural stack lands the foreign-suggester id on the pending-suggestion record so both surfaces share one source of truth. |
|
Flaky tests detected in 42565cf. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25456853520
|
This was referenced May 11, 2026
Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org> Co-authored-by: maxschmeling <maxschmeling@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
* Keep overlapping compaction updates to avoid YDoc divergence * Add backport file --------- Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org> Co-authored-by: maxschmeling <maxschmeling@git.wordpress.org>
The named re-export of `SUGGESTED_DELETION_FORMAT`, `SUGGESTED_ADDITION_FORMAT`, and `registerSuggestionFormats` already pulls the module in, evaluating its top-level format registration. The companion `import './inline-formats';` above it added nothing — and because the file is not listed in the editor package's `sideEffects` allowlist, esbuild emits an `[ignored-bare-import]` warning every dev build. Removing the bare import keeps the registration behavior unchanged (the named re-export still triggers it, as does `with-suggestion-overlay.js` consuming `markContentDiff`/`stripSuggestionMarks`) and clears the warning.
`wordDiff` tokenizes on word/whitespace boundaries, so a multi-word addition or deletion arrives as 2N-1 separate `insert`/`delete` segments. The previous `markContentDiff` wrapped each segment in its own `<ins>`/`<del>`, which combined with the per-element border CSS painted a pipe around every word — the result looked like a bracket grid rather than a single underlined addition. Accumulate consecutive same-type segments into one run and emit a single wrapper per run. Boundary CSS now only paints at the actual edges of each change. Updates two unit tests in `test/inline-formats.js` and two in `test/with-suggestion-overlay.js` that pinned the pre-coalesce per-segment shape.
The overlay HOC suppresses diff marks while the block is selected to keep RichText's value-prop reconciliation from disrupting the caret during continuous typing. But discrete edits — a range Delete, a paste, a single Backspace — also stayed unmarked until focus moved off the block, leaving suggesters with no immediate visual feedback that the suggestion had been captured. Replace the block-selected gate with a ~100 ms idle debounce on `overlayAttributes` change. A single overlay write followed by silence flips the gate within a frame, so marks render almost immediately for discrete actions while continuous typing keeps the timer reset and avoids the original caret-fight scenario. Implemented as `setIsOverlayIdle(false)` during render keyed on a ref change, so the new `false` value is consumed by the same render that observed the overlay change — no one-frame flash of marked output between the change and the timeout cleanup. Adds two unit tests: marks render after the idle window, and stay suppressed while overlay writes churn inside it.
The inline diff overlay lives in React state on `SuggestionOverlayProvider`,
seeded only by direct edits in Suggest intent. That left two gaps:
suggesters lost their pending marks on reload (auto-save persisted
the comment, but the overlay starts empty on mount), and reviewers
viewing the same post in any non-Suggest intent never saw the
inline strike-through/insertion at all — only the sidebar summary.
Add a `SuggestionOverlayHydrator` mounted alongside the existing
suggest-mode siblings (interceptor, autosave) inside the provider.
It re-uses the `useNoteThreads` query from the collab sidebar so
no extra REST traffic is needed, parses each unresolved comment's
`_wp_suggestion` payload via the existing `parseSuggestionPayload`,
turns each `attribute-set` op into a baseline/overlay pair, and
dispatches a new `SEED_FROM_COMMENT` reducer action.
The seeded entry is tagged with `hydratedFromCommentId` so the
HOC can tell a hydrator-sourced entry from a live edit. Two
behaviors follow from that:
- The outer `withSuggestionOverlay` wrap now activates whenever
`isSuggestMode || hasOverlayEntry`, so blocks with a hydrated
entry render with marks even outside Suggest intent.
- Inside `SuggestingBlockEdit`, `wrappedSetAttributes` falls
through to the real `setAttributes` when not in Suggest intent.
Reviewers' keystrokes therefore land on the real block instead
of being trapped in the suggester's overlay.
The hydrator skips entries that already have live overlay writes
(`Object.keys(overlayAttributes).length > 0` and not flagged as
hydrator-sourced) so a refresh of the comment list mid-typing
doesn't clobber the suggester's unsaved state.
Adds three unit tests covering the reducer, the syncedOpsKey
preservation, and the reviewer write-through scenario.
The hydrator re-runs whenever the comment entity record list updates. Without a value-level guard, re-seeding the same payload produced a new overlay state reference on every render and the effect looped, pegging the renderer. Short-circuit when both baseline and overlay attribute shallow-compare equal to the existing entry's, and add unit tests covering the seed paths.
In a real-time collaboration session, the hydrator seeds an overlay entry on every connected client from each persisted suggestion comment. Reviewers route their writes through to the real block, but the merge step still let the suggester's recorded `after` win on overlapping rich-text keys — so the reviewer's typing was masked in the canvas and the diff marks made it look as if their edits had become part of the suggester's proposal. Skip the merge for hydrated reviewer entries once the real content diverges from the suggester's recorded baseline; the existing accept-time conflict prompt covers the case where someone tries to accept a now-stale suggestion.
The `.has-suggestion-addition` rule applied a 1px box-shadow on the left and right of every inserted run, which rendered as visible vertical bars around suggested text. The underline plus the author color already carry the "inserted" treatment (matching the Google Docs suggesting-mode style noted in the surrounding comments), so the box-shadow was redundant and visually noisy.
Replace the 100 ms idle-debounce gate with a strict `isSelected`
guard so the overlay HOC never flips RichText's value prop between
the clean proposed value and the marked HTML while the caret is in
the block.
The idle gate ('Show inline diff marks once edits settle, not only
on blur') tried to surface marks within ~100 ms of a discrete edit
without leaving the block, but rendering `<del>`/`<ins>` wrappers
while the caret is live disrupts the cursor on the next keystroke.
A reproducible failure: typing 'page' after backspacing 'post.'
yields '.egappost' in the suggestion instead of 'page' — each typed
character lands at the boundary of a format element, gets stripped
into the deletion or prepended to the addition, and the result is
a reversed insertion mixed with surviving baseline text.
Marks reappear as soon as focus moves off the block. Reviewers (who
are not editing the suggester's block) continue to see marks on
hydrated entries without selecting anything.
Two unit tests that exercised the idle gate are replaced with one
that pins the new behavior: while `isBlockSelected` is true, the
rendered content stays unmarked. The hydrated-reviewer and
suggester-divergence tests now register the block in the editor
store so `isBlockSelected` returns false (no block selected), which
is the production state for those scenarios.
The hydrated-reviewer scenarios need `isBlockSelected` to return false (no block is selected) so the HOC's selection gate lets the diff marks render. The renderWithProviders helper previously registered only the notices and editor stores; this adds an opt-in `blocks` parameter that registers the block-editor + preferences stores and seeds the live block tree. The orphan-prune effect runs against the live tree on every overlay mutation, so we only register the block-editor store when a test asks for it — synthetic clientIds with no matching block would otherwise be pruned the moment a baseline is captured.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Similar to Google Docs, suggestion mode in Gutenberg now shows suggestions as inline previews:
This screenshot shows a suggested attribute change (bold), addition and deletion:
Closes #77867.
Scope: phases A + B + C in one PR
The tracking issue originally split this work into four phases (A — register format types, B — edit-time interception, C — apply/reject transforms + schema v2 migration, D — edge cases). This PR folds A, B, and C into a single change because the chosen architecture makes B and C structurally smaller than the issue anticipated:
gutenberg/suggested-deletion/-additionformats and ship CSSinline-formats.js,style.scss<del>/<ins>at edit time, persist a marked RichText valuecontenton every render and feeds marked HTML toBlockEditapplyMarkedValue/rejectMarkedValuetransform path; introduce schema v2 with arich-text-markedoperation; migrate v1 payloads on readattribute-setschema and apply/reject path keep working unchanged — no migration, no new op typeWhy the pivot
Edit-time interception (the original Phase B plan) requires hooking key handlers into every RichText that participates in suggest mode and then teaching the auto-save pipeline how to round-trip a marked value. Render-time diff achieves the same visible behavior by deriving the diff every render —
markContentDiff(baseline, proposed)is small, deterministic, and never touches the persistence path. The result:_wp_suggestionmeta) is byte-identical to phase-5 trunk; no schema migration to ship or reason about.attribute-setoperations they always did — no "strip runs with format" helper, no two-pass clean-up.! isBlockSelected), which sidesteps RichText cursor-jumping that would otherwise come from value-prop reconciliation on every keystroke. Docs takes the same end-state route via a custom rendering pipeline; this PR matches the visible result without that pipeline.The tradeoff is that we don't do live-mark-while-typing, and that whole-token format changes surface as a paired delete/insert rather than an in-place format swap. Both are acceptable for v1.
What lands here
packages/editor/src/components/suggestion-mode/inline-formats.jsgutenberg/suggested-deletion→<del class="has-suggestion-deletion">gutenberg/suggested-addition→<ins class="has-suggestion-addition">registerSuggestionFormats()(idempotent), called once on module import so editor bootstrap and integration tests get the formats without a separate init step.interactive: falseand have noeditUI so they never appear in the block toolbar — the formats are applied programmatically by the overlay HOC, not by users clicking a toolbar button.markContentDiff(baseline, proposed)— produces marked HTML where deleted runs are wrapped in<del class="has-suggestion-deletion">and added runs in<ins class="has-suggestion-addition">. Tokenization delegates to the existingwordDiffso the canvas marks share a definition of "what counts as a change" with the sidebar diff summary.stripSuggestionMarks(marked)— reverses the wrap so RichText round-tripping the marked HTML throughsetAttributesdoesn't double up the marks on the next render.packages/editor/src/components/suggestion-mode/with-suggestion-overlay.jsmergedAttributesnow passescontentthroughmarkContentDiffbefore handing it toBlockEdit, so deletions render with the strikethrough and additions render with the underline + side-shadow.wrappedSetAttributesstrips suggestion marks from incoming payloads before storing them in the overlay, so the overlay stays "proposed clean" rather than "proposed marked".! isBlockSelected( clientId )so RichText's value-prop reconciliation doesn't fight the user's caret on every keystroke. Marks reappear as soon as focus moves elsewhere.RICH_TEXT_ATTRIBUTE_KEYSis intentionally narrow (contentonly) so primitive attributes (align,level,fontSize) keep flowing through the existing block-level outline rather than leaking HTML into string slots.style.scss--wp-admin-theme-colorwith sensible fallbacks.applyDiffMarks,stripMarksFromIncoming).What does NOT land here (Phase D follow-up)
<del>runs in real time.<del>run, multi-block changes, partial-tag overlap.content(button text, list items, etc.).Stack base
This branch sits on top of #77407 (
add-suggestion-mode). Merge order is #75147 → #77403–#77407 → this PR → Phase D.Test plan
Unit tests now cover:
gutenberg/suggested-deletionregisters as a non-interactive<del>with the right classgutenberg/suggested-additionregisters as a non-interactive<ins>with the right classgutenberg/suggested-deletionserializes to<del class="has-suggestion-deletion">…</del>gutenberg/suggested-additionserializes to<ins class="has-suggestion-addition">…</ins>markContentDiffhandles identical inputs, pure additions, pure deletions, mid-string replacements, inline-format additions, and null/undefined coercionstripSuggestionMarksis a no-op when no marks present, removes deletion runs, unwraps addition runs, round-trips, and passes null/undefined throughapplyDiffMarksno-ops on missing baseline, no-ops on unchanged content, marks changed content, and skips primitive attributesstripMarksFromIncomingshort-circuits unchanged payloads and strips marks from contentE2E tests cover the three golden-path scenarios:
<ins class="has-suggestion-addition"><del class="has-suggestion-deletion">Cmd/Ctrl+Bon a selected word → after deselect, the bolded run renders as<ins class="has-suggestion-addition"><strong>word</strong></ins>AND its pre-bold version surfaces as a paired<del>All 109 suggestion-mode unit tests pass; all 6 suggestion-mode e2e tests pass locally.
Linked