Skip to content

Suggest mode: reveal a selected suggestion's marker, and stop unplaced note cards eating clicks - #81674

Merged
adamsilverstein merged 1 commit into
WordPress:suggest/inline-wiringfrom
adamsilverstein:fix/73411-f31-marker-reveal
Aug 23, 2026
Merged

adamsilverstein merged 1 commit into
WordPress:suggest/inline-wiringfrom
adamsilverstein:fix/73411-f31-marker-reveal

Conversation

@adamsilverstein

Copy link
Copy Markdown
Member

What

Fixes finding F-31 from the Suggest mode guided testing pass: https://github.com/adamsilverstein/gutenberg/blob/docs/suggest-mode-testing-2026-08-14/suggest-mode-testing/README.md

Part of the Suggest mode work in #73411. Based on suggest/inline-wiring (#80433).

The problem

F-31 recorded two things.

First: clicking a note card selects the right block, but the marker in the canvas gets no active treatment - its class stays the bare wp-suggestion - and nothing scrolls. Block selection is the wrong granularity here. A paragraph can hold several markers, so selecting the block cannot say which run the note is about, and on a long post the marker can be nowhere near the viewport.

Second: during the MU-09 run a floating note card sat on top of a suggestion's Accept button and swallowed the click, which had to be forced through.

The second half turned out to be reproducible, just not where the write-up guessed. It is not a z-index problem. A floating card is position: absolute and gets its top from the board, and the board resolves that from the anchor's measured rect - so there is a frame after mount where a card has no position. An absolutely positioned card with no top does not stay put: it falls back to its static position, which is the panel's origin, on top of whichever card legitimately sits at the top of the board. Being later in tree order, it wins the hit test and takes that card's clicks. Sampling every frame while the board mounts shows two cards landing on the exact same box (129-229 twice), and a second, longer overlap right after: calculateNotePositions reads heights[ id ] || 0, so the first sweep - run before the ResizeObserver has reported anything - treats every card as zero-height and drops them all on their anchors, 64px apart with 100px cards.

The fix

Revealing the marker. A new RevealSelectedSuggestion keys off the note the editor considers selected rather than off a click handler, so every route in behaves the same - clicking a card, arrowing through the sidebar, the List View's focus-note path. It injects a rule for mark.wp-suggestion[data-suggestion-id="N"] through useStyleOverride (the same mechanism NoteHighlightStyles uses for wp-note, and the reason the treatment survives a rich-text re-render that replaces the marker element), tinting and ringing that one marker in the suggester's colour, and scrolls the marker into view with block: 'nearest' so an already-visible marker is left alone. Only threads carrying an inline-suggestion op are revealed; structural and whole-attribute suggestions have nothing in content to point at.

The <mark> selector both features build was duplicated between Notes and Suggestions, so it moved down to inline-markers as getMarkerSelector( className, idAttribute, id ) - the format-agnostic primitive both already sit on. getNoteMarkerSelector now delegates to it and getSuggestionMarkerSelector is new.

The click theft. Two changes, both "don't act on a measurement you don't have yet":

  • FloatingContainer fades an unplaced card and takes it out of the hit test until the board reports a position. Deliberately opacity and pointer-events rather than visibility or display - the board's ResizeObserver still has to measure the card's height to work out where it goes, and the card still has to be focusable, since the pending new-note form is focused the moment it mounts.
  • calculateNotePositions holds every position back until the cards it has to place have been measured, so no card is ever placed from a zero-height sweep. Its anchor pick also falls back to the first thread that has a rect instead of abandoning the whole board when the selected thread's anchor has not been measured - that early return used to leave every card unpositioned, which is the pile-up in its worst form.

One correction to the write-up: the interception is not a z-order bug, and there is no fix in the stacking context. It is a placement race, and the cards that pile up are ones the board has not placed at all.

Screenshots

Selecting a note. Two additions in the same paragraph, with the second note selected in the sidebar. Before, both markers look identical and nothing says which one the note is about. After, " Twice." carries the ring and tint.

Before: the selected note's marker looks the same as the other marker in the paragraph After: the selected note's marker carries a tint and a ring in the author's colour

