Skip to content

fix: reach and operate the editor context popup by keyboard - #161

Merged
Azganoth merged 7 commits into
mainfrom
bug/context-popup-keyboard
Aug 2, 2026
Merged

fix: reach and operate the editor context popup by keyboard#161
Azganoth merged 7 commits into
mainfrom
bug/context-popup-keyboard

Conversation

@Azganoth

@Azganoth Azganoth commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

The context popup opened from Shift+F10 and the Menu key but focus stayed in the editor, Tab closed it and sent focus somewhere unrelated, and any arrow key collapsed the selection and closed it. It was also exposed as an unnamed role="dialog" whose fourteen controls would each have become a tab stop once focus could reach them.

  • A keyboard open takes focus. Shift+F10 and the Menu key are handled as editor keys rather than through the contextmenu event 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.
  • A pointer open does not. Right-click and mouse-selection opens leave focus in the editor, unchanged.
  • The popup is a labeled toolbar. 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 unnamed role="group" rows are gone.
  • Two keys the toolbar cannot leave to Radix. Vertical arrows move between the wrapped rows at the nearest available column, skipping a row with nothing available, because roving focus only walks controls in document order. Escape closes from inside, because the focused control's tooltip is the dismissable layer Radix offers Escape to first.
  • Closing returns focus to the editor with its selection intact, on every path that was holding focus.
  • A scroll no longer closes the popup while focus is inside it, and still does while focus is in the editor.

src/components/ui/Toolbar.tsx is new: a data-slot wrapper over @radix-ui/react-toolbar, which the installed radix-ui package already provides. shadcn/ui ships no toolbar to base it on, and ToggleGroup is 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.tsx covers 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, Enter on a command and through a submenu, Escape, Tab, focus entry per open source, focus return, and both scroll cases.
  • contextPopup.test.tsx covers both context-menu keys, an unmodified F10 left to the editor, the caret and selection anchors, the source surviving an anchor refresh, and the editor selection surviving focus leaving and returning.
  • The submenu Escape and outside-click tests were confirmed to fail against the unmodified component by stashing only EditorContextPopup.tsx. Both assert against a parent that really closes; a mock onClose leaves the popup mounted and makes any "still open" assertion pass regardless.
  • Coverage thresholds clear without change: statements 90.38%, branches 81.15%, functions 91.83%, lines 91.89%.

Manual, on Windows 11 through pnpm tauri dev:

  1. Shift+F10 and the Menu key open the popup and move focus into it, in a paragraph, a list, and a table alike.
  2. Right-click and mouse selection open it without moving focus.
  3. Every quick action, inline format, block format, and both submenus operate from the keyboard; arrows move between controls instead of collapsing the selection.
  4. Closing returns focus to the editor with the selection intact, and the commands act on that selection.

Not verified: Escape inside an open submenu and clicking outside while the popup holds focus, both covered by tests only; platforms other than Windows; a bundled build.

Notes

  • The issue expected removing the onCloseAutoFocus prevention to be enough, since with only a PopoverAnchor Radix's restore is a no-op. The no-op holds, but PopoverContentNonModal calls event.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.
  • The issue also expected the two open paths to be told apart inside the contextmenu handler. That requires reading a synthesized MouseEvent, which reports no button for a keyboard invocation; handling the keys directly needs no such inference.
  • ArrowDown on 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.
  • Escape is 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.
  • Tab and Shift+Tab close 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.
  • Out of scope and unchanged: the command menu bar (Menu bar recent items are ungrouped and fully disabled submenus open empty #119), which commands appear in the popup and their availability rules, and the popup's mouse behavior.

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 Azganoth added the Bug Something isn't working label Aug 2, 2026
@Azganoth Azganoth self-assigned this Aug 2, 2026
@Azganoth
Azganoth merged commit 8d44d20 into main Aug 2, 2026
2 checks passed
@Azganoth
Azganoth deleted the bug/context-popup-keyboard branch August 2, 2026 01:24
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editor context popup cannot be reached or operated by keyboard

1 participant