Skip to content

Toggle the sidebar with Ctrl+B on a layout that does not write Latin letters - #600

Merged
davidmckayv merged 2 commits into
CopilotKit:mainfrom
kevin9327:sidebar-shortcut-non-latin
Sep 18, 2026
Merged

davidmckayv merged 2 commits into
CopilotKit:mainfrom
kevin9327:sidebar-shortcut-non-latin

Conversation

@kevin9327

@kevin9327 kevin9327 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

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 in SidebarProvider (app/src/components/ui/sidebar.tsx) and compares event.key === "b".

key is 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 keyOf in app/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 from code only when the layout writes a character outside ASCII. The sidebar's listener now calls keyOf too. That is one import and one comparison. The modifier check (metaKey || ctrlKey) and everything else in the listener are unchanged.

Where it runs

  • New state that outlives a request? None. This is a keyboard listener in the browser.
  • What happens on the second replica? Nothing server-side is involved.
  • Anything serialised? No.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No. The existing keydown listener compares differently.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. Not touched.
  • New refusals and new failures each write a row. None are added.
  • Nothing new is trusted from the client that the server can resolve itself.

Changelog

  • A line in CHANGELOG.md under Unreleased, worded like the Shift+N and paste lines.

Proof

app/tests/sidebar-shortcut-layouts.test.tsx renders a SidebarProvider, dispatches keydown events on window the way a browser reports them (key from the layout, code from the physical key), and reads back useSidebar().open:

  • Ctrl+B and Cmd+B toggle the sidebar on a layout that writes another script: Ctrl with и/KeyB closes it, and Cmd with β/KeyB opens it again.
  • a layout that writes Latin letters still goes by the letter, wherever its key is: on Dvorak, b on KeyN toggles and x on KeyB does not. This passes on main too.
  • another letter, or the B key without its modifier, still does nothing: т on KeyN with Ctrl, and и on KeyB with no modifier. This passes on main too.

With sidebar.tsx from main (only the test added):

(fail) Ctrl+B and Cmd+B toggle the sidebar on a layout that writes another script
error: expect(received).toBe(expected)
Expected: "closed"
Received: "open"
 2 pass
 1 fail

With the change: 3 pass. bun test --coverage shows both changed lines in sidebar.tsx executed.

App suite (bun test app, Windows): 892 pass, 29 fail on this branch. On main with 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 typecheck in app/ is clean. app/src/components/ui is outside Biome's files.includes, and the test file passes biome 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 app is 925 pass, 0 fail with all five of my open PRs merged together, and format:check, lint and typecheck are green.

🤖 Generated with Claude Code

…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>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@davidmckayv

Copy link
Copy Markdown
Contributor

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. app/src/components/ui was vendored — components.json, the shadcn CLI in the app's dependencies, and a Biome exclusion over the whole directory — so a shadcn add sidebar would have silently reverted your fix, and those files were never formatted or linted. We'd rather own them than work around them, so #605 removes shadcn as a vendor: the manifest and CLI are gone and the directory is checked like the rest of the app.

Once #605 lands, ui/sidebar.tsx is ours to edit and this merges as-is. Two small things when you rebase:

  1. keyOf lowercases, so Ctrl+Shift+B and Ctrl+B with CapsLock will now toggle the sidebar where the old === "b" did not. Probably fine — worth a line in the test either way.
  2. Now that the directory is linted, the file has to pass biome lint --error-on-warnings like everything else.

Worth considering (not a requirement): the combo could move into HOTKEYS in app/src/lib/hotkeys/hotkeys.ts and bind through useHotkey, which is what that file's doc comment promises — "every app-wide keyboard shortcut, in one place" — and would let formatHotkey drive the tooltip instead of the hand-rolled SHORTCUT_LABEL. Your call; the direct fix is fine too.

@davidmckayv davidmckayv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-verified clean; CI green on this sha.

@davidmckayv
davidmckayv merged commit 711bb2d into CopilotKit:main Sep 18, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants