Toggle the sidebar with Ctrl+B on a layout that does not write Latin letters - #600
Conversation
…letters SidebarToggle's tooltip names Ctrl+B, or ⌘B on a Mac, and the sidebar's listener compared KeyboardEvent.key with "b". On a Russian layout the B key writes "и" and on Greek it writes "β", so the shortcut the tooltip names did nothing for anybody with one of those layouts selected. Shift+N had the same miss and was fixed with keyOf, which the paste shortcut on a Bot's screen now reads too. The sidebar reads it as well: the letter a layout writes when it writes one in ASCII, and the physical key when it writes another script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Good catch — this is a real one, and it's a third site: the sidebar has its own inline listener that never went through the hotkey registry, so #580 and #596 didn't cover it. Holding it briefly on a repo decision rather than anything wrong with the patch. Once #605 lands,
Worth considering (not a requirement): the combo could move into |
# Conflicts: # CHANGELOG.md
davidmckayv
left a comment
There was a problem hiding this comment.
Code-verified clean; CI green on this sha.
What this changes
SidebarToggle(app/src/components/layout/sidebar-toggle.tsx) names the sidebar's shortcut in its tooltip as Ctrl+B, or ⌘B on a Mac. Its comment says "An accelerator nobody is told about is not a feature." The listener that implements it is inSidebarProvider(app/src/components/ui/sidebar.tsx) and comparesevent.key === "b".keyis the character the layout writes. On a Russian layout the B key writesи, and on Greek it writesβ, so pressing the shortcut the tooltip names did nothing for anybody with one of those layouts selected.This is the same gap #580 closed for Shift+N and #596 closed for the paste shortcut on a Bot's screen. Both now read the key through
keyOfinapp/src/lib/hotkeys/hotkeys.ts, which returns the letter a layout writes when it writes one in ASCII (so Dvorak still goes by the letter) and the physical key fromcodeonly when the layout writes a character outside ASCII. The sidebar's listener now callskeyOftoo. That is one import and one comparison. The modifier check (metaKey || ctrlKey) and everything else in the listener are unchanged.Where it runs
keydownlistener compares differently.Boundary and audit
Changelog
CHANGELOG.mdunderUnreleased, worded like the Shift+N and paste lines.Proof
app/tests/sidebar-shortcut-layouts.test.tsxrenders aSidebarProvider, dispatcheskeydownevents onwindowthe way a browser reports them (keyfrom the layout,codefrom the physical key), and reads backuseSidebar().open:Ctrl+B and Cmd+B toggle the sidebar on a layout that writes another script: Ctrl withи/KeyBcloses it, and Cmd withβ/KeyBopens it again.a layout that writes Latin letters still goes by the letter, wherever its key is: on Dvorak,bonKeyNtoggles andxonKeyBdoes not. This passes onmaintoo.another letter, or the B key without its modifier, still does nothing:тonKeyNwith Ctrl, andиonKeyBwith no modifier. This passes onmaintoo.With
sidebar.tsxfrommain(only the test added):With the change: 3 pass.
bun test --coverageshows both changed lines insidebar.tsxexecuted.App suite (
bun test app, Windows): 892 pass, 29 fail on this branch. Onmainwith the new test file added, the same 29 fail plus the new test above. The 29 are this machine's environment failures (serve port, WebSocket, path separator, locale) and are identical on both.bun run typecheckinapp/is clean.app/src/components/uiis outside Biome'sfiles.includes, and the test file passesbiome check.On Linux too (
ubuntu-latest, Bun 1.3.14; a throwaway workflow on my fork, since fork PRs get no CI here):bun test appis 925 pass, 0 fail with all five of my open PRs merged together, andformat:check,lintandtypecheckare green.🤖 Generated with Claude Code