Repository navigation
Suggest mode: reveal a selected suggestion's marker, and stop unplaced note cards eating clicks - #81674
Merged
adamsilverstein merged 1 commit intoAug 23, 2026
Conversation
…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.
|
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. |
adamsilverstein
merged commit Aug 23, 2026
a40d2c8
into
WordPress:suggest/inline-wiring
46 checks passed
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
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.
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: absoluteand gets itstopfrom 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 notopdoes 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-229twice), and a second, longer overlap right after:calculateNotePositionsreadsheights[ 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
RevealSelectedSuggestionkeys 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 formark.wp-suggestion[data-suggestion-id="N"]throughuseStyleOverride(the same mechanismNoteHighlightStylesuses forwp-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 withblock: 'nearest'so an already-visible marker is left alone. Only threads carrying aninline-suggestionop 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 toinline-markersasgetMarkerSelector( className, idAttribute, id )- the format-agnostic primitive both already sit on.getNoteMarkerSelectornow delegates to it andgetSuggestionMarkerSelectoris new.The click theft. Two changes, both "don't act on a measurement you don't have yet":
FloatingContainerfades an unplaced card and takes it out of the hit test until the board reports a position. Deliberatelyopacityandpointer-eventsrather thanvisibilityordisplay- 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.calculateNotePositionsholds 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.
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.
Testing instructions
Enable the
gutenberg-suggestion-modeexperiment first.Revealing the marker:
data-suggestion-id.The floating cards:
Automated coverage
New e2e spec
test/e2e/specs/editor/various/suggestion-mode-marker-reveal.spec.js:New unit tests:
collab-sidebar/test/floating-container.js- three cases covering the unplaced, placed, and non-floating statescollab-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 fromgetNoteMarkerSelectorsuggestion-mode/test/reveal-selected-suggestion.js-buildSelectedSuggestionCssBoth 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.jspasses (48 tests), and thepackages/editorunit suite passes (1283 tests). An earlier draft of theFloatingContainerchange usedvisibility: hiddenand broke sixblock-notestests 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.