fix: reach and operate the editor context popup by keyboard - #161
Merged
Conversation
Shift+F10 and the Menu key are handled as editor keys rather than through the contextmenu event they would otherwise produce. That keeps the two open paths apart without having to read a synthesized MouseEvent, which reports no button for a keyboard invocation, and it suppresses the follow-up event so the pointer path cannot reopen the popup underneath the keyboard one. Radix cannot restore focus on close here. A non-modal popover focuses its trigger and then prevents the focus scope's own restore, and this popup has only an anchor, so its restore is a no-op that leaves focus on the body. The popup returns focus to the editor itself instead, and only when it was holding focus, so a close that follows a click elsewhere leaves that alone.
Covers only what the popup does today. The toolbar semantics the same issue asks for are not written down yet, since they are not implemented yet.
The popup was an unnamed dialog whose dozen buttons would each have been a tab stop once focus could reach them. It is a command toolbar, so it says so, and Radix's toolbar carries the roving tabindex and the horizontal traversal. role="toolbar" is repeated on the element because the popover's own role="dialog" arrives through asChild and would otherwise win. Two keys the toolbar cannot leave to Radix. Vertical arrows move between rows at the nearest available column, since roving focus only walks the controls in document order and the popup wraps into rows; a row with nothing available is skipped. Escape closes from inside, because a focused control shows its tooltip and that tooltip is the dismissable layer Radix offers Escape to first, which would otherwise cost a second press. ArrowDown on a submenu trigger keeps opening the submenu rather than leaving the row, which is what a menu button is expected to do.
Completes the documentation the previous docs commit deliberately left out until the toolbar existed.
The popup taking focus rests on two things the editor has to keep doing: a blur must not reach the selection sync and close the popup, and the selection must still be there when focus comes back.
Two paths the toolbar was taking over. A submenu renders in its own portal but stays a React child, so its keys reached the toolbar handler and its Escape closed the whole popup instead of the submenu. And an outside click now leaves focus where it landed: Radix defers that dismissal until after the click, so the popup was returning focus to the editor from a control the user had just clicked, rather than losing a race as expected. The submenu test that should have caught the first one passed a mock onClose, so the popup it asserted was still open could never have closed. Both cases now run against a parent that really closes, and both fail against the unfixed component.
Reference had picked up keyboard behavior that Specification owns, and both documents carried reasoning that belongs to the pull request. Comments that narrated the code, or repeated what the documents now state, are gone; what remains is one line each on the Radix behavior being worked around.
Azganoth
added a commit
that referenced
this pull request
Aug 2, 2026
) ## Summary Two guidance gaps that #161 walked into, both fixed where the guidance is read rather than where the symptom appeared. - **What each document owns.** Reference's description lists "shortcuts", so a surface's keyboard behavior reads as though it belongs there, and the context popup's arrow keys were written into both Reference and Specification before the split was settled. `docs/README.md` now states the distinction it already implied: Reference lists what exists, Specification states how it behaves, and keyboard behavior is Specification's even though Reference inventories the shortcuts. - **What a commit body and other durable prose leave out.** `CONTRIBUTING.md` bans verification evidence, commands run, and per-file summaries from commit bodies, but not the account of how the work unfolded, which is what #161's documentation commits carried: references to sibling commits and to what an earlier attempt got wrong. The new rule names that, ties it to the squash merge that discards the sequence, and goes after the cause — a commit that exists only because documentation was held back until its code landed will produce such a body, so the documentation should land with the change it describes. `AGENTS.md` gains the matching checkpoint under verification, since an agent writes with a whole session in context and the same habit reaches code comments and pull request text. It draws the line at recoverability: rationale a reader cannot reconstruct from the diff stays, the account of how it was reached goes. ## Related Issue Not applicable. Small, self-contained correction, per `CONTRIBUTING.md`. Refs #161, which is where both gaps surfaced. ## Verification `pnpm exec oxfmt --check AGENTS.md CONTRIBUTING.md docs/README.md` passes, and the pre-commit hook ran the same check on each commit. Documentation-only with no executable configuration touched, so no application suite, per the documentation-only rule in `CONTRIBUTING.md` and `AGENTS.md`. Nothing here is mechanically enforced, and that is deliberate: no linter separates a comment that explains a workaround from one that narrates the afternoon, and a gate that cannot make that call would either pass everything or block correct prose. ## Notes - The `Pull Requests` section gets no duplicate rule. The `AGENTS.md` line already covers pull request text, and a second enumeration invites matching the list rather than the principle, which is how the original rule was obeyed to the letter and missed anyway. - The document bullets at the top of `docs/README.md` are unchanged. Removing "shortcuts" from Reference's description would be wrong, since Reference does inventory them; the ambiguity is about behavior versus inventory, which is where the sentence was added.
8 tasks
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.
Summary
The context popup opened from
Shift+F10and theMenukey but focus stayed in the editor,Tabclosed it and sent focus somewhere unrelated, and any arrow key collapsed the selection and closed it. It was also exposed as an unnamedrole="dialog"whose fourteen controls would each have become a tab stop once focus could reach them.Shift+F10and theMenukey are handled as editor keys rather than through thecontextmenuevent they produce, which also suppresses that event so the pointer path cannot reopen the popup underneath the keyboard one. Requests carry the source they came from, and an open popup keeps it, so refreshing its anchor against a moved selection cannot downgrade a keyboard open to one that never took focus.role="toolbar"with an accessible name, and Radix's toolbar carries the roving tabindex and horizontal traversal, so the controls are one tab stop rather than fourteen. The unnamedrole="group"rows are gone.Escapecloses from inside, because the focused control's tooltip is the dismissable layer Radix offersEscapeto first.src/components/ui/Toolbar.tsxis new: adata-slotwrapper over@radix-ui/react-toolbar, which the installedradix-uipackage already provides. shadcn/ui ships no toolbar to base it on, andToggleGroupis the wrong semantics for command buttons.Related Issue
Closes #120
Verification
Automated (
pnpm check:frontend, Windows 11, 951 tests across 92 files, exit 0):EditorContextPopup.test.tsxcovers the toolbar role and name, the absence of a dialog and of unnamed groups, all fourteen controls outside the tab sequence, arrow traversal on both axes with column clamping and a skipped unavailable row, wrapping,Enteron a command and through a submenu,Escape,Tab, focus entry per open source, focus return, and both scroll cases.contextPopup.test.tsxcovers both context-menu keys, an unmodifiedF10left to the editor, the caret and selection anchors, the source surviving an anchor refresh, and the editor selection surviving focus leaving and returning.Escapeand outside-click tests were confirmed to fail against the unmodified component by stashing onlyEditorContextPopup.tsx. Both assert against a parent that really closes; a mockonCloseleaves the popup mounted and makes any "still open" assertion pass regardless.Manual, on Windows 11 through
pnpm tauri dev:Shift+F10and theMenukey open the popup and move focus into it, in a paragraph, a list, and a table alike.Not verified:
Escapeinside an open submenu and clicking outside while the popup holds focus, both covered by tests only; platforms other than Windows; a bundled build.Notes
onCloseAutoFocusprevention to be enough, since with only aPopoverAnchorRadix's restore is a no-op. The no-op holds, butPopoverContentNonModalcallsevent.preventDefault()immediately after it, which suppresses the focus scope's own restore, so removing the prevention drops focus to<body>. The popup returns focus itself instead, and only when it was holding it.contextmenuhandler. That requires reading a synthesizedMouseEvent, which reports no button for a keyboard invocation; handling the keys directly needs no such inference.ArrowDownon a submenu trigger opens the submenu rather than leaving the row, which is what a menu button should do. The consequence is that downward wrapping from the last row is unreachable, since both bottom rows are submenu triggers; upward wrapping works.Escapeis layered, closing an open submenu before the popup, matching the menu bar. The tooltip a focused control shows is the exception, skipped because it is a consequence of focus rather than something the user opened.TabandShift+Tabclose the popup and return focus to the editor rather than cycling inside it. The toolbar is one tab stop by construction and the arrows already wrap, so cycling would trap focus to no end.