The floating board. The first frame each build paints, with the card measurements held back so the frame lasts long enough to photograph. Before, three cards land on top of each other. After, nothing is painted until the board knows where the cards go.

Before: three floating note cards stacked on top of each other at the top of the panel After: the same three cards laid out down the board with no overlap

Testing instructions

Enable the gutenberg-suggestion-mode experiment first.

Revealing the marker:

  1. Create a post with one paragraph and 30 or so filler paragraphs after it.
  2. Switch the editor intent to Suggesting.
  3. Put the caret at the end of the first paragraph and type some text. Wait for the add marker to pick up a data-suggestion-id.
  4. Open "All notes", then scroll the canvas to the bottom so the marker leaves the viewport.
  5. Click the note card.
  6. Before: nothing moves and the marker keeps its resting underline. After: the canvas scrolls the marker back into view and the marker takes a tinted background and a ring in the author's colour.
  7. For the granularity half, put two additions in the same paragraph and select each note in turn. Before: both markers look identical whichever note is selected. After: only the selected note's marker is ringed.

The floating cards:

  1. In a post with three paragraphs, in Suggesting intent, make an addition in each one.
  2. Close the "All notes" sidebar with its X so the floating board takes over.
  3. Watch the moment the cards appear, and click Accept on the topmost one.
  4. Before: the cards flash stacked on each other, and a click on the top card's Accept during that frame lands on the card covering it. After: the cards fade in already laid out, and the click lands where it was aimed.

Automated coverage

New e2e spec test/e2e/specs/editor/various/suggestion-mode-marker-reveal.spec.js:

  • "selecting a note scrolls to its marker and marks it active"
  • "floating note cards never stack on top of one another" - samples every frame while the board mounts and asserts no two cards that can take a click ever overlap

New unit tests:

  • collab-sidebar/test/floating-container.js - three cases covering the unplaced, placed, and non-floating states
  • collab-sidebar/test/utils.js - "anchors on a measured thread when the selected one has no rect" and "holds every position back until the cards have been measured"
  • inline-markers/test/marker-selector.js - the shared selector, including the escaping it inherited from getNoteMarkerSelector
  • suggestion-mode/test/reveal-selected-suggestion.js - buildSelectedSuggestionCss

Both e2e tests were verified red on the unmodified base branch and green with the fix, 10 runs each (--repeat-each=5): 10 failed before, 10 passed after. The three unit tests that pin new behaviour were each confirmed failing with their production hunk stashed.

No regressions: the five existing suggestion-mode* specs pass (64 tests), block-notes.spec.js passes (48 tests), and the packages/editor unit suite passes (1283 tests). An earlier draft of the FloatingContainer change used visibility: hidden and broke six block-notes tests that focus a card the moment it mounts, which is why it fades instead.

AI Use

Code and description both written with 🤖 Claude Code. I will review and test.

…ating clicks

Selecting a note in the sidebar selected its block and stopped there. The
marker itself kept its resting treatment and the canvas never moved, so on a
long post a reviewer had to hunt for the run the note was about, and on a
block holding several markers block selection could not point at one of them
at all. Key the reveal off the note the editor considers selected, so every
route in behaves the same: the marker takes a tint and a ring in the
suggester's colour and scrolls into view.

The same finding recorded a floating note card sitting on top of a
suggestion's Accept button and swallowing the click. A card the board has not
placed has no `top`, so it falls back to the panel's origin on top of whatever
legitimately sits there and, being later in tree order, wins the hit test.
Fade an unplaced card and take it out of the hit test until the board reports
a position, and hold the whole sweep back until the cards have been measured,
since sweeping with unknown heights drops every card on its anchor.
@github-actions github-actions Bot added the [Package] Editor /packages/editor label Aug 15, 2026
@github-actions

Copy link
Copy Markdown

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@adamsilverstein adamsilverstein added the [Type] Bug An existing feature does not function as intended label Aug 15, 2026
@adamsilverstein
adamsilverstein merged commit a40d2c8 into WordPress:suggest/inline-wiring Aug 23, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Editor /packages/editor [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant