Skip to content

fix(desktop): make keyboard shortcuts work on Windows and Linux - #919

Open
ozymandiashh wants to merge 1 commit into
getagentseal:mainfrom
ozymandiashh:fix/918-windows-shortcuts
Open

fix(desktop): make keyboard shortcuts work on Windows and Linux#919
ozymandiashh wants to merge 1 commit into
getagentseal:mainfrom
ozymandiashh:fix/918-windows-shortcuts

Conversation

@ozymandiashh

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes [REQ]Add windows version short-cut key #918: keyboard shortcuts did nothing on Windows and Linux. The renderer's keydown handler required event.metaKey and explicitly rejected event.ctrlKey (app/renderer/App.tsx:443), so navigation (1-8), Settings (,) and Refresh (R) were dead everywhere except macOS, where metaKey is Cmd. On Windows and Linux metaKey is the Super key, which the OS shell takes.
  • Adds app/renderer/lib/platform.ts as the single source of truth: isModifierChord() accepts Cmd-without-Ctrl on darwin and Ctrl-without-Cmd elsewhere, and shortcutLabel() resolves every visible keycap at render time, so Windows shows Ctrl+1 where macOS shows ⌘1 (the reporter's screenshot shows the macOS glyphs on Windows 10).
  • altKey stays rejected on both platforms. This is load-bearing on Windows: AltGr on European layouts is delivered as Ctrl+Alt, so Ctrl+Alt+<digit> must not hijack a character the user is typing.

The macOS chord condition is unchanged. The old guard admitted metaKey && !altKey && !ctrlKey && !shiftKey; the new one admits exactly the same set on darwin.

The Electron application menu is deliberately untouched. It ships no reload/forceReload role and no CmdOrCtrl+R accelerator, which is what leaves Ctrl+R free for the renderer to handle on Windows — app/electron/main.test.ts already pins that invariant.

Also corrects the Settings navigation hint, which read 1-7 while the sidebar has eight numbered destinations (⌘8 → Plans has always worked).

Note on the report: the issue attributes the failure to running without administrator rights. That turned out to be a red herring — globalShortcut appears nowhere in app/electron, so nothing here needs elevation.

Testing

  • I have tested this locally against real data (not just unit tests)
  • npm test passes
  • npm run build succeeds

Unit tests cover both platforms for labels and for keydown dispatch, including the three negatives: Meta on win32, Ctrl on darwin, and the Ctrl+Alt (AltGr) shape.

Beyond the unit tests, I ran the actual renderer under vite with the preload bridge reporting each platform, and drove real keydown events against the mounted app:

platform: win32
  sidebar keycaps   Ctrl+1 Ctrl+2 Ctrl+3 Ctrl+4 Ctrl+5 Ctrl+6 Ctrl+7 Ctrl+8 Ctrl+,
  footer hint       Ctrl+1-8   Ctrl+,   Ctrl+R
  Ctrl+2         -> Sessions
  Ctrl+1         -> Overview
  Ctrl+,         -> Settings
  Ctrl+8         -> Plans
  Meta+3         -> no change   (correctly ignored)
  Ctrl+Alt+4     -> no change   (AltGr shape, correctly ignored)

platform: darwin
  sidebar keycaps   ⌘1 ⌘2 ⌘3 ⌘4 ⌘5 ⌘6 ⌘7 ⌘8 ⌘,
  footer hint       ⌘1-8   ⌘,   ⌘R
  Meta+2         -> Sessions
  Meta+,         -> Settings
  Ctrl+3         -> no change   (correctly ignored)
  Meta+Alt+4     -> no change   (correctly ignored)

Layout was checked too, since Ctrl+1 is a wider keycap than ⌘1 in a fixed 186px sidebar: all nine rows stay single-line, with the tightest row ("Pull requests") keeping about 10px of slack.

npm test in app/: 468 passed across 38 files. npm run build and tsc --noEmit are clean.

I do not have a Windows machine, so the verification above forces the platform the preload reports rather than running on real Windows hardware. Happy to have @thatsam-brainiac or another Windows user confirm on the packaged build.

Known limitation (deliberately out of scope)

Keyboard layouts where the digit row requires Shift (AZERTY and similar) still will not trigger the numbered shortcuts, because the handler matches on event.key. Fixing that means matching event.code and allowing Shift for digits, which is a behaviour change worth its own PR. Happy to follow up if you want it.

Closes getagentseal#918.

The renderer's keydown handler required event.metaKey and explicitly
rejected event.ctrlKey, so every shortcut was dead outside macOS: on
Windows and Linux metaKey is the Super key, which the OS shell takes.
Navigation (1-8), Settings (,) and Refresh (R) all did nothing. The
sidebar and footer hints also hardcoded the Cmd glyph, so a Windows
user was shown chords that could not fire.

Add app/renderer/lib/platform.ts as the single source of truth for
platform-aware shortcuts, reading the platform the preload already
exposes (window.codeburn.platform) with a user-agent fallback for the
non-Electron cases. isModifierChord accepts Cmd-without-Ctrl on darwin
and Ctrl-without-Cmd elsewhere; altKey stays rejected on both, because
AltGr on European Windows layouts arrives as Ctrl+Alt and must not
hijack a typed character. Every visible shortcut label now resolves
through shortcutLabel() at render time, so the sidebar shows Ctrl+1
where macOS shows the Cmd glyph.

The mac chord condition is unchanged: the old guard admitted
metaKey && !altKey && !ctrlKey && !shiftKey, and the new one admits
exactly the same set on darwin.

The Electron application menu is deliberately left alone. It ships no
reload/forceReload role and no CmdOrCtrl+R accelerator, which is what
leaves Ctrl+R free for the renderer to handle on Windows.

Also corrects the Settings navigation hint, which read 1-7 while the
sidebar has eight numbered destinations.

Tests cover both platforms for labels and dispatch, including the
negatives: Meta on win32, Ctrl on darwin, and the Ctrl+Alt AltGr shape.
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.

[REQ]Add windows version short-cut key

1 participant