diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 6d0f5733b41..c65877b64d9 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -186,6 +186,9 @@ jobs: - name: Install dependencies run: bun install --frozen --ignore-scripts + - name: Install Linux native build deps + run: sudo apt-get update && sudo apt-get install -y libx11-dev libxkbfile-dev + - name: Install desktop native dependencies working-directory: apps/desktop run: bun run install:deps diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 946f2bad604..8e2dc1ff84d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,9 @@ jobs: - name: Install dependencies run: bun install --frozen --ignore-scripts + - name: Install Linux native build deps + run: sudo apt-get update && sudo apt-get install -y libx11-dev libxkbfile-dev + - name: Install desktop native dependencies working-directory: apps/desktop run: bun run install:deps @@ -135,6 +138,9 @@ jobs: - name: Install dependencies run: bun install --frozen --ignore-scripts + - name: Install Linux native build deps + run: sudo apt-get update && sudo apt-get install -y libx11-dev libxkbfile-dev + - name: Install desktop native dependencies working-directory: apps/desktop run: bun run install:deps diff --git a/apps/desktop/docs/KEYBOARD_SYSTEM.md b/apps/desktop/docs/KEYBOARD_SYSTEM.md new file mode 100644 index 00000000000..e2c89af83a2 --- /dev/null +++ b/apps/desktop/docs/KEYBOARD_SYSTEM.md @@ -0,0 +1,194 @@ +# Keyboard Shortcut System + +Layout-aware hotkey dispatch + display for the desktop renderer. + +## What it does + +- 50+ shipped default shortcuts (`apps/desktop/src/renderer/hotkeys/registry.ts`). +- User-customizable via Settings → Keyboard, persisted in `localStorage`. +- Each binding can match by **physical key position** (`event.code`) or **printed character** (`event.key`) so users on Dvorak / AZERTY / QWERTZ get shortcuts that follow the labels on their keyboard. +- Display refreshes on the fly when the user switches input source (macOS menu-bar picker, Cmd+Space). +- Terminal forwarding: app hotkeys bubble through xterm; `Ctrl+C/D/Z/S/Q/\` are reserved for the PTY. + +## Public API + +Everything consumers need is re-exported from `renderer/hotkeys`: + +```ts +import { + // dispatch + useHotkey, // register a callback for a HotkeyId + // read + useBinding, getBinding, // current binding (string | v2 object) + getDispatchChord, // imperative event.code-form chord (use for synthesizing KeyboardEvents) + // display + useHotkeyDisplay, // formatted "⌘⇧P" for a HotkeyId + useFormatBinding, // formatted display for a binding shape (e.g. recording UI) + HotkeyLabel, // -rendering component + // recorder + useRecordHotkeys, // capture flow for the Settings page + // registry + HOTKEYS, HotkeyId, PLATFORM, +} from "renderer/hotkeys"; +``` + +Stay out of `stores/keyboardLayoutStore` and `utils/binding.ts` internals unless you're extending the system. + +## Architecture + +``` +┌─────────────────────────────────────────────────────────┐ +│ Electron Main process │ +│ │ +│ native-keymap (npm, Microsoft) │ +│ ├─ getKeyMap() → IKeyboardMapping │ +│ ├─ getCurrentKeyboardLayout() → IKeyboardLayoutInfo │ +│ └─ onDidChangeKeyboardLayout(cb) │ +│ └─ macOS: kTISNotifySelectedKeyboardInputSourceChanged │ +│ │ +│ apps/desktop/src/main/lib/keyboardLayout.ts │ +│ └─ EventEmitter wrapping native-keymap, lazy-init │ +│ │ +│ apps/desktop/src/lib/trpc/routers/keyboardLayout.ts │ +│ └─ get query + changes observable │ +└──────────────────┬──────────────────────────────────────┘ + │ tRPC subscription (observable per AGENTS.md) + ▼ +┌─────────────────────────────────────────────────────────┐ +│ Electron Renderer │ +│ │ +│ hotkeys/stores/keyboardLayoutStore.ts │ +│ └─ Zustand store: { map, layoutId } │ +│ Self-restarting on subscription error │ +│ │ +│ hotkeys/utils/binding.ts → bindingToDispatchChord() │ +│ └─ Single source of truth for translating logical │ +│ bindings to event.code form. Used by: │ +│ - useHotkey (react-hotkeys-hook registration) │ +│ - useHotkeyDisplay / useFormatBinding (rendering) │ +│ - useRecordHotkeys (cross-mode conflict detection) │ +│ - resolveHotkeyFromEvent (terminal forwarding) │ +│ │ +│ hotkeys/display.ts → formatHotkeyDisplay() │ +│ └─ Looks up event.code in layoutMap for printable │ +│ keys; falls back to KEY_DISPLAY (US-ANSI) for │ +│ special keys and when map is null │ +└─────────────────────────────────────────────────────────┘ +``` + +## Binding model + +Each binding is a `ShortcutBinding`: + +```ts +type ShortcutBinding = + | string // legacy / shipped default — implicitly physical + | { version: 2; mode: BindingMode; chord: string }; + +type BindingMode = "physical" | "logical" | "named"; +``` + +| Mode | Match against | Stored chord | Use | +|---|---|---|---| +| `physical` | `event.code` | scan-code-canonical (`"meta+p"` = physical KeyP) | Shipped registry defaults; preserves QWERTY muscle memory | +| `logical` | the produced character | the literal character (`"meta+p"` = the key labeled P) | Default for new user-recorded printable bindings; follows the printed letter across layouts | +| `named` | `event.code` (stable for named keys) | `"meta+enter"`, `"meta+arrowup"`, `"f5"` | Auto-applied to Enter/arrows/F-keys regardless of mode preference | + +**Storage compactness**: physical-mode bindings serialize to bare strings (matches legacy shape, keeps the registry terse). Logical and named modes serialize to the v2 object form. + +## Layout-aware translation + +The single function that bridges modes is `bindingToDispatchChord(binding, layoutMap)`. For every consumer that needs the chord react-hotkeys-hook actually matches against, route through this function: + +``` +binding.mode === "physical" → return chord unchanged +binding.mode === "named" → return chord unchanged (event.code is stable) +binding.mode === "logical" → translateLogicalChord(chord, layoutMap) + → find scan code whose unshifted glyph + matches the chord's letter, + return chord with key replaced. + Falls back to literal chord (US-correct) + when layoutMap is null. +``` + +Example: a logical `meta+z` binding on German QWERTZ resolves to `meta+y` (because German's KeyY position prints "z"), so react-hotkeys-hook fires when the user presses the key labeled Z — same letter, different physical position. + +## Recording flow + +`useRecordHotkeys` captures both `event.code` (codeChord) and `event.key` (keyChord) on each keystroke, plus a classification: + +- **fkey** / **named** → mode forced to `named` regardless of preference. +- **printable** → caller's `preferredMode` (default `"logical"`) decides; `+` falls back to physical to avoid colliding with the chord separator. + +The Settings page passes `preferredMode: "logical"`. Conflict detection compares dispatch chords (post-translation), so logical and physical bindings that collide on the current layout are flagged. + +## Cross-cutting guards + +| Concern | Where | Why | +|---|---|---| +| **AltGr** (Linux/Windows) | `eventToChord` and `useHotkey.shouldIgnoreEvent` | Chromium reports AltGr as ctrlKey+altKey — without suppression, AltGr-typed printables on non-US layouts (`AltGr+E = €` on German) would false-trigger any `ctrl+alt+e` binding. | +| **IME composition** (CJK / dead keys) | `eventToChord` and `useHotkey.shouldIgnoreEvent` | `event.isComposing` and Safari's `keyCode === 229` short-circuit matching. Modifier+letter chords bypass IME on macOS by OS design. | +| **Terminal-reserved chords** | `TERMINAL_RESERVED_CHORDS` set | `Ctrl+C/D/Z/S/Q/\` always go to PTY; recorder rejects them with an error. | + +## Migration + +The v1→v2 hotkey storage migration was shipped April 2026 and removed in commit `16f0da83e` (3 months later, after every active user had the `hotkey-overrides-migrated-v2` marker). If a user genuinely hasn't launched the app since April, they see default bindings instead of their v1 customizations; v1 overrides remain in main-process state via the `uiState.hotkeys.get` tRPC endpoint and could be recovered if anyone asks. + +## Decision history (brief) + +- **April 2026** — Initial refactor. Unified everything on `event.code` (recorder, dispatch, terminal forwarding). Preserved the bare-string storage shape. See `plans/done/20260412-keyboard-recorder-ctrl-binding-fix.md`. +- **April 27, 2026** — Layout audit and Phase 0–2 plan. Briefly tried `navigator.keyboard.getLayoutMap()` to avoid the native-keymap dep; switched back after discovering Chromium's `layoutchange` event doesn't fire for macOS input-source switches. native-keymap hooks `kTISNotifySelectedKeyboardInputSourceChanged` directly, which fires reliably. See `plans/done/20260427-keyboard-layout-plan.md`. +- **April 28, 2026** — Phase 1 (native-keymap) + Phase 2 (dual-mode bindings) shipped. v1 migration removed. + +## Known gaps / future work + +| Item | Status | +|---|---| +| **Menu accelerator sync** — `main/lib/menu.ts` hardcodes `CmdOrCtrl+R/,//Shift+Q`; they shadow user rebinds | Demand-driven. The single concrete user-visible gap. | +| **v1 terminal handler** uses catch-all `ctrl/meta` skip → starves TUIs of unbound chords like Ctrl+R | Tracked in `plans/20260409-tui-hotkey-forwarding.md`; v2 already correct. | +| **AltGr first-class binding token** | Reserved but never wired. AltGr is suppressed at match time, but a user can't *record* `AltGr+E` as their own chord. Drop or implement on demand. | +| **Numpad / Digit disambiguation** | Collapsed: `Numpad1` and `Digit1` both canonicalize to `"1"`. No current need for separate bindings. | +| **Shifted-layer display** | We use the unshifted glyph + ⇧ symbol convention (macOS). `native-keymap` exposes `withShift` / `withAltGr` data we don't read. | +| **Physical/logical mode toggle in Settings UI** | Backend supports both modes; UI defaults new printable recordings to logical with no opt-in to physical. Add a toggle if a user requests it. | +| **Layout-id telemetry** | `layoutId` is in the store but never reported. Cheap if product wants the data. | +| **Multi-stroke chords** (`Ctrl+K Ctrl+S`) | No demand. | +| **When-clauses / context system** | No demand; per-component `useHotkey` registration is sufficient. | + +## Out of scope + +- VSCode-style `KeybindingResolver` / context engine. +- `globalShortcut` (system-wide hotkeys). +- Per-extension keybinding contributions. +- Vendoring VSCode's static layout files (only if `native-keymap` ever proves insufficient). + +## Key files + +``` +apps/desktop/src/main/lib/keyboardLayout.ts # native-keymap wrapper +apps/desktop/src/lib/trpc/routers/keyboardLayout.ts # tRPC bridge +apps/desktop/src/renderer/hotkeys/ +├── registry.ts # shipped defaults +├── types.ts # ShortcutBinding, BindingMode +├── display.ts # formatHotkeyDisplay, glyphForCode +├── stores/ +│ ├── hotkeyOverridesStore.ts # localStorage user overrides +│ └── keyboardLayoutStore.ts # tRPC mirror with retry +├── hooks/ +│ ├── useBinding/ # binding + dispatch chord +│ ├── useHotkey/ # register a callback +│ ├── useHotkeyDisplay/ # formatted display +│ └── useRecordHotkeys/ # Settings recording flow +├── utils/ +│ ├── binding.ts # parse / serialize / translate +│ └── resolveHotkeyFromEvent.ts # canonicalization, terminal index +└── components/HotkeyLabel/ # -rendering component + +apps/desktop/src/main/lib/menu.ts # ⚠ hardcoded; see "Known gaps" +apps/desktop/src/renderer/lib/terminal/ # terminal forwarding integration +``` + +## References + +- VSCode keyboardLayoutMainService: https://github.com/microsoft/vscode/blob/main/src/vs/platform/keyboardLayout/electron-main/keyboardLayoutMainService.ts +- `native-keymap`: https://github.com/microsoft/node-native-keymap +- MDN `KeyboardEvent.code`: https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_code_values diff --git a/apps/desktop/package.json b/apps/desktop/package.json index b5a1a256482..a43bf426c88 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -188,6 +188,7 @@ "lucide-react": "^0.563.0", "mastracode": "0.15.0-alpha.3", "nanoid": "^5.1.6", + "native-keymap": "^3.3.9", "node-addon-api": "^7.1.0", "node-pty": "1.1.0", "os-locale": "^6.0.2", diff --git a/apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md b/apps/desktop/plans/done/20260412-keyboard-recorder-ctrl-binding-fix.md similarity index 100% rename from apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md rename to apps/desktop/plans/done/20260412-keyboard-recorder-ctrl-binding-fix.md diff --git a/apps/desktop/plans/done/20260427-keyboard-layout-plan.md b/apps/desktop/plans/done/20260427-keyboard-layout-plan.md new file mode 100644 index 00000000000..6ea6d24f0dd --- /dev/null +++ b/apps/desktop/plans/done/20260427-keyboard-layout-plan.md @@ -0,0 +1,478 @@ +# Keyboard Layout & Shortcut Plan + +**Date:** 2026-04-27 +**Branch:** `keyboard-shortcut-analysi` +**Scope:** `apps/desktop` keyboard shortcut matching, recording, display, migration, terminal forwarding, and Electron menu accelerators. +**Supersedes:** drafts of `20260427-keyboard-layout-options.md` and `20260427-keyboard-shortcut-system-audit.md`. +**Builds on:** `apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md` (April refactor — established `event.code` baseline) and `apps/desktop/plans/20260409-tui-hotkey-forwarding.md` (xterm forwarding). + +## Objective + +Make Superset's desktop keyboard handling correct on every keyboard layout — Dvorak, AZERTY, QWERTZ, Spanish, CJK IME — without regressing existing US-QWERTY users' muscle memory. + +## TL;DR + +Three phases: + +1. **Phase 0 — Correctness fixes (~1 day):** AltGr guard, IME composition guard, fail-closed dead-key sanitizer, `event.key` discipline in `line-edit-translations.ts`. No new deps. **Status: shipped.** +2. **Phase 1 — Layout service via `native-keymap` (~half day):** Microsoft's `native-keymap` in the Electron main process; expose live `{ layoutId, keymap }` to the renderer via tRPC observable; layout-aware display + reliable on-the-fly switching. Matching stays on `event.code`. +3. **Phase 2 — Versioned dual-mode bindings (~1–2 weeks):** Each binding declares `matchMode: "physical" | "logical"`. Existing bindings migrate as `physical` (preserves muscle memory); new user-recorded printable bindings default to `logical`. `react-hotkeys-hook`'s `useKey: true` option does the matching — no custom matcher needed. + +Phase 3 is demand-driven (menu accelerator sync, multi-stroke chords, when-clauses). + +## Decision history: browser API → native-keymap + +We initially planned Phase 1 with `native-keymap`, briefly switched to `navigator.keyboard.getLayoutMap()` after verifying it returned data in our packaged Electron 40 `file://` build, then switched back to `native-keymap` after empirical testing showed the browser API can't observe macOS input-source switches in real time. + +What we learned, in order: + +1. **Browser API returns data in Electron `file://`** (verified: 48-entry map with correct US glyphs). The codebase comment claiming otherwise was stale. +2. **Browser API only exposes the unshifted glyph per code** (`Map`); native-keymap exposes 4 layers plus dead-key flags. For *display* we don't need the extra layers. +3. **`layoutchange` doesn't fire for macOS input-source switches.** Empirically tested: switching ABC → German via the menu-bar input picker did not trigger the listener. The browser API treats input-source changes as IME events, not layout changes. +4. **VSCode solves this with `native-keymap`** — its mac implementation hooks `kTISNotifySelectedKeyboardInputSourceChanged` (Apple distributed notification) which fires for *every* input-source change. See `~/workplace/native-keymap/src/keyboard_mac.mm:182-189` and `~/workplace/vscode/src/vs/platform/keyboardLayout/electron-main/keyboardLayoutMainService.ts:48-60`. +5. **The desktop already ships native modules** (`better-sqlite3`, `node-pty`, `@parcel/watcher`) — `electron-rebuild` infrastructure is in place; native-keymap is one more dep in an existing pipeline. Native-binding risk is essentially zero. +6. **Hand-rolled alternatives create bug surface:** focus / visibilitychange / keydown-mismatch heuristics, US-fingerprint detection that false-positives on UK, two parallel detection paths (boot probe + runtime store). Adopting native-keymap lets us **delete** these instead of accumulating workarounds. + +Net of the swap: ~175 LOC + several heuristic edge cases removed; ~160 LOC + tests added; single source of truth; reliable runtime updates. + +## Design principle: physical vs logical key identity + +The central design question is *how a binding identifies a key*. Two valid identities: + +- **Physical** (`event.code`): the hardware position. The QWERTY-`P` slot is `KeyP` on every layout. +- **Logical** (`event.key` or layout-resolved character): the character the active layout produces. On Dvorak, the physical `KeyR` slot produces `p`. + +Picking only one globally creates bad behavior for some users. Today we are physical-only: + +- ✅ Stable: `Cmd+Shift+P` works regardless of the user's layout. +- ❌ Unintuitive: a Dvorak user pressing the key labeled `P` on their keyboard does *not* trigger Quick Open — they must press the key labeled `L` (the QWERTY-`P` position). +- ❌ UI lies on non-US layouts: the Settings page shows "⌘P" but on QWERTZ that's the wrong glyph for the physical slot. + +The fix is **per-binding match mode**, defaulting to physical for shipped defaults (preserves today's behavior for everyone) and logical for new user-recorded printable bindings (matches what users expect from their printed keys). + +## Current state (brief) + +Hotkeys live in `apps/desktop/src/renderer/hotkeys/`. The April 2026 refactor (PR #3391) unified the system on `event.code`: + +- Registry of 50+ per-platform defaults — `registry.ts:29-571` +- Canonical normalization — `utils/resolveHotkeyFromEvent.ts:42-89` +- Customization UI with conflict detection — `routes/_authenticated/settings/keyboard/page.tsx` +- Zustand + localStorage overrides — `stores/hotkeyOverridesStore.ts` +- v1→v2 migration with sanitizer — `migrate.ts`, `utils/sanitizeOverride.ts` +- Terminal forwarding via xterm `customKeyEventHandler` — `lib/terminal/terminal-runtime.ts` +- 62 unit tests across 4 files + +What's missing — addressed by this plan: + +| Gap | Severity | Phase | +|---|---|---| +| `MAC_US_DEAD_KEYS` rewrites apply when layout is unknown (fails open) | medium-high | 0 | +| AltGr (`event.getModifierState("AltGraph")`) treated as Ctrl+Alt | medium | 0 | +| No IME `event.isComposing` guard | low–medium | 0 | +| `line-edit-translations.ts` uses `event.key` without a discipline note | low (latent) | 0 | +| `navigator.keyboard.getLayoutMap()` is unreliable in Electron `file://`; layout never re-detected | medium-high | 1 | +| Display uses hardcoded US glyphs regardless of layout | medium | 1 | +| Bindings can only match physical position | medium | 2 | +| Hardcoded menu accelerators in `main/lib/menu.ts:13-100` shadow user bindings | medium | 3 | +| v1 terminal handler returns `false` for all `ctrl/meta` (starves TUIs) | medium | 3 (already in `20260409-tui-hotkey-forwarding.md`) | + +## Library decision: `native-keymap` + +- **Microsoft, MIT, v3.3.9 (Jan 2026), actively maintained**, ~125k weekly downloads. Same library VSCode ships. +- API: + ```ts + import { + getKeyMap, + getCurrentKeyboardLayout, + onDidChangeKeyboardLayout, + } from "native-keymap"; + ``` +- `getKeyMap()` returns `{ ScanCodeName: { value, withShift, withAltGr, withShiftAltGr, valueIsDeadKey, ... } }` for the current OS layout. +- Constraints: + - Native node-gyp addon → must run after `electron-rebuild`. Ships prebuilt binaries for major Electron ABIs. + - **Main-process only.** Wire to renderer via tRPC observable per `apps/desktop/AGENTS.md` (`trpc-electron` requires observables, not async generators). + - Linux dev/CI needs `libx11-dev` + `libxkbfile-dev`. + +Conceptual role: a supplementary lookup table answering *"on this user's current OS layout, what character is printed at physical position `KeyZ`?"* It does **not** replace `event.code` matching. Phase 1 uses it for display + reliable layout detection; Phase 2 uses its `value` field to resolve logical bindings. + +## Considered alternatives (rejected) + +- **`keyboard-layout` (Atom):** archived 2022, dead `nan` dep. +- **`mousetrap` / `hotkeys-js`:** match on deprecated `keyCode`; layout-unaware. +- **`tinykeys`:** no layout features beyond what `react-hotkeys-hook` already gives us. +- **Vendor full VSCode keybinding engine (`KeyCodeUtils`, `KeybindingResolver`):** long import tail; overkill. +- **Browser-only via `navigator.keyboard.getLayoutMap()`:** unreliable in Electron `file://`; current fallback behavior already proves the risk. +- **Vendor VSCode `keyboardLayouts/*.ts` static tables:** 70+ files, MIT, but custom layouts uncovered. Keep as future fallback if `native-keymap` ever proves insufficient. + +## Cross-cutting requirements (checklist for every phase) + +- **AltGr:** check `event.getModifierState("AltGraph")`; do not treat AltGr as Ctrl+Alt unless a binding explicitly opts in. +- **IME composition:** ignore matching during composition: `event.isComposing || event.keyCode === 229`. +- **Dead keys:** never rewrite a composed/dead-key glyph without layout certainty. Fail closed. +- **Numpad:** decide explicitly whether `Numpad1` and `Digit1` collapse (today: yes, via `normalizeToken` stripping `Digit`/`Numpad`). Phase 2 may differentiate for power users. +- **Special keys:** Enter, Escape, Backspace, Delete, F-keys, arrows, Home/End/PageUp/PageDown match by stable named-key (`event.code` is stable for these regardless of layout). +- **Conflict detection:** must run in the same mode used at runtime (physical-vs-physical, logical-vs-logical, mixed flagged). +- **Electron menus:** generate accelerators from effective bindings where representable; omit native accelerator otherwise rather than showing a wrong one. + +--- + +## Phase 0 — Correctness fixes + +**Goal:** close existing layout-related bugs without changing matching semantics or adding deps. +**Effort:** ~1 day. +**Owner:** anyone. + +### Implementation + +#### 0.1 AltGr guard in `eventToChord` + +`apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts:71-82` + +Add an `altGr` modifier flag derived from `event.getModifierState("AltGraph")`. When true on Linux/Windows, do not also set `ctrl` and `alt` from `event.ctrlKey`/`event.altKey` — Chromium reports both for AltGr. Bindings must opt in to AltGr explicitly via an `altgr+` token (none today; reserved for Phase 2). + +```ts +export function eventToChord(event: KeyboardEvent): string | null { + if (event.code === undefined) return null; + if (event.isComposing || event.keyCode === 229) return null; // see 0.2 + const key = normalizeToken(event.code); + if (isIgnorableKey(key)) return null; + + const altGr = event.getModifierState?.("AltGraph") === true; + const mods: string[] = []; + if (event.metaKey) mods.push("meta"); + if (event.ctrlKey && !altGr) mods.push("ctrl"); + if (event.altKey && !altGr) mods.push("alt"); + if (event.shiftKey) mods.push("shift"); + if (altGr) mods.push("altgr"); // dropped on match unless binding opts in + mods.sort(); + return [...mods, key].join("+"); +} +``` + +`canonicalizeChord` strips unknown `altgr` tokens for backward compatibility until Phase 2 adds first-class support. Net effect today: `Ctrl+Alt+E` typed via AltGr+E on a German layout no longer matches a US `Ctrl+Alt+E` binding. (Today it matches and surprises users.) + +#### 0.2 IME composition guard + +Same file, top of `eventToChord` (shown above). Returns `null` during composition so `react-hotkeys-hook` doesn't fire. + +#### 0.3 Fail-closed dead-key sanitizer + +`apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts:47-80, 98-121` +`apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts:21-43` +`apps/desktop/src/renderer/hotkeys/migrate.ts:36` + +Today `isUSCompatibleLayout()` returns `true` when `navigator.keyboard.getLayoutMap()` is unavailable (common in Electron `file://`) — so the US-Mac dead-key rewrite table runs on non-US Macs. + +Flip the fallback: + +```ts +// detectUSLayout.ts +export async function isUSCompatibleLayout(): Promise { + const keyboard = (navigator as Navigator & { keyboard?: Keyboard }).keyboard; + if (!keyboard?.getLayoutMap) return "unknown"; + try { /* probe ... */ } catch { return "unknown"; } +} +``` + +In `migrate.ts`, treat `"unknown"` as "do not apply US dead-key rewrites" — pass `assumeUSMacLayout: false`. Sanitizer drops the entry instead (existing behavior for invalid entries; user gets a console message and sees the binding empty, which is recoverable). Phase 1 deletes this file entirely. + +#### 0.4 `event.key` discipline in `line-edit-translations.ts` + +`apps/desktop/src/renderer/lib/terminal/line-edit-translations.ts:21-42` + +Add a top-of-file comment: + +```ts +// CONTRACT: only check event.key for stable named keys +// (Backspace, ArrowLeft/Right, Home, End, ...). Never event.key for printable +// characters — those vary by layout and break non-US users. Use event.code +// via resolveHotkeyFromEvent for printable keys. +``` + +No code change today; the comment exists to prevent regression. + +### Tests + +| Test | File | What it covers | +|---|---|---| +| AltGr does not double-match Ctrl+Alt bindings | `utils/resolveHotkeyFromEvent.test.ts` (new case) | Synthetic `KeyboardEvent` with `getModifierState("AltGraph") === true`, `ctrlKey: true`, `altKey: true` → chord excludes `ctrl` and `alt` | +| `event.isComposing` short-circuits matching | `utils/resolveHotkeyFromEvent.test.ts` (new case) | `eventToChord` returns `null` when `isComposing` is true | +| `event.keyCode === 229` short-circuits matching | same | covers Safari IME path | +| Sanitizer fails closed on unknown layout | `utils/overrideSanitizer.test.ts` (new case) | `assumeUSMacLayout: false` drops entries that match `MAC_US_DEAD_KEYS` keys | +| `isUSCompatibleLayout()` returns `"unknown"` when API absent | new file `utils/detectUSLayout.test.ts` | mock `navigator.keyboard = undefined` | + +### Manual QA + +- [ ] Linux QWERTZ: type `Ctrl+Alt+E` via AltGr+E in a textarea — does **not** trigger any app hotkey bound to `ctrl+alt+e`. +- [ ] macOS, US-EN keyboard active, Japanese input source available: switch to Japanese, type `かな` in a chat input — no hotkeys fire mid-composition. +- [ ] Fresh install on German Mac: v1→v2 migration drops legacy `meta+option+@` rather than rewriting it to a wrong physical slot. Console logs the drop count. + +### Acceptance + +- All Phase 0 tests pass; existing 62 tests still pass. +- Manual QA above all green. +- No new runtime deps added. + +--- + +## Phase 1 — Layout service via `native-keymap` + +**Goal:** non-US users see the printed glyph of the physical key bound; on-the-fly input-source switches reflect immediately. +**Effort:** ~half day. +**Depends on:** Phase 0. +**Outcome:** Settings page and tooltips show "⌘Y" instead of "⌘Z" on QWERTZ for the default `meta+z` (physical KeyZ) binding. Switching between ABC and German via the macOS menu-bar input picker updates the display without app reload. Matching semantics unchanged. + +### Architecture (mirrors VSCode) + +``` +┌──────────────────────────────────────────────┐ +│ Electron Main process │ +│ │ +│ native-keymap (npm) │ +│ ├─ getKeyMap() → IKeyboardMapping │ +│ ├─ getCurrentKeyboardLayout() → IKeyboardLayoutInfo │ +│ └─ onDidChangeKeyboardLayout(cb) │ +│ └─ macOS: kTISNotifySelectedKeyboardInputSourceChanged │ +│ │ +│ apps/desktop/src/main/lib/keyboardLayout.ts │ +│ └─ EventEmitter wrapping native-keymap │ +└──────────────────┬───────────────────────────┘ + │ tRPC observable (per AGENTS.md) + ▼ +┌──────────────────────────────────────────────┐ +│ Electron Renderer process │ +│ │ +│ apps/desktop/src/renderer/hotkeys/stores/ │ +│ keyboardLayoutStore.ts │ +│ └─ Zustand store, subscribed to tRPC │ +│ └─ flattens IKeyMapping → Map │ +│ │ +│ display.ts → formatHotkeyDisplay(chord, platform, layoutMap) │ +└──────────────────────────────────────────────┘ +``` + +### Implementation + +#### 1.1 Add the dependency + +```bash +cd apps/desktop && bun add native-keymap +``` + +`electron-rebuild` is already in the pipeline (we ship `better-sqlite3`, `node-pty`, `@parcel/watcher`). Linux dev/CI: ensure `libx11-dev` + `libxkbfile-dev` are installed. + +#### 1.2 Main-process wrapper + +``` +apps/desktop/src/main/lib/keyboardLayout.ts +``` + +Wraps `native-keymap`. Lazy-init on first call. Exposes `getSnapshot()` and `onChange(cb): unsubscribe` via a single EventEmitter. Mirrors VSCode's `keyboardLayoutMainService.ts`. + +#### 1.3 tRPC router + +``` +apps/desktop/src/lib/trpc/routers/keyboardLayout.ts +``` + +`get` query + `changes` subscription. Subscription **must** be an `observable` (`apps/desktop/AGENTS.md` — `trpc-electron` rejects async generators). Mount under root router in `routers/index.ts`. + +#### 1.4 Renderer store + +``` +apps/desktop/src/renderer/hotkeys/stores/keyboardLayoutStore.ts +``` + +Replace the `navigator.keyboard.getLayoutMap` probe + `layoutchange` listener with a tRPC subscription. Flatten `IKeyMapping` (4 fields per code) to `Map` so consumers (display.ts) don't change. No focus / visibilitychange listeners. No debug shims. + +#### 1.5 Deletions (sweep) + +- `apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts` — superseded by `keyboardLayoutStore.layoutId`. The 6-key fingerprint heuristic also false-positives on UK; the layout id is authoritative. +- `apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.test.ts` — goes with it. +- `migrate.ts` — read layout id from store instead of probing. +- The window debug shims (`__hotkeyLayoutMap`, `__refreshHotkeyLayout`). + +#### 1.6 No change + +- `display.ts` — already accepts `layoutMap: ReadonlyMap | null` (Phase 1 first attempt). The IPC just feeds the same shape. +- `useHotkey` / `useHotkeyDisplay` — unchanged. +- `formatHotkeyDisplay` consumers — unchanged. + +### Tests + +| Test | File | Covers | +|---|---|---| +| `glyphForCode("z", null)` → `null` | `display.test.ts` (extended) | falls back to KEY_DISPLAY when API unavailable | +| `glyphForCode("z", usMap)` → `"Z"` | same | US baseline | +| `glyphForCode("z", qwertzMap)` → `"Y"` | same | QWERTZ — physical Z slot prints Y | +| `glyphForCode("slash", azertyMap)` → glyph from map | same | non-letter punctuation | +| `glyphForCode("arrowup", anyMap)` → `null` | same | special keys bypass keymap | +| `formatHotkeyDisplay("meta+z", "mac", qwertzMap)` → `"⌘Y"` | same | end-to-end display swap | +| `formatHotkeyDisplay("meta+z", "mac", null)` matches today's output | same | regression guard | + +### Manual QA + +| Layout | Action | Expected | +|---|---|---| +| US QWERTY | Open Settings → Keyboard | Glyphs identical to today | +| German QWERTZ (or any non-US, e.g. UK) | Open Settings → Keyboard | Default `meta+z`-style binding shows the printed key glyph for the user's physical KeyZ slot | +| Switch layout mid-session | Watch Settings page | Display refreshes within ~1 second (native-keymap `onDidChangeKeyboardLayout`, hooked to the OS distributed notification) | + +### Acceptance + +- All tests pass. +- Manual QA on US (regression) + at least one non-US layout (verifies the swap). +- One new native dep (`native-keymap`); `electron-rebuild` already runs for `better-sqlite3` / `node-pty` so no new build-pipeline work. + +--- + +## Phase 2 — Versioned dual-mode bindings + +**Goal:** allow each binding to declare physical vs logical match. Existing bindings stay physical; new printable user bindings default to logical. +**Effort:** ~1–2 weeks. +**Depends on:** Phase 1 (layout service). +**Outcome:** Dvorak / AZERTY / QWERTZ users get shortcuts that follow their printed keys. + +### Binding shape + +```ts +// apps/desktop/src/renderer/hotkeys/types.ts +export type ShortcutBinding = + | string // legacy v1; treated as { version: 1, matchMode: "physical", chord: string } + | { + version: 2; + matchMode: "physical" | "logical" | "named"; + modifiers: { + meta?: boolean; + ctrl?: boolean; + alt?: boolean; + altGr?: boolean; + shift?: boolean; + }; + // exactly one of: + code?: string; // matchMode: "physical" — e.g. "KeyP" + key?: string; // matchMode: "logical" — e.g. "p" + named?: NamedKey; // matchMode: "named" — e.g. "Enter", "ArrowUp", "F5" + // for display only; recorder fills these in: + recordedAt?: { layoutId: string; glyph: string }; + }; +``` + +`named` covers Enter, Escape, Backspace, Delete, arrows, F-keys, Home/End/PageUp/PageDown — keys whose `event.code` *is* the right identity regardless of layout. + +### Match algorithm + +```ts +function matches(event: KeyboardEvent, b: ShortcutBinding, keymap: Keymap): boolean { + if (event.isComposing || event.keyCode === 229) return false; + if (typeof b === "string") return matchesPhysical(event, parseLegacy(b)); + if (!modifiersMatch(event, b.modifiers)) return false; + switch (b.matchMode) { + case "physical": return event.code === b.code; + case "named": return event.code === b.named; + case "logical": { + // Prefer event.key when it's a single printable char; fall back to keymap lookup. + const produced = isSinglePrintable(event.key) + ? event.key.toLowerCase() + : keymap[event.code]?.value?.toLowerCase(); + return produced === b.key?.toLowerCase(); + } + } +} +``` + +### Migration + +- v1 string bindings → `{ version: 2, matchMode: "physical", code: , modifiers: ... }`. Codified in `migrate.ts`. Default registry follows the same shape. +- All shipped defaults stay `matchMode: "physical"` — preserves muscle memory for everyone on day 1. +- Recorder writes `matchMode: "named"` for Enter/Escape/etc., `matchMode: "logical"` for printable-with-modifier (the common case for new bindings), `matchMode: "physical"` if the user toggles the advanced "by physical position" option. + +### Recorder & UI + +- `useRecordHotkeys` captures `{ code, key, modifiers, layoutId }` simultaneously. +- Settings → Keyboard adds an "advanced" disclosure on the recording row showing the captured `code`, `key`, and a toggle. Default is logical for printable, physical only when toggled. +- Conflict detection runs per match-mode pair: + - physical vs physical → string compare + - logical vs logical → string compare + - physical vs logical → resolve via current keymap; if they collide on this layout, warn but allow +- Display: + - logical / named → show the bound character / named key directly + - physical → show `glyphForCode(code, ..., keymap)` with a small "physical" badge + +### Tests + +#### Unit + +| Test | File | Covers | +|---|---|---| +| `matches` returns true for physical binding hitting same code on any layout | `utils/match.test.ts` (new) | regression of today's behavior | +| `matches` for logical binding on Dvorak: typing the key labeled `p` (physical KeyR) triggers `{ key: "p" }` | same | the new capability | +| `matches` for named binding: ArrowUp triggers regardless of any layout-shifted glyph | same | named is layout-immune | +| Conflict detector: physical `meta+p` and logical `meta+p` collide on US, not on Dvorak | `utils/conflicts.test.ts` (new) | per-mode honesty | +| Migration: legacy string `"meta+p"` becomes `{ matchMode: "physical", code: "KeyP" }` | `migrate.test.ts` (extended) | preserves muscle memory | +| Recorder default for `Cmd+Shift+P` → `matchMode: "logical"`, `key: "p"` | `useRecordHotkeys.test.ts` (extended) | new bindings follow printed keys | +| Recorder for `Cmd+ArrowUp` → `matchMode: "named"`, `named: "ArrowUp"` | same | named keys recognized | +| AltGr binding: `altGr+e` on a layout where AltGr+E is `€` matches that key | `utils/match.test.ts` | AltGr first-class | +| Logical binding with shift fallback via keymap when `event.key` is a dead-key replacement | same | fallback path | + +#### Integration + +| Test | Covers | +|---|---| +| Headless Electron: load app on simulated German layout, register a logical `meta+z` binding, dispatch synthetic event with `code: "KeyZ", key: "y"` → does not fire | logical mode honors layout | +| Same setup, dispatch `code: "KeyY", key: "z"` → fires | logical match | +| US layout: same logical `meta+z` fires for `code: "KeyZ", key: "z"` | US baseline | +| Storage roundtrip: save v2 binding → reload app → matches & displays correctly | persistence | + +#### Manual QA matrix (Phase 2) + +For each of {US QWERTY, Dvorak, German QWERTZ, French AZERTY, Spanish, Japanese with US fallback}: + +| Action | Expected | +|---|---| +| Default `Cmd+Shift+P` (Quick Open) — physical | Same physical key as today | +| Record new logical binding `Cmd+J` for some action | Pressing the key labeled `J` on the user's keyboard fires; UI shows "⌘J" | +| Record physical binding via advanced toggle on same chord | Pressing the QWERTY-`J` slot fires; UI shows the layout's glyph for that slot with "physical" badge | +| Conflict warning when both above coexist on the same layout | Visible warning; both still saved | +| Default `Cmd+ArrowUp` — named | Always fires regardless of layout | +| Type a sentence with the bound character in a text field | Hotkey does **not** fire (input focus + IME guard work) | +| Switch system layout mid-session | Display refreshes; physical bindings unchanged; logical bindings now match the new printed keys | + +### Acceptance + +- All v1 bindings continue to work for existing users (regression suite of the 62 April tests passes unchanged). +- Phase 2 tests pass. +- Manual QA matrix complete on at least US, German, AZERTY, and Dvorak. +- No localStorage corruption — old format reads cleanly, new format roundtrips. + +--- + +## Phase 3 — Menu sync & advanced (demand-driven) + +Each item is independently scoped; pick up only when warranted. + +| Item | Why | +|---|---| +| Generate `main/lib/menu.ts` accelerators from effective bindings | Today `Reload`, `Show Hotkeys`, `Open Settings`, `Quit` are hardcoded and silently shadow user rebinds | +| Migrate v1 terminal `Terminal/helpers.ts:677-679` to `resolveHotkeyFromEvent` | Already in `20260409-tui-hotkey-forwarding.md`; lets v1 TUIs receive unbound `Ctrl+R` etc. | +| Multi-stroke chords (`Ctrl+K Ctrl+S`) | Ship if a feature actually needs the keyspace | +| When-clauses / context system | Ship if global conflicts get hard to reason about | +| Vendor selected VSCode `keyboardLayouts/*.ts` files | Only if `native-keymap` proves insufficient on a real user's machine | + +## Out of scope + +- VSCode-style `KeybindingResolver` / context engine. +- Global system shortcuts via `globalShortcut` (we have no use case). +- Configurable shortcut scopes per pane / per mode (works today via per-component `useHotkey`). +- Per-extension keybinding contributions (no extension surface). + +## References + +- April 2026 baseline: `apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md` +- TUI forwarding: `apps/desktop/plans/20260409-tui-hotkey-forwarding.md` +- `native-keymap` on npm: https://www.npmjs.com/package/native-keymap +- `native-keymap` source: https://github.com/microsoft/node-native-keymap +- VSCode `KeyboardLayoutMainService` (architecture reference, not vendored): https://github.com/microsoft/vscode/blob/main/src/vs/platform/keyboardLayout/electron-main/keyboardLayoutMainService.ts +- VSCode `keyboardLayouts/` data files (vendor candidate, MIT): https://github.com/microsoft/vscode/tree/main/src/vs/workbench/services/keybinding/browser/keyboardLayouts +- MDN `KeyboardEvent.code`: https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_code_values +- MDN `KeyboardEvent.getModifierState`: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/getModifierState diff --git a/apps/desktop/plans/done/20260428-keyboard-qa-plan.md b/apps/desktop/plans/done/20260428-keyboard-qa-plan.md new file mode 100644 index 00000000000..32032c4b26f --- /dev/null +++ b/apps/desktop/plans/done/20260428-keyboard-qa-plan.md @@ -0,0 +1,213 @@ +# Keyboard Shortcut System — Manual QA Plan + +**Date:** 2026-04-28 +**Branch:** `keyboard-shortcut-analysi` +**Covers:** Phases 0–2 of `apps/desktop/plans/20260427-keyboard-layout-plan.md` (commits `eaf066364` → `5749f28b3`). + +This plan validates user-facing behavior. **Dev server is fine** — the renderer code paths exercised here behave identically to a packaged build. (We empirically verified `navigator.keyboard.getLayoutMap()` works in `file://` Electron 40, and `native-keymap` is loaded the same way in both modes once `bun run install:deps` has run.) A packaged build is only needed for final pre-release sign-off / bundle-size checks. + +--- + +## Setup + +### Required +- macOS with US (ABC) input source active +- A second input source: **German** (preferred, exercises Y/Z swap + ü/ö/ä) or **French (AZERTY)** (exercises punctuation more) +- App running (`bun dev` or packaged build — either works) + +### Optional but valuable +- Linux or Windows machine with a non-US layout for AltGr testing +- Japanese / Korean input source for IME testing +- Dvorak layout for the most dramatic logical-binding test + +### Quick switching +On macOS, add input sources via *System Settings → Keyboard → Input Sources*. Toggle via the menu-bar input picker (or `⌃Space` / `⌃⌥Space` if assigned). + +--- + +## 1. Smoke regression (US baseline) + +**Must pass before anything else.** If any of these fail, stop and revert. + +- [ ] App launches; no console errors related to `native-keymap`, `keyboardLayout`, or hotkeys. +- [ ] `Cmd+Shift+P` opens Quick Open. +- [ ] `Cmd+T` opens a new terminal tab. +- [ ] `Cmd+,` opens Settings. +- [ ] Settings → Keyboard renders all categories. Glyphs match what they did before this work (e.g. `⌘[`, `⌘]`, `⌘,`, `⌘P`). +- [ ] Click any hotkey row → recording → press a new combo → save. Reload app — binding persists. +- [ ] `Reset all` clears overrides and restores defaults. + +--- + +## 2. Layout-aware display (Phase 1) + +Verifies `native-keymap` is feeding live data and the display refreshes on the fly. + +### 2.1 Display swap on input-source change + +- [ ] US active. Settings → Keyboard. Note the glyph for **Navigate Back** (should be `⌘[`) and **Navigate Forward** (should be `⌘]`). +- [ ] Switch to **German** via menu-bar picker. **Without reloading or reopening Settings:** + - [ ] **Navigate Back** display updates to `⌘Ü` + - [ ] **Navigate Forward** display updates to `⌘+` + - [ ] Update happens within ~1 second +- [ ] Switch back to US. Glyphs revert to `⌘[` and `⌘]`. + +### 2.2 Layout-stable keys don't change + +- [ ] Arrow-key bindings (e.g. any `⌘↑`-style binding) display the same on every layout — they're named keys, immune to layout. +- [ ] **Open Settings** (`⌘,`) still dispatches correctly on every layout (the comma key is at the same physical position on US/UK/German). On AZERTY the comma is at a different physical position and the displayed glyph may change accordingly — verify it dispatches when pressing the actual comma key, regardless of label. + +### 2.3 Live-data sanity (DevTools, optional) + +In renderer DevTools console: +```js +JSON.stringify([ + ["KeyA", await navigator.keyboard.getLayoutMap().then(m => m.get("KeyA"))], + ["KeyZ", await navigator.keyboard.getLayoutMap().then(m => m.get("KeyZ"))], +]) +``` +On German: `KeyZ` should be `"y"`. On US: `"z"`. (We use `native-keymap` internally, not this API, but it's a quick sanity check that the OS *is* reporting different layouts.) + +--- + +## 3. Logical bindings (Phase 2 — the feature) + +Verifies that bindings can be recorded by **printed character** and follow that character across layouts. + +### 3.1 Record a logical binding + +- [ ] US active. Settings → Keyboard. Pick any printable shortcut (e.g. one of the workspace bindings). +- [ ] Click its current binding to start recording. +- [ ] Press `Cmd+Shift+J` (or any unused chord). Save. +- [ ] Settings shows the new binding as `⌘⇧J`. +- [ ] Reload the app — binding persists, fires correctly when pressed. +- [ ] Inspect `localStorage.getItem("hotkey-overrides")` — the override is a v2 object: `{ version: 2, mode: "logical", chord: "meta+shift+j" }`. + +### 3.2 The cross-layout payoff + +This is what Phase 2 was built for. Demonstrates that a binding follows the *printed character*, not the *physical position*. + +- [ ] Switch to **German**. +- [ ] In Settings → Keyboard, find the binding you recorded above (`⌘⇧J`). It still displays as `⌘⇧J`. +- [ ] Press the **key labeled `J`** on your German layout. The binding fires. + - On a German keyboard, the J key is at the same physical position as US, so this is unremarkable. The interesting test: +- [ ] Re-record any letter binding by pressing **the key labeled Z** on German (which is physical KeyY). E.g. record as `⌘Z`. +- [ ] Settings shows `⌘Z`. +- [ ] Switch back to **US**. The binding still displays as `⌘Z`. +- [ ] Press US's `Z` (physical KeyZ). The binding fires. +- [ ] **Before Phase 2, this was impossible** — the binding would have been bound to physical KeyY on German, so on US it would fire when pressing Y, not Z. + +### 3.3 Default registry bindings stay physical + +The shipped defaults preserve today's behavior — only *new user recordings* default to logical. + +- [ ] On US, Settings → Keyboard. The default `Cmd+P` (Quick Open) display is `⌘P`. +- [ ] Switch to **German**. Display becomes `⌘P` still — same physical key (P is at the same position on German anyway). +- [ ] But for a default that uses a key whose position differs (none today, since registry doesn't bind Y or Z): the display would update via the layout map. This is layout-aware *display*, not mode-switching. + +### 3.4 Named keys are layout-immune + +- [ ] Re-record a binding to use `Cmd+ArrowUp`. Save. +- [ ] Switch to German. Display unchanged: `⌘↑`. +- [ ] Press `Cmd+ArrowUp`. Fires. +- [ ] Same test for an F-key binding. + +### 3.5 Conflict detection across modes + +- [ ] On US, record any binding as `Cmd+Shift+K`. +- [ ] Try to record a *different* hotkey to the same `Cmd+Shift+K`. Conflict dialog should appear. +- [ ] "Reassign" should move the binding to the new hotkey, clear the old. + +--- + +## 4. Edge cases & defensive guards (Phase 0) + +### 4.1 IME composition guard + +Mac with Japanese (Hiragana) input source. + +- [ ] Switch to Japanese. Type into any text field (chat input, settings search). Composition underline appears. +- [ ] Press Enter — composition commits as Japanese characters. ✓ correct macOS behavior. +- [ ] Press `Cmd+P` mid-composition — Quick Open opens. ✓ correct macOS behavior (Cmd bypasses IME). +- [ ] No console errors during any of this. + +The guard prevents bare-key (no-modifier) hotkeys from firing during composition. We don't have any bare-key bindings, so this is mostly preventive — main thing is no regressions. + +### 4.2 AltGr guard (Linux/Windows w/ German, optional) + +On a Linux or Windows machine with German layout: + +- [ ] Press `AltGr+E` in a text field. `€` is typed. No app hotkey fires. +- [ ] Confirms `ctrl+alt+e` bindings (if any) don't fire on AltGr-typed printables. + +Skip if you don't have a Linux/Windows test machine. + +### 4.3 Migration is fail-closed (Mac with v1 overrides, optional) + +If you have a Mac with the old v1 hotkey storage (pre-April migration): + +- [ ] On a non-US Mac (e.g. German active), launch the app. +- [ ] Watch console for `[hotkeys] Migrated N override(s), dropped K invalid`. +- [ ] Specifically, v1 entries that look like Mac-Option dead-key glyphs (e.g. `meta+alt+å`) should be **dropped**, not silently rewritten to wrong physical keys. + +Skip if you're already on v2 (most likely). + +### 4.4 Terminal forwarding still works + +- [ ] Open a terminal pane. Run `nvim` or `htop` or any TUI. +- [ ] `Ctrl+C`, `Ctrl+D`, `Ctrl+Z`, `Ctrl+S`, `Ctrl+Q`, `Ctrl+\` all reach the TUI/PTY (terminal-reserved). +- [ ] `Cmd+T`, `Cmd+Shift+P` etc. still trigger app hotkeys even with terminal focused. +- [ ] Rebind a hotkey to `Ctrl+R`, then in a terminal press `Ctrl+R` — should trigger the app hotkey, not reach the shell. (Phase 2 dispatch into the terminal-forwarding reverse index.) + +### 4.5 Shifted glyphs + +- [ ] Record a binding to `Cmd+Shift+/` (US: `?`). Display: `⌘⇧/` (we use unshifted-glyph + ⇧ symbol convention, not `⌘⇧?`). + +--- + +## 5. Storage shape (sanity check) + +Open DevTools, paste: +```js +JSON.parse(localStorage.getItem("hotkey-overrides") || "{}") +``` + +- [ ] Existing physical / shipped-default overrides remain bare strings: `"meta+shift+j"`. +- [ ] New logical overrides are v2 objects: `{ version: 2, mode: "logical", chord: "meta+shift+j" }`. +- [ ] Explicitly unassigned hotkeys are `null`. +- [ ] No `undefined` values, no malformed entries. + +--- + +## 6. Performance / no-regression + +- [ ] App launch time feels normal (no perceptible delay added by `native-keymap` lazy-init or layout subscription). +- [ ] Settings → Keyboard scrolls smoothly with all bindings rendered. +- [ ] Switching input sources doesn't cause UI lag or stutter. +- [ ] Heavy typing in the chat / editor doesn't show input lag (the keydown listener at document level for hotkey dispatch is cheap, but worth checking). + +--- + +## What to do if something fails + +| Symptom | Likely cause | Where to look | +|---|---|---| +| Display doesn't swap on layout change | `native-keymap` IPC not flowing | Main process logs; `apps/desktop/src/main/lib/keyboardLayout.ts` | +| Logical binding fires for wrong key | `translateLogicalChord` misbehaving | `apps/desktop/src/renderer/hotkeys/utils/binding.ts` | +| Default hotkey stops firing | Registry / migration regression | `apps/desktop/src/renderer/hotkeys/registry.ts`, `migrate.ts` | +| Storage corruption | `setOverride` / `serializeBinding` writing wrong shape | `stores/hotkeyOverridesStore.ts`, `utils/binding.ts` | +| Terminal swallows app hotkey | Reverse index not subscribing to layout changes | `utils/resolveHotkeyFromEvent.ts` | +| Console error mentioning `keymapping.node` | `electron-rebuild` didn't run for native-keymap | `bun run install:deps` in `apps/desktop` | + +--- + +## Sign-off criteria + +- [ ] Section 1 (smoke) all green. +- [ ] Section 2 (display) all green on at least US + one non-US layout. +- [ ] Section 3 (logical bindings) — at least 3.1 + 3.2 green; 3.3–3.5 if time. +- [ ] Section 4 (edge cases) — 4.1 + 4.4 green; 4.2/4.3 if you have the environment. +- [ ] Section 5 (storage) green. +- [ ] No console errors throughout. + +If all of the above pass, the keyboard system is ready to ship. diff --git a/apps/desktop/runtime-dependencies.ts b/apps/desktop/runtime-dependencies.ts index dc4c7712bfb..fa36b93e61b 100644 --- a/apps/desktop/runtime-dependencies.ts +++ b/apps/desktop/runtime-dependencies.ts @@ -43,6 +43,12 @@ const externalizedRuntimeModules: ExternalizedRuntimeModule[] = [ packagedCopies: [copyWholeModule("node-pty")], asarUnpackGlobs: ["**/node_modules/node-pty/**/*"], }, + { + specifier: "native-keymap", + materialize: ["native-keymap"], + packagedCopies: [copyWholeModule("native-keymap")], + asarUnpackGlobs: ["**/node_modules/native-keymap/**/*"], + }, { specifier: "@superset/macos-process-metrics", materialize: ["@superset/macos-process-metrics"], diff --git a/apps/desktop/src/lib/trpc/routers/index.ts b/apps/desktop/src/lib/trpc/routers/index.ts index c3a0db245b2..ef1b871c458 100644 --- a/apps/desktop/src/lib/trpc/routers/index.ts +++ b/apps/desktop/src/lib/trpc/routers/index.ts @@ -13,6 +13,7 @@ import { createConfigRouter } from "./config"; import { createExternalRouter } from "./external"; import { createFilesystemRouter } from "./filesystem"; import { createHostServiceCoordinatorRouter } from "./host-service-coordinator"; +import { createKeyboardLayoutRouter } from "./keyboardLayout"; import { createMenuRouter } from "./menu"; import { createMigrationRouter } from "./migration"; import { createNotificationsRouter } from "./notifications"; @@ -54,6 +55,7 @@ export const createAppRouter = (getWindow: () => BrowserWindow | null) => { uiState: createUiStateRouter(), ringtone: createRingtoneRouter(getWindow), hostServiceCoordinator: createHostServiceCoordinatorRouter(), + keyboardLayout: createKeyboardLayoutRouter(), migration: createMigrationRouter(), }); }; diff --git a/apps/desktop/src/lib/trpc/routers/keyboardLayout.ts b/apps/desktop/src/lib/trpc/routers/keyboardLayout.ts new file mode 100644 index 00000000000..b698e15b73f --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/keyboardLayout.ts @@ -0,0 +1,25 @@ +import { observable } from "@trpc/server/observable"; +import { + getKeyboardLayoutSnapshot, + type KeyboardLayoutData, + onKeyboardLayoutChange, +} from "main/lib/keyboardLayout"; +import { publicProcedure, router } from ".."; + +export const createKeyboardLayoutRouter = () => { + return router({ + get: publicProcedure.query((): KeyboardLayoutData => { + return getKeyboardLayoutSnapshot(); + }), + // observable (not async generator) per apps/desktop/AGENTS.md — + // trpc-electron only supports observables for IPC subscriptions. + changes: publicProcedure.subscription(() => { + return observable((emit) => { + // Prime the subscriber with the current snapshot so the renderer + // store doesn't have to make a separate query on subscribe. + emit.next(getKeyboardLayoutSnapshot()); + return onKeyboardLayoutChange((data) => emit.next(data)); + }); + }), + }); +}; diff --git a/apps/desktop/src/main/lib/keyboardLayout.ts b/apps/desktop/src/main/lib/keyboardLayout.ts new file mode 100644 index 00000000000..ba88ac51b39 --- /dev/null +++ b/apps/desktop/src/main/lib/keyboardLayout.ts @@ -0,0 +1,101 @@ +import { EventEmitter } from "node:events"; + +// Wraps native-keymap for the renderer (mirrors VSCode's +// keyboardLayoutMainService). Lazy-loads on first read so the native module +// only initializes when actually needed. On macOS, native-keymap hooks +// Apple's kTISNotifySelectedKeyboardInputSourceChanged distributed +// notification — input-source switches fire onChange within milliseconds, +// which navigator.keyboard.layoutchange does not do in Chromium. + +export interface KeyboardLayoutData { + /** OS-specific layout id, e.g. "com.apple.keylayout.German". Empty if unavailable. */ + layoutId: string; + /** Localized human-readable name, e.g. "German". Empty if unavailable. */ + layoutName: string; + /** Map. Phase 2 may extend with shifted/altgr layers. */ + unshifted: Record; +} + +const EMPTY: KeyboardLayoutData = { + layoutId: "", + layoutName: "", + unshifted: {}, +}; + +const emitter = new EventEmitter(); +let cached: KeyboardLayoutData = EMPTY; +let initialized = false; + +type NativeKeymapModule = typeof import("native-keymap"); + +let nativeKeymap: NativeKeymapModule | null = null; + +function loadNative(): NativeKeymapModule | null { + if (nativeKeymap) return nativeKeymap; + try { + nativeKeymap = require("native-keymap") as NativeKeymapModule; + return nativeKeymap; + } catch (err) { + console.error("[keyboardLayout] failed to load native-keymap:", err); + return null; + } +} + +function read(): KeyboardLayoutData { + const mod = loadNative(); + if (!mod) return EMPTY; + try { + const info = mod.getCurrentKeyboardLayout() as { + id?: string; + name?: string; + localizedName?: string; + lang?: string; + } | null; + const map = mod.getKeyMap() as Record; + const unshifted: Record = {}; + for (const [code, entry] of Object.entries(map)) { + if (entry?.value) unshifted[code] = entry.value; + } + return { + layoutId: info?.id ?? info?.name ?? "", + layoutName: info?.localizedName ?? info?.name ?? "", + unshifted, + }; + } catch (err) { + console.error("[keyboardLayout] read failed:", err); + return EMPTY; + } +} + +function ensureInitialized(): void { + if (initialized) return; + initialized = true; + const mod = loadNative(); + if (!mod) return; + cached = read(); + try { + mod.onDidChangeKeyboardLayout(() => { + cached = read(); + emitter.emit("change", cached); + }); + } catch (err) { + console.error("[keyboardLayout] failed to register listener:", err); + } +} + +/** Current layout snapshot. Initializes native-keymap on first call. */ +export function getKeyboardLayoutSnapshot(): KeyboardLayoutData { + ensureInitialized(); + return cached; +} + +/** Subscribe to layout changes. Returns an unsubscribe function. */ +export function onKeyboardLayoutChange( + cb: (data: KeyboardLayoutData) => void, +): () => void { + ensureInitialized(); + emitter.on("change", cb); + return () => { + emitter.off("change", cb); + }; +} diff --git a/apps/desktop/src/renderer/hotkeys/display.test.ts b/apps/desktop/src/renderer/hotkeys/display.test.ts index 9e56bbac551..1afb504335a 100644 --- a/apps/desktop/src/renderer/hotkeys/display.test.ts +++ b/apps/desktop/src/renderer/hotkeys/display.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { formatHotkeyDisplay } from "./display"; +import { formatHotkeyDisplay, glyphForCode } from "./display"; describe("formatHotkeyDisplay", () => { it("formats a mac chord with modifier glyphs and no separator", () => { @@ -43,3 +43,110 @@ describe("formatHotkeyDisplay", () => { }); }); }); + +describe("glyphForCode", () => { + const usMap = new Map([ + ["KeyA", "a"], + ["KeyZ", "z"], + ["Slash", "/"], + ["Quote", "'"], + ["Digit5", "5"], + ]); + const qwertzMap = new Map([ + ["KeyA", "a"], + ["KeyZ", "y"], // QWERTZ — Y/Z swapped + ["Slash", "-"], + ["Quote", "ä"], + ]); + + it("returns null when no layout map provided", () => { + expect(glyphForCode("z", null)).toBeNull(); + }); + + it("returns the printed glyph for a letter on the current layout", () => { + expect(glyphForCode("a", usMap)).toBe("A"); + expect(glyphForCode("z", usMap)).toBe("Z"); + expect(glyphForCode("z", qwertzMap)).toBe("Y"); + }); + + it("returns the printed glyph for digits", () => { + expect(glyphForCode("5", usMap)).toBe("5"); + }); + + it("returns the printed glyph for punctuation tokens", () => { + expect(glyphForCode("slash", usMap)).toBe("/"); + expect(glyphForCode("slash", qwertzMap)).toBe("-"); + }); + + it("returns null for special keys that don't have a printable glyph", () => { + expect(glyphForCode("enter", usMap)).toBeNull(); + expect(glyphForCode("arrowup", usMap)).toBeNull(); + expect(glyphForCode("escape", usMap)).toBeNull(); + expect(glyphForCode("f5", usMap)).toBeNull(); + }); + + it("returns null when the layout map has no entry for the code", () => { + expect(glyphForCode("z", new Map())).toBeNull(); + }); + + it("returns null for multi-character (composing) glyphs", () => { + const composing = new Map([["KeyA", "ʼa"]]); + expect(glyphForCode("a", composing)).toBeNull(); + }); + + it("preserves non-ASCII glyphs that would expand on uppercase (ß, ı)", () => { + // "ß".toUpperCase() === "SS" in JS — would break single-keycap display + const german = new Map([["KeyS", "ß"]]); + expect(glyphForCode("s", german)).toBe("ß"); + const turkish = new Map([["KeyI", "ı"]]); + expect(glyphForCode("i", turkish)).toBe("ı"); + }); +}); + +describe("formatHotkeyDisplay — layout-aware", () => { + const usMap = new Map([ + ["KeyZ", "z"], + ["Slash", "/"], + ["BracketLeft", "["], + ]); + const qwertzMap = new Map([ + ["KeyZ", "y"], + ["Slash", "-"], + ]); + + it("uses the layout glyph for printable keys when a map is provided", () => { + expect(formatHotkeyDisplay("meta+z", "mac", qwertzMap).text).toBe("⌘Y"); + expect(formatHotkeyDisplay("ctrl+slash", "linux", qwertzMap).text).toBe( + "Ctrl+-", + ); + }); + + it("falls back to KEY_DISPLAY when layoutMap is null (regression — current behavior)", () => { + expect(formatHotkeyDisplay("meta+z", "mac", null).text).toBe("⌘Z"); + expect(formatHotkeyDisplay("ctrl+slash", "linux", null).text).toBe( + "Ctrl+/", + ); + }); + + it("matches today's output for a US-equivalent map (no visible change)", () => { + expect(formatHotkeyDisplay("meta+z", "mac").text).toBe( + formatHotkeyDisplay("meta+z", "mac", usMap).text, + ); + expect(formatHotkeyDisplay("ctrl+slash", "linux").text).toBe( + formatHotkeyDisplay("ctrl+slash", "linux", usMap).text, + ); + }); + + it("special keys ignore layoutMap and keep their symbol", () => { + // Even if a malicious map tried to remap "Enter", we ignore it for + // special keys since glyphForCode returns null for them. + const weird = new Map([["Enter", "X"]]); + expect(formatHotkeyDisplay("meta+enter", "mac", weird).text).toBe("⌘↵"); + }); + + it("falls back to KEY_DISPLAY when layoutMap is missing the code (e.g. Numpad)", () => { + expect(formatHotkeyDisplay("meta+bracketleft", "mac", qwertzMap).text).toBe( + "⌘[", + ); + }); +}); diff --git a/apps/desktop/src/renderer/hotkeys/display.ts b/apps/desktop/src/renderer/hotkeys/display.ts index 269f55a1183..ba9a08cf14d 100644 --- a/apps/desktop/src/renderer/hotkeys/display.ts +++ b/apps/desktop/src/renderer/hotkeys/display.ts @@ -39,6 +39,47 @@ const KEY_DISPLAY: Record = { bracketright: "]", }; +// canonical token (e.g. "z", "slash") → event.code (e.g. "KeyZ", "Slash") +// for keymap lookup against the layout data sourced from native-keymap. +// Only includes printable keys whose glyph varies by layout. Special keys +// (Enter, arrows, etc.) deliberately stay on KEY_DISPLAY — their +// event.code isn't a printable character. +const PRINTABLE_TO_SCAN_CODE: Record = { + slash: "Slash", + backslash: "Backslash", + comma: "Comma", + period: "Period", + semicolon: "Semicolon", + quote: "Quote", + backquote: "Backquote", + minus: "Minus", + equal: "Equal", + bracketleft: "BracketLeft", + bracketright: "BracketRight", +}; + +function canonicalToScanCode(canonical: string): string | null { + if (/^[a-z]$/.test(canonical)) return `Key${canonical.toUpperCase()}`; + if (/^[0-9]$/.test(canonical)) return `Digit${canonical}`; + return PRINTABLE_TO_SCAN_CODE[canonical] ?? null; +} + +/** Glyph printed at this physical key on the user's current layout, or null. */ +export function glyphForCode( + canonical: string, + layoutMap: ReadonlyMap | null, +): string | null { + if (!layoutMap) return null; + const scan = canonicalToScanCode(canonical); + if (!scan) return null; + const v = layoutMap.get(scan); + if (!v || v.length !== 1) return null; + // Uppercase only ASCII letters. Some layout glyphs expand to multiple + // characters when uppercased (`ß` → `SS`, Turkish `ı` → `I`/`İ`) which + // would break single-glyph keycap rendering — keep those as-is. + return /^[a-z]$/.test(v) ? v.toUpperCase() : v; +} + const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift"] as const; type Modifier = (typeof MODIFIER_ORDER)[number]; @@ -48,10 +89,18 @@ const isModifier = (p: string): p is Modifier => /** * Format a chord string into display symbols. * e.g. `"meta+shift+n"` on mac → `{ keys: ["⌘", "⇧", "N"], text: "⌘⇧N" }` + * + * `layoutMap` (optional) is `Map` derived from + * the OS keyboard layout (sourced from native-keymap via the main process). + * When provided, printable keys (letters/digits/punctuation) are looked up + * so the displayed glyph matches what the user sees on their physical key + * — e.g. `meta+z` shows `⌘Y` on a German QWERTZ keyboard. When null, falls + * back to the US-ANSI glyph table. */ export function formatHotkeyDisplay( keys: string | null, platform: Platform, + layoutMap: ReadonlyMap | null = null, ): HotkeyDisplay { if (!keys) return { keys: ["Unassigned"], text: "Unassigned" }; @@ -68,7 +117,12 @@ export function formatHotkeyDisplay( const modSymbols = MODIFIER_ORDER.filter((m) => modifiers.includes(m)).map( (m) => MODIFIER_DISPLAY[platform][m], ); - const keyDisplay = KEY_DISPLAY[key] ?? key.toUpperCase(); + // Order matters: layoutMap wins for printable keys (so QWERTZ shows the + // user's printed glyph for `KeyZ`), KEY_DISPLAY wins for special keys + // (Enter, arrows, etc. — glyphForCode returns null for these because + // PRINTABLE_TO_SCAN_CODE doesn't include them). + const keyDisplay = + glyphForCode(key, layoutMap) ?? KEY_DISPLAY[key] ?? key.toUpperCase(); const displayKeys = [...modSymbols, keyDisplay]; const separator = platform === "mac" ? "" : "+"; return { keys: displayKeys, text: displayKeys.join(separator) }; diff --git a/apps/desktop/src/renderer/hotkeys/hooks/index.ts b/apps/desktop/src/renderer/hotkeys/hooks/index.ts index 4faaa115b01..681c17d9108 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/index.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/index.ts @@ -1,3 +1,4 @@ +export { getBinding, getDispatchChord, useBinding } from "./useBinding"; export { useHotkey } from "./useHotkey"; -export { useHotkeyDisplay } from "./useHotkeyDisplay"; +export { useFormatBinding, useHotkeyDisplay } from "./useHotkeyDisplay"; export { useRecordHotkeys } from "./useRecordHotkeys"; diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useBinding/index.ts b/apps/desktop/src/renderer/hotkeys/hooks/useBinding/index.ts index ebadbce8a4f..5c4b30a8835 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useBinding/index.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useBinding/index.ts @@ -1 +1 @@ -export { getBinding, useBinding } from "./useBinding"; +export { getBinding, getDispatchChord, useBinding } from "./useBinding"; diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts b/apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts index 949eb8fc598..f3ba672b9ce 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts @@ -1,8 +1,16 @@ import { HOTKEYS, type HotkeyId } from "../../registry"; import { useHotkeyOverridesStore } from "../../stores/hotkeyOverridesStore"; +import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore"; +import type { ShortcutBinding } from "../../types"; +import { bindingToDispatchChord } from "../../utils/binding"; -/** Reactive: get the effective key binding for a hotkey (override ?? default) */ -export function useBinding(id: HotkeyId): string | null { +/** + * Reactive: get the effective binding for a hotkey (override ?? default). + * Returns the raw stored shape — bare chord string (legacy / shipped + * defaults, treated as physical mode) or v2 object. Use `parseBinding` to + * normalize. + */ +export function useBinding(id: HotkeyId): ShortcutBinding | null { return useHotkeyOverridesStore((state) => { if (!id) return null; if (id in state.overrides) return state.overrides[id] ?? null; @@ -10,10 +18,23 @@ export function useBinding(id: HotkeyId): string | null { }); } -/** Imperative: get the effective key binding (for non-React contexts like xterm) */ -export function getBinding(id: HotkeyId): string | null { +/** Imperative version of {@link useBinding} for non-React contexts. */ +export function getBinding(id: HotkeyId): ShortcutBinding | null { const state = useHotkeyOverridesStore.getState(); if (!id) return null; if (id in state.overrides) return state.overrides[id] ?? null; return HOTKEYS[id]?.key ?? null; } + +/** + * Imperative dispatch-form chord (event.code-based, layout-translated for + * logical bindings). Use when synthesizing KeyboardEvents that should match + * the same registration `useHotkey` makes — otherwise the event won't fire + * the bound handler on non-US layouts. + */ +export function getDispatchChord(id: HotkeyId): string | null { + return bindingToDispatchChord( + getBinding(id), + useKeyboardLayoutStore.getState().map, + ); +} diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts b/apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts index 33fd696bf77..a36cbd8657d 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts @@ -3,27 +3,49 @@ import { type Options, useHotkeys } from "react-hotkeys-hook"; import { formatHotkeyDisplay } from "../../display"; import type { HotkeyId } from "../../registry"; import { PLATFORM } from "../../registry"; +import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore"; import type { HotkeyDisplay } from "../../types"; +import { bindingToDispatchChord } from "../../utils/binding"; import { useBinding } from "../useBinding"; +// react-hotkeys-hook doesn't check AltGraph or IME composition. Use its +// `ignoreEventWhen` option (runs after match, before preventDefault) to +// suppress those events so AltGr-typed printables and IME keystrokes pass +// through to the focused element. +function shouldIgnoreEvent(e: KeyboardEvent): boolean { + if (e.isComposing || e.keyCode === 229) return true; + if (e.getModifierState?.("AltGraph") === true) return true; + return false; +} + export function useHotkey( id: HotkeyId, callback: (e: KeyboardEvent) => void, options?: Options, ): HotkeyDisplay { - const keys = useBinding(id); + const binding = useBinding(id); + const layoutMap = useKeyboardLayoutStore((s) => s.map); + const chord = bindingToDispatchChord(binding, layoutMap); const callbackRef = useRef(callback); callbackRef.current = callback; + const callerIgnore = options?.ignoreEventWhen; useHotkeys( - keys ?? "", + chord ?? "", (e, _h) => { if (options?.preventDefault !== false) { e.preventDefault(); } callbackRef.current(e); }, - { enableOnFormTags: true, enableOnContentEditable: true, ...options }, - [keys], + { + enableOnFormTags: true, + enableOnContentEditable: true, + ...options, + ignoreEventWhen: callerIgnore + ? (e) => shouldIgnoreEvent(e) || callerIgnore(e) + : shouldIgnoreEvent, + }, + [chord], ); - return formatHotkeyDisplay(keys, PLATFORM); + return formatHotkeyDisplay(chord, PLATFORM, layoutMap); } diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/index.ts b/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/index.ts index a8fc6580bfc..6ffafab8271 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/index.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/index.ts @@ -1 +1 @@ -export { useHotkeyDisplay } from "./useHotkeyDisplay"; +export { useFormatBinding, useHotkeyDisplay } from "./useHotkeyDisplay"; diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts b/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts index 9ec2ba7130f..9df55b5c205 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts @@ -1,10 +1,34 @@ import { useMemo } from "react"; import { formatHotkeyDisplay } from "../../display"; import { PLATFORM } from "../../registry"; -import type { HotkeyDisplay } from "../../types"; +import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore"; +import type { HotkeyDisplay, ShortcutBinding } from "../../types"; +import { bindingToDispatchChord } from "../../utils/binding"; import { useBinding } from "../useBinding"; export function useHotkeyDisplay(id: string): HotkeyDisplay { const binding = useBinding(id as Parameters[0]); - return useMemo(() => formatHotkeyDisplay(binding, PLATFORM), [binding]); + const layoutMap = useKeyboardLayoutStore((s) => s.map); + const chord = bindingToDispatchChord(binding, layoutMap); + return useMemo( + () => formatHotkeyDisplay(chord, PLATFORM, layoutMap), + [chord, layoutMap], + ); +} + +/** + * Format an arbitrary binding (e.g. one captured during recording, before + * it's saved) with layout-aware glyphs. Use this when you have a + * ShortcutBinding but no registered hotkey id — most callers should use + * {@link useHotkeyDisplay} via the hotkey id. + */ +export function useFormatBinding( + binding: ShortcutBinding | null, +): HotkeyDisplay { + const layoutMap = useKeyboardLayoutStore((s) => s.map); + const chord = bindingToDispatchChord(binding, layoutMap); + return useMemo( + () => formatHotkeyDisplay(chord, PLATFORM, layoutMap), + [chord, layoutMap], + ); } diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.test.ts b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.test.ts index e12d434f337..3329c66f6bb 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.test.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.test.ts @@ -1,16 +1,19 @@ import { describe, expect, it } from "bun:test"; -import { captureHotkeyFromEvent } from "./useRecordHotkeys"; +import { + captureHotkeyFromEvent, + resolveCapturedBinding, +} from "./useRecordHotkeys"; /** - * Covers the three regressions fixed in - * apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md + * Covers the regressions fixed in + * apps/desktop/plans/20260412-keyboard-recorder-ctrl-binding-fix.md plus + * Phase 2 additions (logical/named classification, dual-form capture). * * Note: `captureHotkeyFromEvent` reads `PLATFORM` via registry.ts, which in a * Bun test runtime without a DOM navigator resolves to "mac". The meta-on- * non-Mac branch is exercised indirectly via review, not here. */ -// Bun test runtime has no DOM KeyboardEvent; stub just what the code reads. interface StubInit { code?: string | undefined; key?: string; @@ -22,8 +25,6 @@ interface StubInit { function ev(init: StubInit): KeyboardEvent { return { type: "keydown", - // Preserve explicit `undefined` so the captureHotkeyFromEvent guard - // against synthetic events (no event.code) is actually exercised. ...("code" in init ? { code: init.code } : { code: "" }), key: init.key ?? "", ctrlKey: !!init.ctrlKey, @@ -67,50 +68,52 @@ describe("captureHotkeyFromEvent — Bug 1: lone Ctrl must not auto-commit", () }); }); -describe("captureHotkeyFromEvent — Bug 2: uses event.code, not event.key", () => { - it("Ctrl+Shift+2 produces ctrl+shift+2 (not ctrl+shift+@)", () => { +describe("captureHotkeyFromEvent — codeChord uses event.code, not event.key", () => { + it("Ctrl+Shift+2 codeChord is ctrl+shift+2 (not ctrl+shift+@)", () => { const captured = captureHotkeyFromEvent( ev({ code: "Digit2", key: "@", ctrlKey: true, shiftKey: true }), ); - expect(captured).toBe("ctrl+shift+2"); + expect(captured?.codeChord).toBe("ctrl+shift+2"); }); - it("Alt+L on Mac (where event.key is `¬`) produces alt+l via event.code", () => { - // This event would only pass the `must include ctrl/meta` gate with ctrl - // also held; the point of the test is that we read `code`, not `key`. + it("Alt+L on Mac (event.key=`¬`) codeChord is ctrl+alt+l via event.code", () => { const captured = captureHotkeyFromEvent( ev({ code: "KeyL", key: "¬", ctrlKey: true, altKey: true }), ); - expect(captured).toBe("ctrl+alt+l"); + expect(captured?.codeChord).toBe("ctrl+alt+l"); }); - it("Ctrl+[ produces ctrl+bracketleft (registry form)", () => { + it("Ctrl+[ codeChord is ctrl+bracketleft (registry form)", () => { const captured = captureHotkeyFromEvent( ev({ code: "BracketLeft", key: "[", ctrlKey: true }), ); - expect(captured).toBe("ctrl+bracketleft"); + expect(captured?.codeChord).toBe("ctrl+bracketleft"); }); - it("Ctrl+/ produces ctrl+slash", () => { + it("Ctrl+/ codeChord is ctrl+slash", () => { const captured = captureHotkeyFromEvent( ev({ code: "Slash", key: "/", ctrlKey: true }), ); - expect(captured).toBe("ctrl+slash"); + expect(captured?.codeChord).toBe("ctrl+slash"); }); - it("Meta+Alt+ArrowUp produces meta+alt+arrowup", () => { + it("Meta+Alt+ArrowUp codeChord is meta+alt+arrowup", () => { const captured = captureHotkeyFromEvent( ev({ code: "ArrowUp", key: "ArrowUp", metaKey: true, altKey: true }), ); - expect(captured).toBe("meta+alt+arrowup"); + expect(captured?.codeChord).toBe("meta+alt+arrowup"); }); - it("F-keys are accepted without requiring a modifier", () => { - expect(captureHotkeyFromEvent(ev({ code: "F1", key: "F1" }))).toBe("f1"); - expect(captureHotkeyFromEvent(ev({ code: "F12", key: "F12" }))).toBe("f12"); + it("F-keys are accepted without a modifier", () => { + expect( + captureHotkeyFromEvent(ev({ code: "F1", key: "F1" }))?.codeChord, + ).toBe("f1"); + expect( + captureHotkeyFromEvent(ev({ code: "F12", key: "F12" }))?.codeChord, + ).toBe("f12"); }); - it("returns null when event.code is undefined (synthetic / autofill events)", () => { + it("returns null when event.code is undefined", () => { expect( captureHotkeyFromEvent(ev({ code: undefined, ctrlKey: true })), ).toBeNull(); @@ -129,6 +132,114 @@ describe("captureHotkeyFromEvent — modifier ordering", () => { shiftKey: true, }), ); - expect(captured).toBe("meta+ctrl+alt+shift+k"); + expect(captured?.codeChord).toBe("meta+ctrl+alt+shift+k"); + }); +}); + +describe("captureHotkeyFromEvent — classification & dual-form capture", () => { + it("classifies F-keys", () => { + const captured = captureHotkeyFromEvent(ev({ code: "F5", key: "F5" })); + expect(captured?.classification).toBe("fkey"); + }); + + it("classifies named keys (Enter, ArrowUp, Backspace, ...)", () => { + expect( + captureHotkeyFromEvent(ev({ code: "Enter", key: "Enter", metaKey: true })) + ?.classification, + ).toBe("named"); + expect( + captureHotkeyFromEvent( + ev({ code: "ArrowUp", key: "ArrowUp", metaKey: true }), + )?.classification, + ).toBe("named"); + }); + + it("classifies letters/digits/punctuation as printable", () => { + expect( + captureHotkeyFromEvent(ev({ code: "KeyP", key: "p", metaKey: true })) + ?.classification, + ).toBe("printable"); + expect( + captureHotkeyFromEvent(ev({ code: "Slash", key: "/", ctrlKey: true })) + ?.classification, + ).toBe("printable"); + }); + + it("named/fkey: keyChord matches codeChord", () => { + const named = captureHotkeyFromEvent( + ev({ code: "Enter", key: "Enter", metaKey: true }), + ); + expect(named?.keyChord).toBe(named?.codeChord); + const fkey = captureHotkeyFromEvent(ev({ code: "F1", key: "F1" })); + expect(fkey?.keyChord).toBe(fkey?.codeChord); + }); + + it("printable: keyChord uses event.key (Dvorak: KeyR + key='p' → meta+p)", () => { + const captured = captureHotkeyFromEvent( + ev({ code: "KeyR", key: "p", metaKey: true }), + ); + expect(captured?.codeChord).toBe("meta+r"); + expect(captured?.keyChord).toBe("meta+p"); + }); + + it("printable: keyChord lowercases shifted glyphs (Shift+P → 'P' → 'p')", () => { + const captured = captureHotkeyFromEvent( + ev({ code: "KeyP", key: "P", metaKey: true, shiftKey: true }), + ); + expect(captured?.keyChord).toBe("meta+shift+p"); + }); + + it("printable with multi-char event.key falls back to codeChord", () => { + // Dead-key composition or "Process" can produce non-single-char event.key + const captured = captureHotkeyFromEvent( + ev({ code: "KeyA", key: "Dead", metaKey: true }), + ); + expect(captured?.keyChord).toBe(captured?.codeChord); + }); + + it("printable '+' falls back to codeChord (would collide with chord separator)", () => { + // Shift+= on US produces event.key "+" — accepting it as a logical + // token would build "meta+shift++" which can't be parsed back. + const captured = captureHotkeyFromEvent( + ev({ code: "Equal", key: "+", metaKey: true, shiftKey: true }), + ); + expect(captured?.codeChord).toBe("meta+shift+equal"); + expect(captured?.keyChord).toBe(captured?.codeChord); + }); +}); + +describe("resolveCapturedBinding", () => { + function capture(init: StubInit) { + const captured = captureHotkeyFromEvent(ev(init)); + if (!captured) { + throw new Error("expected captureHotkeyFromEvent to succeed"); + } + return captured; + } + + it("F-keys force named regardless of preferredMode", () => { + const captured = capture({ code: "F5", key: "F5" }); + expect(resolveCapturedBinding(captured, "logical").mode).toBe("named"); + expect(resolveCapturedBinding(captured, "physical").mode).toBe("named"); + }); + + it("Named keys force named regardless of preferredMode", () => { + const captured = capture({ code: "Enter", key: "Enter", metaKey: true }); + expect(resolveCapturedBinding(captured, "logical").mode).toBe("named"); + expect(resolveCapturedBinding(captured, "physical").mode).toBe("named"); + }); + + it("Printable + logical → keyChord", () => { + const captured = capture({ code: "KeyR", key: "p", metaKey: true }); + const resolved = resolveCapturedBinding(captured, "logical"); + expect(resolved.mode).toBe("logical"); + expect(resolved.chord).toBe("meta+p"); + }); + + it("Printable + physical → codeChord", () => { + const captured = capture({ code: "KeyR", key: "p", metaKey: true }); + const resolved = resolveCapturedBinding(captured, "physical"); + expect(resolved.mode).toBe("physical"); + expect(resolved.chord).toBe("meta+r"); }); }); diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts index 8f7cdb0a4a7..cc8ac66e0f1 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts @@ -1,7 +1,20 @@ import { useEffect, useRef } from "react"; import { HOTKEYS, type HotkeyId, PLATFORM } from "../../registry"; import { useHotkeyOverridesStore } from "../../stores/hotkeyOverridesStore"; -import type { Platform } from "../../types"; +import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore"; +import type { + BindingMode, + ParsedBinding, + Platform, + ShortcutBinding, +} from "../../types"; +import { + bindingsEqual, + bindingToDispatchChord, + isFunctionKey, + NAMED_KEYS, + serializeBinding, +} from "../../utils/binding"; import { canonicalizeChord, isIgnorableKey, @@ -14,17 +27,27 @@ import { // reordering at compare time. const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift"] as const; -export function captureHotkeyFromEvent(event: KeyboardEvent): string | null { - // event.code (not event.key) so Shift+2 records as `2`, Alt+L on Mac as - // `l`, and non-US layouts produce stable tokens matching the registry. - if (event.code === undefined) return null; - const key = normalizeToken(event.code); - if (isIgnorableKey(key)) return null; +export interface CapturedHotkey { + /** Modifiers + canonical(event.code). Always meaningful. */ + codeChord: string; + /** Modifiers + lowercased event.key for printable letters/digits/punctuation; + * identical to codeChord for named keys / F-keys. */ + keyChord: string; + classification: "named" | "fkey" | "printable"; +} - const isFKey = /^f([1-9]|1[0-2])$/.test(key); - // On Mac, Option is a legitimate shortcut modifier (e.g. ⌥⌫ for delete-word). - // Elsewhere, Alt is the menu key and AltGr masquerades as ctrl+alt, so we - // still require ctrl/meta. +export function captureHotkeyFromEvent( + event: KeyboardEvent, +): CapturedHotkey | null { + if (event.code === undefined) return null; + const codeKey = normalizeToken(event.code); + if (isIgnorableKey(codeKey)) return null; + + const isFKey = isFunctionKey(codeKey); + const isNamed = NAMED_KEYS.has(codeKey); + // Mac Option is a legitimate shortcut modifier (⌥⌫ = delete-word). On + // other platforms Alt is the menu key and AltGr masquerades as ctrl+alt, + // so we still require ctrl/meta. const altIsAppModifier = PLATFORM === "mac" && event.altKey; if (!isFKey && !event.ctrlKey && !event.metaKey && !altIsAppModifier) { return null; @@ -35,9 +58,40 @@ export function captureHotkeyFromEvent(event: KeyboardEvent): string | null { if (event.ctrlKey) modifiers.add("ctrl"); if (event.altKey) modifiers.add("alt"); if (event.shiftKey) modifiers.add("shift"); - const ordered = MODIFIER_ORDER.filter((m) => modifiers.has(m)); - return [...ordered, key].join("+"); + + const codeChord = [...ordered, codeKey].join("+"); + + let classification: "named" | "fkey" | "printable" = "printable"; + if (isFKey) classification = "fkey"; + else if (isNamed) classification = "named"; + + let keyChord = codeChord; + if (classification === "printable") { + const produced = (event.key ?? "").toLowerCase(); + // Single printable char only — strings like "Dead", "Process" or + // multi-char IME output stay on codeChord. "+" would collide with + // the chord separator and break round-tripping (`meta+shift++`). + if (produced.length === 1 && /\S/.test(produced) && produced !== "+") { + keyChord = [...ordered, produced].join("+"); + } + } + return { codeChord, keyChord, classification }; +} + +/** + * Pick the right chord + mode for a captured event, given a user mode + * preference. F-keys and named keys force `named` regardless of preference. + */ +export function resolveCapturedBinding( + captured: CapturedHotkey, + preferredMode: "physical" | "logical", +): ParsedBinding { + if (captured.classification === "fkey" || captured.classification === "named") + return { mode: "named", chord: captured.codeChord }; + const mode: BindingMode = preferredMode; + const chord = mode === "logical" ? captured.keyChord : captured.codeChord; + return { mode, chord }; } // Chords the OS / shell is likely to intercept. Binding is allowed (Linux @@ -82,24 +136,42 @@ function checkReserved( return null; } -function getHotkeyConflict(keys: string, excludeId: HotkeyId): HotkeyId | null { +function getHotkeyConflict( + candidate: ShortcutBinding, + excludeId: HotkeyId, +): HotkeyId | null { const { overrides } = useHotkeyOverridesStore.getState(); - const canonicalKeys = canonicalizeChord(keys); + const layoutMap = useKeyboardLayoutStore.getState().map; + const candidateDispatch = bindingToDispatchChord(candidate, layoutMap); + if (!candidateDispatch) return null; + const target = canonicalizeChord(candidateDispatch); for (const id of Object.keys(HOTKEYS) as HotkeyId[]) { if (id === excludeId) continue; const effective = id in overrides ? overrides[id] : HOTKEYS[id].key; - if (effective && canonicalizeChord(effective) === canonicalKeys) return id; + if (!effective) continue; + const otherDispatch = bindingToDispatchChord(effective, layoutMap); + if (otherDispatch && canonicalizeChord(otherDispatch) === target) return id; } return null; } interface UseRecordHotkeysOptions { - onSave?: (id: HotkeyId, keys: string) => void; + /** User's mode preference for new printable bindings. Default `"logical"` + * — the recorded chord follows the printed character (Dvorak user + * pressing the P-labeled key gets a binding for the P character, which + * works on any layout). F-keys and named keys ignore this and use + * `"named"` mode regardless. */ + preferredMode?: "physical" | "logical"; + onSave?: (id: HotkeyId, binding: ShortcutBinding) => void; onCancel?: () => void; onUnassign?: (id: HotkeyId) => void; - onConflict?: (targetId: HotkeyId, keys: string, conflictId: HotkeyId) => void; + onConflict?: ( + targetId: HotkeyId, + binding: ShortcutBinding, + conflictId: HotkeyId, + ) => void; onReserved?: ( - keys: string, + binding: ShortcutBinding, info: { reason: string; severity: "error" | "warning" }, ) => void; } @@ -135,32 +207,35 @@ export function useRecordHotkeys( const captured = captureHotkeyFromEvent(event); if (!captured) return; - const reserved = checkReserved(captured); + const preferredMode = optionsRef.current?.preferredMode ?? "logical"; + const parsed = resolveCapturedBinding(captured, preferredMode); + const binding = serializeBinding(parsed); + + // Reserved chords gate on the dispatch chord (event.code form), since + // that's what the OS / terminal sees when the user presses the key. + const reserved = checkReserved(captured.codeChord); if (reserved?.severity === "error") { - optionsRef.current?.onReserved?.(captured, reserved); + optionsRef.current?.onReserved?.(binding, reserved); return; } - const conflictId = getHotkeyConflict(captured, recordingId); + const conflictId = getHotkeyConflict(binding, recordingId); if (conflictId) { - optionsRef.current?.onConflict?.(recordingId, captured, conflictId); + optionsRef.current?.onConflict?.(recordingId, binding, conflictId); return; } if (reserved?.severity === "warning") { - optionsRef.current?.onReserved?.(captured, reserved); + optionsRef.current?.onReserved?.(binding, reserved); } - const defaultKey = HOTKEYS[recordingId].key; - if ( - defaultKey && - canonicalizeChord(captured) === canonicalizeChord(defaultKey) - ) { + const defaultBinding = HOTKEYS[recordingId].key; + if (defaultBinding && bindingsEqual(binding, defaultBinding)) { resetOverride(recordingId); } else { - setOverride(recordingId, captured); + setOverride(recordingId, binding); } - optionsRef.current?.onSave?.(recordingId, captured); + optionsRef.current?.onSave?.(recordingId, binding); }; window.addEventListener("keydown", handler, { capture: true }); diff --git a/apps/desktop/src/renderer/hotkeys/index.ts b/apps/desktop/src/renderer/hotkeys/index.ts index 9874b054093..0cdc8a0ef37 100644 --- a/apps/desktop/src/renderer/hotkeys/index.ts +++ b/apps/desktop/src/renderer/hotkeys/index.ts @@ -1,17 +1,31 @@ export { HotkeyLabel } from "./components/HotkeyLabel"; export { formatHotkeyDisplay } from "./display"; -export { useHotkey, useHotkeyDisplay, useRecordHotkeys } from "./hooks"; -export { getBinding } from "./hooks/useBinding"; +export { + getBinding, + getDispatchChord, + useBinding, + useFormatBinding, + useHotkey, + useHotkeyDisplay, + useRecordHotkeys, +} from "./hooks"; export { HOTKEYS, type HotkeyId, PLATFORM } from "./registry"; -export { useHotkeyOverridesStore } from "./stores/hotkeyOverridesStore"; +export { useHotkeyOverridesStore } from "./stores"; export type { + BindingMode, HotkeyCategory, HotkeyDefinition, HotkeyDisplay, + ParsedBinding, Platform, + ShortcutBinding, } from "./types"; export { + bindingsEqual, + defaultModeForChord, isTerminalReservedEvent, matchesChord, + parseBinding, resolveHotkeyFromEvent, + serializeBinding, } from "./utils"; diff --git a/apps/desktop/src/renderer/hotkeys/migrate.ts b/apps/desktop/src/renderer/hotkeys/migrate.ts deleted file mode 100644 index 29a41a98535..00000000000 --- a/apps/desktop/src/renderer/hotkeys/migrate.ts +++ /dev/null @@ -1,62 +0,0 @@ -/** - * One-time migration from the old hotkey storage (main process JSON file via tRPC) - * to the new localStorage-based Zustand store. - * - * Marker key is bumped (`-v2`) so users who migrated on the pre-sanitizer - * build re-run once and get their corrupt entries dropped. - */ - -import { electronTrpcClient } from "renderer/lib/trpc-client"; -import { PLATFORM } from "./registry"; -import { isUSCompatibleLayout } from "./utils/detectUSLayout"; -import { sanitizeOverride } from "./utils/sanitizeOverride"; - -const MIGRATION_MARKER_KEY = "hotkey-overrides-migrated-v2"; - -const PLATFORM_MAP = { - mac: "darwin", - windows: "win32", - linux: "linux", -} as const; - -export async function migrateHotkeyOverrides(): Promise { - if (localStorage.getItem(MIGRATION_MARKER_KEY)) return; - - try { - const oldState = await electronTrpcClient.uiState.hotkeys.get.query(); - const oldPlatformKey = PLATFORM_MAP[PLATFORM]; - const oldOverrides = oldState?.byPlatform?.[oldPlatformKey]; - if (!oldOverrides || Object.keys(oldOverrides).length === 0) { - localStorage.setItem(MIGRATION_MARKER_KEY, "1"); - console.log("[hotkeys] Migration skipped — no old overrides found"); - return; - } - - const assumeUSMacLayout = - PLATFORM === "mac" ? await isUSCompatibleLayout() : true; - - const cleaned: Record = {}; - let dropped = 0; - for (const [id, raw] of Object.entries(oldOverrides)) { - const sanitized = sanitizeOverride(raw, { assumeUSMacLayout }); - if (sanitized === undefined) { - dropped++; - continue; - } - cleaned[id] = sanitized; - } - - localStorage.setItem( - "hotkey-overrides", - JSON.stringify({ state: { overrides: cleaned }, version: 0 }), - ); - localStorage.setItem(MIGRATION_MARKER_KEY, "1"); - console.log( - `[hotkeys] Migrated ${Object.keys(cleaned).length} override(s)` + - (dropped > 0 ? `, dropped ${dropped} invalid` : ""), - ); - } catch (error) { - // Marker intentionally not set — transient tRPC failures retry next boot. - console.log("[hotkeys] Migration failed, will retry next boot:", error); - } -} diff --git a/apps/desktop/src/renderer/hotkeys/stores/hotkeyOverridesStore.ts b/apps/desktop/src/renderer/hotkeys/stores/hotkeyOverridesStore.ts index bacf6572c00..2cb2acedfe1 100644 --- a/apps/desktop/src/renderer/hotkeys/stores/hotkeyOverridesStore.ts +++ b/apps/desktop/src/renderer/hotkeys/stores/hotkeyOverridesStore.ts @@ -1,9 +1,13 @@ import { create } from "zustand"; import { createJSONStorage, persist } from "zustand/middleware"; +import type { ShortcutBinding } from "../types"; interface HotkeyOverridesState { - overrides: Record; - setOverride: (id: string, keys: string | null) => void; + /** Per-hotkey-id override. `null` = explicit unassignment. Stored as the + * ShortcutBinding shape: bare string for physical-mode bindings (legacy + * + shipped defaults), v2 object for logical / named modes. */ + overrides: Record; + setOverride: (id: string, binding: ShortcutBinding | null) => void; resetOverride: (id: string) => void; resetAll: () => void; } diff --git a/apps/desktop/src/renderer/hotkeys/stores/keyboardLayoutStore.ts b/apps/desktop/src/renderer/hotkeys/stores/keyboardLayoutStore.ts new file mode 100644 index 00000000000..9b602cceca4 --- /dev/null +++ b/apps/desktop/src/renderer/hotkeys/stores/keyboardLayoutStore.ts @@ -0,0 +1,55 @@ +import type { KeyboardLayoutData } from "main/lib/keyboardLayout"; +import { electronTrpcClient } from "renderer/lib/trpc-client"; +import { create } from "zustand"; + +// Mirror of the main-process layout service for synchronous reads from +// React. Lives in main because macOS input-source switches (menu-bar +// picker, Cmd+Space) don't fire navigator.keyboard's `layoutchange` — +// native-keymap hooks the OS-level +// kTISNotifySelectedKeyboardInputSourceChanged distributed notification, +// which fires for every input-source change. + +interface State { + /** Map. Null until the first tRPC payload + * arrives (~10ms after window load); display falls back to US-ANSI + * glyphs while null. */ + map: ReadonlyMap | null; + /** OS-specific layout id, e.g. "com.apple.keylayout.German". */ + layoutId: string; +} + +export const useKeyboardLayoutStore = create(() => ({ + map: null, + layoutId: "", +})); + +function applySnapshot(data: KeyboardLayoutData): void { + useKeyboardLayoutStore.setState({ + map: new Map(Object.entries(data.unshifted)), + layoutId: data.layoutId, + }); +} + +// Process-lifetime subscription. If it errors, retry with backoff — +// otherwise `map` would stay null until window reload and every hotkey +// label would silently fall back to US-ANSI glyphs. +const RETRY_BACKOFF_MS = [1_000, 2_000, 5_000, 10_000]; +let retryAttempt = 0; + +function startKeyboardLayoutSync(): void { + electronTrpcClient.keyboardLayout.changes.subscribe(undefined, { + onData: (data) => { + retryAttempt = 0; + applySnapshot(data); + }, + onError: (err) => { + console.error("[keyboardLayoutStore] subscription error:", err); + const idx = Math.min(retryAttempt, RETRY_BACKOFF_MS.length - 1); + const delay = RETRY_BACKOFF_MS[idx] ?? 10_000; + retryAttempt++; + setTimeout(startKeyboardLayoutSync, delay); + }, + }); +} + +startKeyboardLayoutSync(); diff --git a/apps/desktop/src/renderer/hotkeys/types.ts b/apps/desktop/src/renderer/hotkeys/types.ts index a45a59c8f12..96d6982c73e 100644 --- a/apps/desktop/src/renderer/hotkeys/types.ts +++ b/apps/desktop/src/renderer/hotkeys/types.ts @@ -27,3 +27,35 @@ export interface HotkeyDefinition { category: HotkeyCategory; description?: string; } + +/** + * How a binding identifies a key: + * - `physical`: matches `event.code` — same physical key on every layout. + * Default for shipped registry entries (preserves QWERTY muscle memory). + * - `logical`: matches the produced character (`event.key`) — same printed + * letter on every layout, even when it lives on different physical keys. + * Default for new user-recorded printable bindings. + * - `named`: stable named keys (Enter, ArrowUp, F1-F12, ...). Used + * automatically for non-printable keys regardless of preference. + */ +export type BindingMode = "physical" | "logical" | "named"; + +/** + * Stored as a bare chord string for legacy / shipped defaults (implicitly + * physical) or a v2 object for explicit modes. The legacy string form is + * preserved indefinitely so default registry entries stay terse. + */ +export type ShortcutBinding = + | string + | { + version: 2; + mode: BindingMode; + /** Canonical form, e.g. "meta+shift+p", "ctrl+slash". */ + chord: string; + }; + +/** Normalized view of a binding, regardless of stored form. */ +export interface ParsedBinding { + mode: BindingMode; + chord: string; +} diff --git a/apps/desktop/src/renderer/hotkeys/utils/binding.test.ts b/apps/desktop/src/renderer/hotkeys/utils/binding.test.ts new file mode 100644 index 00000000000..6282ee88ad2 --- /dev/null +++ b/apps/desktop/src/renderer/hotkeys/utils/binding.test.ts @@ -0,0 +1,292 @@ +import { describe, expect, it } from "bun:test"; +import { + bindingsEqual, + bindingToDispatchChord, + defaultModeForChord, + parseBinding, + serializeBinding, + translateLogicalChord, +} from "./binding"; + +describe("defaultModeForChord", () => { + it("classifies named keys as 'named'", () => { + expect(defaultModeForChord("meta+enter")).toBe("named"); + expect(defaultModeForChord("ctrl+arrowup")).toBe("named"); + expect(defaultModeForChord("alt+up")).toBe("named"); + expect(defaultModeForChord("escape")).toBe("named"); + expect(defaultModeForChord("backspace")).toBe("named"); + }); + + it("classifies F-keys as 'named'", () => { + expect(defaultModeForChord("f1")).toBe("named"); + expect(defaultModeForChord("meta+f10")).toBe("named"); + expect(defaultModeForChord("f12")).toBe("named"); + }); + + it("classifies letters/digits/punctuation as 'physical'", () => { + expect(defaultModeForChord("meta+p")).toBe("physical"); + expect(defaultModeForChord("ctrl+shift+1")).toBe("physical"); + expect(defaultModeForChord("meta+slash")).toBe("physical"); + expect(defaultModeForChord("ctrl+bracketleft")).toBe("physical"); + }); +}); + +describe("parseBinding", () => { + it("treats legacy string as physical for printable keys", () => { + expect(parseBinding("meta+p")).toEqual({ + mode: "physical", + chord: "meta+p", + }); + }); + + it("treats legacy string as named for special keys", () => { + expect(parseBinding("meta+enter")).toEqual({ + mode: "named", + chord: "meta+enter", + }); + expect(parseBinding("f5")).toEqual({ mode: "named", chord: "f5" }); + }); + + it("preserves explicit v2 object form", () => { + expect( + parseBinding({ version: 2, mode: "logical", chord: "meta+p" }), + ).toEqual({ mode: "logical", chord: "meta+p" }); + expect( + parseBinding({ version: 2, mode: "physical", chord: "meta+p" }), + ).toEqual({ mode: "physical", chord: "meta+p" }); + }); +}); + +describe("serializeBinding", () => { + it("compacts physical mode to bare string (matches legacy storage)", () => { + expect(serializeBinding({ mode: "physical", chord: "meta+p" })).toBe( + "meta+p", + ); + }); + + it("encodes logical mode as v2 object", () => { + expect(serializeBinding({ mode: "logical", chord: "meta+p" })).toEqual({ + version: 2, + mode: "logical", + chord: "meta+p", + }); + }); + + it("encodes named mode as v2 object", () => { + expect(serializeBinding({ mode: "named", chord: "meta+enter" })).toEqual({ + version: 2, + mode: "named", + chord: "meta+enter", + }); + }); + + it("canonicalizes the chord on serialize", () => { + expect(serializeBinding({ mode: "physical", chord: "shift+ctrl+k" })).toBe( + "ctrl+shift+k", + ); + }); + + it("round-trips legacy physical bindings unchanged", () => { + const legacy: string = "meta+shift+p"; + const round = serializeBinding(parseBinding(legacy)); + expect(round).toBe(legacy); + }); + + it("round-trips logical bindings as v2 objects", () => { + const v2 = { + version: 2 as const, + mode: "logical" as const, + chord: "meta+p", + }; + const round = serializeBinding(parseBinding(v2)); + expect(round).toEqual(v2); + }); +}); + +describe("translateLogicalChord", () => { + const usMap = new Map([ + ["KeyA", "a"], + ["KeyP", "p"], + ["KeyR", "r"], + ["KeyZ", "z"], + ["Slash", "/"], + ["Quote", "'"], + ["Semicolon", ";"], + ]); + // Dvorak: physical KeyR position prints "p", physical KeyP prints "l" + const dvorakMap = new Map([ + ["KeyA", "a"], + ["KeyP", "l"], + ["KeyR", "p"], + ["KeyZ", ";"], + ["Quote", "q"], + ]); + // QWERTZ: KeyY/KeyZ swapped, Slash → "-" + const qwertzMap = new Map([ + ["KeyY", "z"], + ["KeyZ", "y"], + ["Slash", "-"], + ]); + + it("returns null when layout map is unavailable", () => { + expect(translateLogicalChord("meta+p", null)).toBeNull(); + }); + + it("US layout: chord round-trips unchanged", () => { + expect(translateLogicalChord("meta+p", usMap)).toBe("meta+p"); + expect(translateLogicalChord("ctrl+shift+a", usMap)).toBe("ctrl+shift+a"); + }); + + it("Dvorak: meta+p (logical) translates to meta+r (physical R prints 'p')", () => { + expect(translateLogicalChord("meta+p", dvorakMap)).toBe("meta+r"); + }); + + it("QWERTZ: meta+y (logical) translates to meta+z (physical Z prints 'y')", () => { + expect(translateLogicalChord("meta+y", qwertzMap)).toBe("meta+z"); + }); + + it("translates punctuation aliases via their US glyph", () => { + // US: Slash prints "/" → no change + expect(translateLogicalChord("ctrl+slash", usMap)).toBe("ctrl+slash"); + // QWERTZ: Slash prints "-", but the binding wants the "/" character. + // On QWERTZ "/" is at Shift+7, not on a single key — so no scan code + // has unshifted glyph "/", returns null (caller falls back). + expect(translateLogicalChord("ctrl+slash", qwertzMap)).toBeNull(); + }); + + it("named keys (Enter, ArrowUp, F-keys) pass through unchanged", () => { + expect(translateLogicalChord("meta+enter", dvorakMap)).toBe("meta+enter"); + expect(translateLogicalChord("ctrl+arrowup", dvorakMap)).toBe( + "ctrl+arrowup", + ); + expect(translateLogicalChord("f5", dvorakMap)).toBe("f5"); + }); + + it("returns null when the produced character isn't on the keyboard", () => { + // Logical "meta+ñ" — no scan code in usMap has "ñ" as unshifted glyph + expect(translateLogicalChord("meta+ñ", usMap)).toBeNull(); + }); + + it("preserves modifier order from the input chord", () => { + // Verify modifiers stay in their input order; canonicalizeChord sorts them + const result = translateLogicalChord("alt+meta+shift+p", dvorakMap); + expect(result).toBe("alt+meta+shift+r"); + }); +}); + +describe("bindingToDispatchChord", () => { + const usMap = new Map([ + ["KeyP", "p"], + ["KeyR", "r"], + ["KeyY", "y"], + ["KeyZ", "z"], + ]); + // QWERTZ: Y/Z swapped + const qwertzMap = new Map([ + ["KeyY", "z"], + ["KeyZ", "y"], + ]); + + it("returns null for null binding", () => { + expect(bindingToDispatchChord(null, usMap)).toBeNull(); + }); + + it("legacy string (physical) returns chord unchanged", () => { + expect(bindingToDispatchChord("meta+p", usMap)).toBe("meta+p"); + expect(bindingToDispatchChord("meta+p", qwertzMap)).toBe("meta+p"); + }); + + it("explicit physical mode returns chord unchanged", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "physical", chord: "meta+p" }, + qwertzMap, + ), + ).toBe("meta+p"); + }); + + it("named mode returns chord unchanged", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "named", chord: "meta+enter" }, + qwertzMap, + ), + ).toBe("meta+enter"); + }); + + // The bug we just fixed: logical bindings recorded on QWERTZ were displayed + // as if their key were a scan-code token. Verify the translation flips the + // chord to the equivalent event.code form before display/dispatch. + it("logical binding on QWERTZ: meta+z translates to meta+y (KeyY prints 'z')", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "logical", chord: "meta+z" }, + qwertzMap, + ), + ).toBe("meta+y"); + }); + + it("logical binding on QWERTZ: meta+y translates to meta+z (KeyZ prints 'y')", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "logical", chord: "meta+y" }, + qwertzMap, + ), + ).toBe("meta+z"); + }); + + it("logical binding on US: identity (printed char == scan-code token)", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "logical", chord: "meta+z" }, + usMap, + ), + ).toBe("meta+z"); + }); + + it("logical binding falls back to literal chord when layoutMap missing", () => { + expect( + bindingToDispatchChord( + { version: 2, mode: "logical", chord: "meta+z" }, + null, + ), + ).toBe("meta+z"); + }); +}); + +describe("bindingsEqual", () => { + it("nulls match nulls", () => { + expect(bindingsEqual(null, null)).toBe(true); + expect(bindingsEqual(null, "meta+p")).toBe(false); + expect(bindingsEqual("meta+p", null)).toBe(false); + }); + + it("legacy string matches itself across modifier reorderings", () => { + expect(bindingsEqual("meta+shift+p", "shift+meta+p")).toBe(true); + }); + + it("legacy physical does NOT equal explicit logical with same chord", () => { + expect( + bindingsEqual("meta+p", { version: 2, mode: "logical", chord: "meta+p" }), + ).toBe(false); + }); + + it("legacy physical equals explicit physical with same chord", () => { + expect( + bindingsEqual("meta+p", { + version: 2, + mode: "physical", + chord: "meta+p", + }), + ).toBe(true); + }); + + it("two logical bindings with equivalent chords match", () => { + expect( + bindingsEqual( + { version: 2, mode: "logical", chord: "shift+meta+p" }, + { version: 2, mode: "logical", chord: "meta+shift+p" }, + ), + ).toBe(true); + }); +}); diff --git a/apps/desktop/src/renderer/hotkeys/utils/binding.ts b/apps/desktop/src/renderer/hotkeys/utils/binding.ts new file mode 100644 index 00000000000..f83ad421f81 --- /dev/null +++ b/apps/desktop/src/renderer/hotkeys/utils/binding.ts @@ -0,0 +1,133 @@ +import type { ParsedBinding, ShortcutBinding } from "../types"; +import { canonicalizeChord, normalizeToken } from "./resolveHotkeyFromEvent"; + +/** + * Keys whose `event.code` is stable across keyboard layouts (Enter, arrows, + * Backspace, ...). Tokens listed here are post-`normalizeToken` form — + * aliases like `esc` / `up` / `return` resolve to their canonical names + * (`escape`, `arrowup`, `enter`) before lookup, so this set must mirror the + * canonical side only. + */ +export const NAMED_KEYS = new Set([ + "enter", + "escape", + "backspace", + "delete", + "tab", + "space", + "arrowup", + "arrowdown", + "arrowleft", + "arrowright", + "home", + "end", + "pageup", + "pagedown", + "insert", +]); + +export function isFunctionKey(token: string): boolean { + return /^f([1-9]|1[0-2])$/.test(token); +} + +/** Mode used when promoting a legacy string binding (no explicit mode). */ +export function defaultModeForChord(chord: string): "physical" | "named" { + const parts = canonicalizeChord(chord).split("+"); + const key = parts[parts.length - 1]; + if (!key) return "physical"; + if (NAMED_KEYS.has(key) || isFunctionKey(key)) return "named"; + return "physical"; +} + +/** Normalize a stored binding (string or v2 object) into `{ mode, chord }`. */ +export function parseBinding(binding: ShortcutBinding): ParsedBinding { + if (typeof binding === "string") { + return { mode: defaultModeForChord(binding), chord: binding }; + } + return { mode: binding.mode, chord: binding.chord }; +} + +/** + * Compact storage form: physical → bare string (matches legacy storage and + * shipped registry defaults); logical / named → v2 object. + */ +export function serializeBinding(parsed: ParsedBinding): ShortcutBinding { + const chord = canonicalizeChord(parsed.chord); + if (parsed.mode === "physical") return chord; + return { version: 2, mode: parsed.mode, chord }; +} + +/** + * Resolve a binding to the event.code-form chord react-hotkeys-hook matches + * against. Logical bindings are translated through the current layout map + * (e.g. logical `meta+z` on QWERTZ becomes `meta+y` because the Z character + * lives on physical KeyY there). Single source of truth shared by useHotkey, + * useHotkeyDisplay, useFormatBinding, the conflict detector, and the + * terminal-forwarding reverse index. + */ +export function bindingToDispatchChord( + binding: ShortcutBinding | null, + layoutMap: ReadonlyMap | null, +): string | null { + if (!binding) return null; + const parsed = parseBinding(binding); + if (parsed.mode !== "logical") return parsed.chord; + return translateLogicalChord(parsed.chord, layoutMap) ?? parsed.chord; +} + +/** Two bindings refer to the same chord under the same matching semantics. */ +export function bindingsEqual( + a: ShortcutBinding | null, + b: ShortcutBinding | null, +): boolean { + if (a === null || b === null) return a === b; + const pa = parseBinding(a); + const pb = parseBinding(b); + return ( + pa.mode === pb.mode && + canonicalizeChord(pa.chord) === canonicalizeChord(pb.chord) + ); +} + +// Registry's canonical token form ("slash") → layout-map's unshifted glyph form ("/"). +const PUNCT_ALIAS_TO_GLYPH: Record = { + slash: "/", + backslash: "\\", + comma: ",", + period: ".", + semicolon: ";", + quote: "'", + backquote: "`", + minus: "-", + equal: "=", + bracketleft: "[", + bracketright: "]", +}; + +/** + * Translate a logical chord ("meta+p") into the equivalent event.code-based + * chord for the user's current layout. On US QWERTY: `meta+p` → `meta+p`. + * On Dvorak: `meta+p` → `meta+r` (physical KeyR prints "p"). Named/F-keys + * pass through unchanged. Returns null when the produced character isn't on + * the keyboard — caller falls back to the untranslated chord. + */ +export function translateLogicalChord( + chord: string, + layoutMap: ReadonlyMap | null, +): string | null { + if (!layoutMap) return null; + const canonical = canonicalizeChord(chord); + const parts = canonical.split("+"); + const key = parts[parts.length - 1]; + if (!key) return null; + if (NAMED_KEYS.has(key) || isFunctionKey(key)) return canonical; + + const targetGlyph = PUNCT_ALIAS_TO_GLYPH[key] ?? key; + for (const [scanCode, glyph] of layoutMap) { + if (glyph.toLowerCase() === targetGlyph.toLowerCase()) { + parts[parts.length - 1] = normalizeToken(scanCode); + return parts.join("+"); + } + } + return null; +} diff --git a/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts b/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts deleted file mode 100644 index e19117ccc0e..00000000000 --- a/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts +++ /dev/null @@ -1,48 +0,0 @@ -// Chromium's Keyboard Map API tells us how each physical key is labeled on -// the active layout. Used to gate the Mac Option dead-key rewrites in -// sanitizeOverride — on non-US layouts, Option+ produces different -// glyphs than US (e.g., German Option+Q = •), so our US-based rewrite table -// would produce wrong bindings. -// -// Fallback is optimistic (returns `true`) because: -// - `navigator.keyboard` is gated on secure contexts; packaged Electron -// renderers on file:// won't expose it, and we'd rather rewrite than drop -// for the common case. -// - Non-Mac users are unaffected either way (the dead-key glyphs aren't -// typeable at all, so detection is moot). - -interface KeyboardLayoutMap extends ReadonlyMap {} -interface Keyboard { - getLayoutMap?: () => Promise; -} - -let cached: Promise | null = null; - -export function isUSCompatibleLayout(): Promise { - if (cached) return cached; - cached = probe(); - return cached; -} - -async function probe(): Promise { - const keyboard = (navigator as Navigator & { keyboard?: Keyboard }).keyboard; - if (!keyboard?.getLayoutMap) return true; - try { - const map = await keyboard.getLayoutMap(); - return ( - map.get("KeyA") === "a" && - map.get("KeyQ") === "q" && - map.get("KeyW") === "w" && - map.get("KeyZ") === "z" && - map.get("Semicolon") === ";" && - map.get("Quote") === "'" - ); - } catch { - return true; - } -} - -// Exposed for tests — resets the cached probe result. -export function resetUSLayoutCacheForTests(): void { - cached = null; -} diff --git a/apps/desktop/src/renderer/hotkeys/utils/index.ts b/apps/desktop/src/renderer/hotkeys/utils/index.ts index 00b72d7c88c..de0c7b781bf 100644 --- a/apps/desktop/src/renderer/hotkeys/utils/index.ts +++ b/apps/desktop/src/renderer/hotkeys/utils/index.ts @@ -1,5 +1,13 @@ export { + bindingsEqual, + bindingToDispatchChord, + defaultModeForChord, + parseBinding, + serializeBinding, + translateLogicalChord, +} from "./binding"; +export { + isTerminalReservedEvent, matchesChord, resolveHotkeyFromEvent, } from "./resolveHotkeyFromEvent"; -export { isTerminalReservedEvent } from "./utils"; diff --git a/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts b/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts deleted file mode 100644 index 8b17835f767..00000000000 --- a/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts +++ /dev/null @@ -1,266 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { sanitizeOverride } from "./sanitizeOverride"; - -describe("sanitizeOverride — input shape", () => { - it("preserves an explicit unassignment (null)", () => { - expect(sanitizeOverride(null)).toBeNull(); - }); - - it("drops empty / non-string values", () => { - expect(sanitizeOverride(undefined)).toBeUndefined(); - expect(sanitizeOverride("")).toBeUndefined(); - expect(sanitizeOverride(" ")).toBeUndefined(); - expect(sanitizeOverride(42)).toBeUndefined(); - expect(sanitizeOverride({})).toBeUndefined(); - }); - - it("drops chords with only modifiers", () => { - expect(sanitizeOverride("ctrl+shift")).toBeUndefined(); - expect(sanitizeOverride("meta")).toBeUndefined(); - }); - - it("drops pre-fix `ctrl+control` garbage (no real key)", () => { - expect(sanitizeOverride("ctrl+control")).toBeUndefined(); - }); -}); - -describe("sanitizeOverride — identity / canonicalization (already-v2 chords)", () => { - it("single letter / digit with modifier", () => { - expect(sanitizeOverride("meta+k")).toBe("meta+k"); - expect(sanitizeOverride("meta+1")).toBe("meta+1"); - expect(sanitizeOverride("meta+d")).toBe("meta+d"); - expect(sanitizeOverride("ctrl+i")).toBe("ctrl+i"); - }); - - it("reorders modifiers lexically", () => { - expect(sanitizeOverride("shift+ctrl+k")).toBe("ctrl+shift+k"); - expect(sanitizeOverride("meta+alt+shift+1")).toBe("alt+meta+shift+1"); - }); - - it("aliases arrow tokens to arrow form", () => { - expect(sanitizeOverride("meta+alt+up")).toBe("alt+meta+arrowup"); - expect(sanitizeOverride("meta+alt+left")).toBe("alt+meta+arrowleft"); - }); - - it("accepts multi-char event.code-style key names", () => { - expect(sanitizeOverride("meta+bracketleft")).toBe("meta+bracketleft"); - expect(sanitizeOverride("alt+bracketleft")).toBe("alt+bracketleft"); - expect(sanitizeOverride("meta+slash")).toBe("meta+slash"); - expect(sanitizeOverride("meta+semicolon")).toBe("meta+semicolon"); - expect(sanitizeOverride("meta+shift+semicolon")).toBe( - "meta+shift+semicolon", - ); - expect(sanitizeOverride("meta+quote")).toBe("meta+quote"); - expect(sanitizeOverride("meta+minus")).toBe("meta+minus"); - expect(sanitizeOverride("meta+end")).toBe("meta+end"); - expect(sanitizeOverride("meta+pagedown")).toBe("meta+pagedown"); - }); - - it("accepts function keys F1–F12", () => { - expect(sanitizeOverride("f1")).toBe("f1"); - expect(sanitizeOverride("f2")).toBe("f2"); - expect(sanitizeOverride("f10")).toBe("f10"); - expect(sanitizeOverride("f12")).toBe("f12"); - expect(sanitizeOverride("meta+f1")).toBe("meta+f1"); - }); -}); - -describe("sanitizeOverride — v1 punctuation rewrite (unshifted)", () => { - it("rewrites comma / period / semicolon / quote / backquote", () => { - expect(sanitizeOverride("meta+,")).toBe("meta+comma"); - expect(sanitizeOverride("meta+.")).toBe("meta+period"); - expect(sanitizeOverride("meta+;")).toBe("meta+semicolon"); - expect(sanitizeOverride("ctrl+'")).toBe("ctrl+quote"); - expect(sanitizeOverride("meta+`")).toBe("meta+backquote"); - }); - - it("rewrites minus / equal", () => { - expect(sanitizeOverride("ctrl+-")).toBe("ctrl+minus"); - expect(sanitizeOverride("meta+-")).toBe("meta+minus"); - expect(sanitizeOverride("meta+=")).toBe("meta+equal"); - }); - - it("rewrites brackets / backslash", () => { - expect(sanitizeOverride("meta+[")).toBe("meta+bracketleft"); - expect(sanitizeOverride("meta+]")).toBe("meta+bracketright"); - expect(sanitizeOverride("meta+\\")).toBe("meta+backslash"); - }); -}); - -describe("sanitizeOverride — v1 shifted-char recovery (US QWERTY)", () => { - it("recovers shifted digits", () => { - expect(sanitizeOverride("ctrl+shift+!")).toBe("ctrl+shift+1"); - expect(sanitizeOverride("meta+ctrl+shift+@")).toBe("ctrl+meta+shift+2"); - expect(sanitizeOverride("meta+shift+#")).toBe("meta+shift+3"); - expect(sanitizeOverride("meta+shift+$")).toBe("meta+shift+4"); - expect(sanitizeOverride("meta+shift+%")).toBe("meta+shift+5"); - expect(sanitizeOverride("meta+shift+^")).toBe("meta+shift+6"); - expect(sanitizeOverride("meta+shift+&")).toBe("meta+shift+7"); - expect(sanitizeOverride("meta+shift+*")).toBe("meta+shift+8"); - expect(sanitizeOverride("meta+shift+(")).toBe("meta+shift+9"); - expect(sanitizeOverride("meta+shift+)")).toBe("meta+shift+0"); - }); - - it("recovers shifted punctuation", () => { - expect(sanitizeOverride("meta+shift+_")).toBe("meta+shift+minus"); - expect(sanitizeOverride("meta+shift+{")).toBe("meta+shift+bracketleft"); - expect(sanitizeOverride("meta+shift+}")).toBe("meta+shift+bracketright"); - expect(sanitizeOverride("meta+shift+|")).toBe("meta+shift+backslash"); - expect(sanitizeOverride("meta+shift+:")).toBe("meta+shift+semicolon"); - expect(sanitizeOverride('meta+shift+"')).toBe("meta+shift+quote"); - expect(sanitizeOverride("meta+shift+<")).toBe("meta+shift+comma"); - expect(sanitizeOverride("meta+shift+>")).toBe("meta+shift+period"); - expect(sanitizeOverride("meta+shift+?")).toBe("meta+shift+slash"); - expect(sanitizeOverride("meta+shift+~")).toBe("meta+shift+backquote"); - }); -}); - -describe("sanitizeOverride — numpad-minus hardware edge case", () => { - // On external keyboards, Shift+NumpadSubtract produces event.key === "-" - // (shift doesn't affect numpad), so v1 stored "meta+shift+-" instead of - // the expected "meta+shift+_". Both shapes must converge to the same v2 - // chord so the user's binding survives. - it("maps shift + unshifted minus to shift+minus", () => { - expect(sanitizeOverride("meta+shift+-")).toBe("meta+shift+minus"); - }); - - it("maps shift + shifted minus (_) to shift+minus (same target)", () => { - expect(sanitizeOverride("meta+shift+_")).toBe("meta+shift+minus"); - }); -}); - -describe("sanitizeOverride — macOS Option dead-key recovery", () => { - // Real data: v1 stored Option+ as the resulting glyph - // because its encoder used event.key. Those chars aren't typeable - // without Option on any layout, so we can safely map them back. - it("recovers Option + digit row glyphs", () => { - expect(sanitizeOverride("meta+alt+¡")).toBe("alt+meta+1"); - expect(sanitizeOverride("meta+alt+™")).toBe("alt+meta+2"); - expect(sanitizeOverride("meta+alt+£")).toBe("alt+meta+3"); - expect(sanitizeOverride("meta+alt+¢")).toBe("alt+meta+4"); - expect(sanitizeOverride("meta+alt+∞")).toBe("alt+meta+5"); - expect(sanitizeOverride("meta+alt+§")).toBe("alt+meta+6"); - expect(sanitizeOverride("meta+alt+¶")).toBe("alt+meta+7"); - expect(sanitizeOverride("meta+alt+•")).toBe("alt+meta+8"); - expect(sanitizeOverride("meta+alt+ª")).toBe("alt+meta+9"); - expect(sanitizeOverride("meta+alt+º")).toBe("alt+meta+0"); - }); - - it("recovers Option + letter glyphs", () => { - expect(sanitizeOverride("meta+alt+å")).toBe("alt+meta+a"); - expect(sanitizeOverride("meta+alt+∂")).toBe("alt+meta+d"); - expect(sanitizeOverride("meta+alt+ç")).toBe("alt+meta+c"); - expect(sanitizeOverride("alt+¬")).toBe("alt+l"); - expect(sanitizeOverride("ctrl+ß")).toBe("ctrl+s"); - expect(sanitizeOverride("meta+alt+Ω")).toBe("alt+meta+z"); - }); -}); - -describe("sanitizeOverride — non-US Mac layout opts out of dead-key recovery", () => { - // On non-US Mac (German, French, etc.), Option+ produces - // different glyphs than US, so our US-based dead-key table can't be - // trusted. Migrator passes assumeUSMacLayout=false and we drop rather - // than silently rebind to the wrong physical key. - const nonUS = { assumeUSMacLayout: false }; - - it("drops Mac Option digit-row glyphs", () => { - expect(sanitizeOverride("meta+alt+ª", nonUS)).toBeUndefined(); - expect(sanitizeOverride("meta+alt+™", nonUS)).toBeUndefined(); - expect(sanitizeOverride("meta+alt+¡", nonUS)).toBeUndefined(); - }); - - it("drops Mac Option letter glyphs", () => { - expect(sanitizeOverride("meta+alt+å", nonUS)).toBeUndefined(); - expect(sanitizeOverride("meta+alt+∂", nonUS)).toBeUndefined(); - expect(sanitizeOverride("alt+¬", nonUS)).toBeUndefined(); - }); - - it("still rewrites ASCII punctuation and shifted glyphs", () => { - // Layout-gate only affects Mac Option dead keys — punctuation - // rewrites are always-on. - expect(sanitizeOverride("meta+,", nonUS)).toBe("meta+comma"); - expect(sanitizeOverride("meta+ctrl+shift+@", nonUS)).toBe( - "ctrl+meta+shift+2", - ); - expect(sanitizeOverride("meta+shift+-", nonUS)).toBe("meta+shift+minus"); - }); - - it("still canonicalizes already-v2 chords", () => { - expect(sanitizeOverride("meta+alt+up", nonUS)).toBe("alt+meta+arrowup"); - expect(sanitizeOverride("f1", nonUS)).toBe("f1"); - expect(sanitizeOverride("meta+bracketleft", nonUS)).toBe( - "meta+bracketleft", - ); - }); -}); - -describe("sanitizeOverride — best-effort drops (intractable)", () => { - it("drops corrupt chords whose key was literal `+` (separator collision)", () => { - // v1 stored Shift+Equal as "shift++", which splits into empty tokens. - expect(sanitizeOverride("meta+shift++")).toBeUndefined(); - expect(sanitizeOverride("meta++")).toBeUndefined(); - }); - - it("drops unknown non-ASCII glyphs (non-US layouts we can't guess)", () => { - expect(sanitizeOverride("meta+alt+ü")).toBeUndefined(); - expect(sanitizeOverride("ctrl+é")).toBeUndefined(); - }); -}); - -describe("sanitizeOverride — real captured blobs (integration smoke)", () => { - // Every override that appeared in the three leveldb / app-state dumps - // we scanned (indigo-pentaceratops v1.4.7, tray-polling-fix, and the - // current hotkeys-fixes branch). Locks in the 90% best-effort path. - const cases: Array<[string, string | null]> = [ - // --- indigo-pentaceratops (v1.4.7, event.key style) --- - ["meta+2", "meta+2"], - ["meta+,", "meta+comma"], - ["meta+;", "meta+semicolon"], - ["ctrl+'", "ctrl+quote"], - ["ctrl+-", "ctrl+minus"], - ["meta+ctrl+shift+@", "ctrl+meta+shift+2"], - ["meta+ctrl+shift+n", "ctrl+meta+shift+n"], - ["meta+shift+n", "meta+shift+n"], - ["meta+d", "meta+d"], - ["meta+1", "meta+1"], - ["meta+alt+left", "alt+meta+arrowleft"], - ["meta+-", "meta+minus"], - ["meta+shift+-", "meta+shift+minus"], - ["meta+shift+slash", "meta+shift+slash"], - // Mac Option dead-key chars captured in real data - ["meta+alt+ª", "alt+meta+9"], - ["meta+alt+å", "alt+meta+a"], - ["meta+alt+™", "alt+meta+2"], - // --- tray-polling-fix (v2 new format, variety of named keys) --- - ["meta+alt+1", "alt+meta+1"], - ["meta+alt+equal", "alt+meta+equal"], - ["meta+minus", "meta+minus"], - ["meta+slash", "meta+slash"], - ["meta+semicolon", "meta+semicolon"], - ["meta+shift+semicolon", "meta+shift+semicolon"], - ["meta+quote", "meta+quote"], - ["meta+end", "meta+end"], - ["meta+pagedown", "meta+pagedown"], - ["meta+s", "meta+s"], - ["ctrl+i", "ctrl+i"], - ["ctrl+1", "ctrl+1"], - // --- hotkeys-fixes (current branch, function-key & modifier mixes) --- - ["alt+shift+2", "alt+shift+2"], - ["meta+9", "meta+9"], - ["f1", "f1"], - ["f2", "f2"], - ["f10", "f10"], - ["meta+f1", "meta+f1"], - ["alt+3", "alt+3"], - ["alt+shift+5", "alt+shift+5"], - ["meta+alt+shift+1", "alt+meta+shift+1"], - ["alt+bracketleft", "alt+bracketleft"], - ["alt+shift+9", "alt+shift+9"], - ]; - - for (const [input, expected] of cases) { - it(`${input} -> ${expected}`, () => { - expect(sanitizeOverride(input)).toBe(expected); - }); - } -}); diff --git a/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.test.ts b/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.test.ts index 237c328e983..d46ee4154ed 100644 --- a/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.test.ts +++ b/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.test.ts @@ -1,17 +1,17 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { HOTKEYS, type HotkeyId } from "../registry"; import { useHotkeyOverridesStore } from "../stores/hotkeyOverridesStore"; -import type { HotkeyDefinition } from "../types"; +import type { HotkeyDefinition, ShortcutBinding } from "../types"; import { canonicalizeChord, eventToChord, isIgnorableKey, + isTerminalReservedEvent, matchesChord, normalizeToken, resolveHotkeyFromEvent, TERMINAL_RESERVED_CHORDS, } from "./resolveHotkeyFromEvent"; -import { isTerminalReservedEvent } from "./utils"; // Minimal stub — the renderer references `navigator` only at import time. // Bun's test runtime doesn't have a DOM navigator by default; registry.ts @@ -126,6 +126,9 @@ interface StubInit { metaKey?: boolean; altKey?: boolean; shiftKey?: boolean; + altGraph?: boolean; + isComposing?: boolean; + keyCode?: number; } function ev(init: StubInit): KeyboardEvent { return { @@ -136,11 +139,15 @@ function ev(init: StubInit): KeyboardEvent { metaKey: !!init.metaKey, altKey: !!init.altKey, shiftKey: !!init.shiftKey, + isComposing: !!init.isComposing, + keyCode: init.keyCode ?? 0, + getModifierState: (mod: string) => + mod === "AltGraph" ? !!init.altGraph : false, } as unknown as KeyboardEvent; } describe("resolveHotkeyFromEvent — live override index", () => { - let originalOverrides: Record; + let originalOverrides: Record; beforeEach(() => { originalOverrides = useHotkeyOverridesStore.getState().overrides; }); @@ -297,6 +304,57 @@ describe("eventToChord", () => { expect(eventToChord(ev({ code: "ControlLeft", ctrlKey: true }))).toBeNull(); expect(eventToChord(ev({ code: "CapsLock" }))).toBeNull(); }); + + // AltGr on Linux/Windows is reported as ctrlKey+altKey. Without the guard, + // AltGr+E on a German layout (which produces €) would match a US + // `ctrl+alt+e` binding. Suppress both when AltGraph is set so AltGr-typed + // printables can never trigger Ctrl+Alt hotkeys. + it("suppresses ctrl/alt when AltGraph modifier is held", () => { + expect( + eventToChord( + ev({ + code: "KeyE", + ctrlKey: true, + altKey: true, + altGraph: true, + }), + ), + ).toBe("e"); + }); + + it("AltGr+letter does not match a real ctrl+alt binding", () => { + const altGrEvent = ev({ + code: "KeyE", + ctrlKey: true, + altKey: true, + altGraph: true, + }); + expect(matchesChord(altGrEvent, "ctrl+alt+e")).toBe(false); + }); + + it("real Ctrl+Alt (no AltGraph) still matches", () => { + const realCtrlAlt = ev({ + code: "KeyE", + ctrlKey: true, + altKey: true, + altGraph: false, + }); + expect(matchesChord(realCtrlAlt, "ctrl+alt+e")).toBe(true); + }); + + // IME composition: keydown during dead-key / CJK composition must not fire + // hotkeys. Safari uses keyCode 229 in lieu of isComposing. + it("returns null during IME composition (isComposing)", () => { + expect( + eventToChord(ev({ code: "KeyA", metaKey: true, isComposing: true })), + ).toBeNull(); + }); + + it("returns null when keyCode is 229 (Safari IME)", () => { + expect( + eventToChord(ev({ code: "KeyA", metaKey: true, keyCode: 229 })), + ).toBeNull(); + }); }); describe("matchesChord", () => { diff --git a/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts b/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts index 3cf565379d5..42a4585426c 100644 --- a/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts +++ b/apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts @@ -1,5 +1,8 @@ import { HOTKEYS, type HotkeyId } from "../registry"; import { useHotkeyOverridesStore } from "../stores/hotkeyOverridesStore"; +import { useKeyboardLayoutStore } from "../stores/keyboardLayoutStore"; +import type { ShortcutBinding } from "../types"; +import { bindingToDispatchChord } from "./binding"; /** * KeyboardEvent → registered {@link HotkeyId}, or `null` if unbound. Uses the @@ -70,12 +73,21 @@ export function canonicalizeChord(chord: string): string { /** KeyboardEvent → canonical chord (comparable to {@link canonicalizeChord} output), or null for pure modifier / synthetic presses. */ export function eventToChord(event: KeyboardEvent): string | null { if (event.code === undefined) return null; + // IME composition: keydown during CJK / dead-key composition must not + // trigger hotkeys. Safari reports keyCode 229 instead of isComposing. + if (event.isComposing || event.keyCode === 229) return null; const key = normalizeToken(event.code); if (isIgnorableKey(key)) return null; + // AltGr is reported by Chromium as ctrlKey+altKey on Windows/Linux. + // Treating that combination as Ctrl+Alt would let printable keystrokes on + // non-US layouts (e.g. AltGr+E = € on German) accidentally trigger + // ctrl+alt+e bindings. Suppress both when AltGr is held; no binding opts + // into AltGr explicitly. + const altGraph = event.getModifierState?.("AltGraph") === true; const mods: string[] = []; if (event.metaKey) mods.push("meta"); - if (event.ctrlKey) mods.push("ctrl"); - if (event.altKey) mods.push("alt"); + if (event.ctrlKey && !altGraph) mods.push("ctrl"); + if (event.altKey && !altGraph) mods.push("alt"); if (event.shiftKey) mods.push("shift"); mods.sort(); return [...mods, key].join("+"); @@ -95,8 +107,16 @@ export const TERMINAL_RESERVED_CHORDS = new Set( ), ); +/** True if the event matches a chord the terminal must always receive. */ +export function isTerminalReservedEvent(event: KeyboardEvent): boolean { + const chord = eventToChord(event); + if (!chord) return false; + return TERMINAL_RESERVED_CHORDS.has(chord); +} + function buildRegisteredAppChords( - overrides: Record, + overrides: Record, + layoutMap: ReadonlyMap | null, ): Map { const map = new Map(); for (const id of Object.keys(HOTKEYS) as HotkeyId[]) { @@ -105,18 +125,30 @@ function buildRegisteredAppChords( // Explicit unassignment (null override) must drop from the index — else // the terminal's isAppHotkey check would swallow the freed chord. if (hasOverride && override === null) continue; - const keys = override ?? HOTKEYS[id].key; - if (!keys) continue; - map.set(canonicalizeChord(keys), id); + const binding = override ?? HOTKEYS[id].key; + if (!binding) continue; + const dispatchChord = bindingToDispatchChord(binding, layoutMap); + if (!dispatchChord) continue; + map.set(canonicalizeChord(dispatchChord), id); } return map; } -// Reassigned on each override-store change; `let` is required so the -// subscribe callback can replace the reference the resolver reads. +// Reassigned on each override OR layout change; `let` is required so the +// subscribe callbacks can replace the reference the resolver reads. let registeredAppChords = buildRegisteredAppChords( useHotkeyOverridesStore.getState().overrides, + useKeyboardLayoutStore.getState().map, ); useHotkeyOverridesStore.subscribe((state) => { - registeredAppChords = buildRegisteredAppChords(state.overrides); + registeredAppChords = buildRegisteredAppChords( + state.overrides, + useKeyboardLayoutStore.getState().map, + ); +}); +useKeyboardLayoutStore.subscribe((state) => { + registeredAppChords = buildRegisteredAppChords( + useHotkeyOverridesStore.getState().overrides, + state.map, + ); }); diff --git a/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts b/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts deleted file mode 100644 index 8993ab9f589..00000000000 --- a/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { canonicalizeChord, MODIFIERS } from "./resolveHotkeyFromEvent"; - -// v1 stored tokens via event.key; v2 matches via event.code. These maps -// rewrite raw glyphs to v2's code names where the mapping is unambiguous. - -// Always safe: US-ANSI punctuation + shifted glyphs. The `event.code` -// position of these keys is consistent across Latin layouts (AZERTY is the -// edge case — rare enough to accept the occasional drop). -const ALWAYS_SAFE_REWRITES: Record = { - ",": "comma", - ".": "period", - ";": "semicolon", - "'": "quote", - "`": "backquote", - "-": "minus", - "=": "equal", - "[": "bracketleft", - "]": "bracketright", - "\\": "backslash", - "/": "slash", - "!": "1", - "@": "2", - "#": "3", - $: "4", - "%": "5", - "^": "6", - "&": "7", - "*": "8", - "(": "9", - ")": "0", - _: "minus", - "{": "bracketleft", - "}": "bracketright", - "|": "backslash", - ":": "semicolon", - '"': "quote", - "<": "comma", - ">": "period", - "?": "slash", - "~": "backquote", -}; - -// macOS Option+ glyphs on **US** layout. On German Mac, -// Option+Q produces `•` (which US maps to Option+8) — same glyph, different -// physical key. Gated behind a US-layout detection in migrate.ts to avoid -// silently rewriting bindings to the wrong physical key on non-US Macs. -const MAC_US_DEAD_KEYS: Record = { - "¡": "1", - "™": "2", - "£": "3", - "¢": "4", - "∞": "5", - "§": "6", - "¶": "7", - "•": "8", - ª: "9", - º: "0", - å: "a", - "∫": "b", - ç: "c", - "∂": "d", - ƒ: "f", - "©": "g", - "˙": "h", - "∆": "j", - "˚": "k", - "¬": "l", - µ: "m", - ø: "o", - π: "p", - œ: "q", - "®": "r", - ß: "s", - "†": "t", - "√": "v", - "∑": "w", - "≈": "x", - "¥": "y", - Ω: "z", -}; - -export interface SanitizeOverrideOptions { - /** Apply US-Mac Option dead-key rewrites. Caller should pass `false` when - * the current keyboard layout is not US-compatible. Default `true`. */ - assumeUSMacLayout?: boolean; -} - -/** - * Validates a migrated override string. Drops pre-fix garbage - * (`ctrl+control`, modifier-only chords, unknown non-ASCII glyphs) that the - * old recorder could produce and that would never match `event.code`-based - * dispatch. - * - * - Returns the canonicalized chord on success. - * - Returns `null` to preserve an explicit unassignment. - * - Returns `undefined` to signal the caller should drop the entry. - */ -export function sanitizeOverride( - value: unknown, - options: SanitizeOverrideOptions = {}, -): string | null | undefined { - if (value === null) return null; - if (typeof value !== "string" || !value.trim()) return undefined; - const { assumeUSMacLayout = true } = options; - const rewritten = value - .split("+") - .map((part) => { - const safe = ALWAYS_SAFE_REWRITES[part]; - if (safe) return safe; - if (assumeUSMacLayout) { - const deadKey = MAC_US_DEAD_KEYS[part]; - if (deadKey) return deadKey; - } - return part; - }) - .join("+"); - const canonical = canonicalizeChord(rewritten); - const keys = canonical.split("+").filter((p) => !MODIFIERS.has(p)); - if (keys.length !== 1 || !/^[a-z0-9]+$/.test(keys[0])) return undefined; - return canonical; -} diff --git a/apps/desktop/src/renderer/hotkeys/utils/utils.ts b/apps/desktop/src/renderer/hotkeys/utils/utils.ts deleted file mode 100644 index dd5628a3478..00000000000 --- a/apps/desktop/src/renderer/hotkeys/utils/utils.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { - eventToChord, - TERMINAL_RESERVED_CHORDS, -} from "./resolveHotkeyFromEvent"; - -/** True if the event matches a chord the terminal must always receive. */ -export function isTerminalReservedEvent(event: KeyboardEvent): boolean { - const chord = eventToChord(event); - if (!chord) return false; - return TERMINAL_RESERVED_CHORDS.has(chord); -} diff --git a/apps/desktop/src/renderer/lib/terminal/line-edit-translations.ts b/apps/desktop/src/renderer/lib/terminal/line-edit-translations.ts index 7753993fdc9..b6806fe0100 100644 --- a/apps/desktop/src/renderer/lib/terminal/line-edit-translations.ts +++ b/apps/desktop/src/renderer/lib/terminal/line-edit-translations.ts @@ -17,6 +17,12 @@ function onlyMod(event: KeyboardEvent, mod: "meta" | "alt" | "ctrl"): boolean { * Translate Mac Cmd+/Option+ and Windows Ctrl+ arrow / backspace chords into * the escape sequences shells expect. Returns the bytes to send, or null if * this chord isn't a line-edit translation. + * + * CONTRACT: only check `event.key` for stable named keys (Backspace, + * ArrowLeft/Right, Home, End, ...). Never `event.key` for printable + * characters — those vary by layout (`event.key === "p"` on QWERTY is `"r"` + * on Dvorak) and silently break non-US users. Use `event.code` via + * `resolveHotkeyFromEvent` for any printable-key translation. */ export function translateLineEditChord( event: KeyboardEvent, diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SearchBarTrigger/SearchBarTrigger.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SearchBarTrigger/SearchBarTrigger.tsx index 6d461bba9b4..c27b05fce7a 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SearchBarTrigger/SearchBarTrigger.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SearchBarTrigger/SearchBarTrigger.tsx @@ -1,7 +1,7 @@ import { Kbd, KbdGroup } from "@superset/ui/kbd"; import { useCallback } from "react"; import { LuSearch } from "react-icons/lu"; -import { getBinding, useHotkeyDisplay } from "renderer/hotkeys"; +import { getDispatchChord, useHotkeyDisplay } from "renderer/hotkeys"; interface SearchBarTriggerProps { workspaceName?: string; @@ -40,10 +40,8 @@ export function SearchBarTrigger({ workspaceName }: SearchBarTriggerProps) { const isUnassigned = display.length === 1 && display[0] === "Unassigned"; const handleClick = useCallback(() => { - const keys = getBinding("QUICK_OPEN"); - if (keys) { - dispatchHotkeyEvent(keys); - } + const chord = getDispatchChord("QUICK_OPEN"); + if (chord) dispatchHotkeyEvent(chord); }, []); const fullPlaceholder = workspaceName diff --git a/apps/desktop/src/renderer/routes/_authenticated/layout.tsx b/apps/desktop/src/renderer/routes/_authenticated/layout.tsx index ff5732698e8..6159bbdd648 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/layout.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/layout.tsx @@ -17,7 +17,6 @@ import { useUpdateListener } from "renderer/components/UpdateToast"; import { env } from "renderer/env.renderer"; import { useIsV2CloudEnabled } from "renderer/hooks/useIsV2CloudEnabled"; import { useOnlineStatus } from "renderer/hooks/useOnlineStatus"; -import { migrateHotkeyOverrides } from "renderer/hotkeys/migrate"; import { authClient, getAuthToken } from "renderer/lib/auth-client"; import { dragDropManager } from "renderer/lib/dnd"; import { electronTrpc } from "renderer/lib/electron-trpc"; @@ -70,13 +69,6 @@ function AuthenticatedLayout() { useAgentHookListener(); useUpdateListener(); - // One-time migration from old hotkey storage to new localStorage-based store - useEffect(() => { - void migrateHotkeyOverrides().catch((error) => { - console.error("[hotkeys] Migration failed:", error); - }); - }, []); - // Update workspace-run pane state on terminal exit electronTrpc.notifications.subscribe.useSubscription(undefined, { onData: (event) => { diff --git a/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx index 503bcaf75c1..1d1cc0bd511 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx @@ -14,11 +14,11 @@ import { createFileRoute } from "@tanstack/react-router"; import { useMemo, useState } from "react"; import { HiMagnifyingGlass } from "react-icons/hi2"; import { - formatHotkeyDisplay, HOTKEYS, type HotkeyCategory, type HotkeyId, - PLATFORM, + type ShortcutBinding, + useFormatBinding, useHotkeyDisplay, useHotkeyOverridesStore, useRecordHotkeys, @@ -118,7 +118,7 @@ function KeyboardShortcutsPage() { const [recordingId, setRecordingId] = useState(null); const [pendingConflict, setPendingConflict] = useState<{ targetId: HotkeyId; - keys: string; + binding: ShortcutBinding; conflictId: HotkeyId; } | null>(null); @@ -127,14 +127,18 @@ function KeyboardShortcutsPage() { const setOverride = useHotkeyOverridesStore((s) => s.setOverride); useRecordHotkeys(recordingId, { + // New printable bindings follow the printed character (matches what the + // user sees on their keyboard). F-keys / named keys are forced to + // "named" by the recorder regardless of this preference. + preferredMode: "logical", onSave: () => setRecordingId(null), onCancel: () => setRecordingId(null), onUnassign: () => setRecordingId(null), - onConflict: (targetId, keys, conflictId) => { - setPendingConflict({ targetId, keys, conflictId }); + onConflict: (targetId, binding, conflictId) => { + setPendingConflict({ targetId, binding, conflictId }); setRecordingId(null); }, - onReserved: (_keys, info) => { + onReserved: (_binding, info) => { if (info.severity === "error") { toast.error(info.reason); setRecordingId(null); @@ -166,10 +170,12 @@ function KeyboardShortcutsPage() { const handleConflictReassign = () => { if (!pendingConflict) return; setOverride(pendingConflict.conflictId, null); - setOverride(pendingConflict.targetId, pendingConflict.keys); + setOverride(pendingConflict.targetId, pendingConflict.binding); setPendingConflict(null); }; + const conflictDisplay = useFormatBinding(pendingConflict?.binding ?? null); + return (
{/* Header */} @@ -278,7 +284,7 @@ function KeyboardShortcutsPage() {
{pendingConflict - ? `${formatHotkeyDisplay(pendingConflict.keys, PLATFORM).text} is already assigned to "${ + ? `${conflictDisplay.text} is already assigned to "${ HOTKEYS[pendingConflict.conflictId].label }".` : ""} diff --git a/bun.lock b/bun.lock index ed538233849..7e7ced2b40c 100644 --- a/bun.lock +++ b/bun.lock @@ -110,7 +110,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.6.1", + "version": "1.6.2", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36", @@ -265,6 +265,7 @@ "lucide-react": "^0.563.0", "mastracode": "0.15.0-alpha.3", "nanoid": "^5.1.6", + "native-keymap": "^3.3.9", "node-addon-api": "^7.1.0", "node-pty": "1.1.0", "os-locale": "^6.0.2", @@ -4824,6 +4825,8 @@ "napi-build-utils": ["napi-build-utils@2.0.0", "", {}, "sha512-GEbrYkbfF7MoNaoh2iGG84Mnf/WZfB0GdGEsM8wz7Expx/LlWf5U8t9nvJKXSp3qr5IsEbK04cBGhol/KwOsWA=="], + "native-keymap": ["native-keymap@3.3.9", "", {}, "sha512-d/ydQ5x+GM5W0dyAjFPwexhtc9CDH1g/xWZESS5CXk16ThyFzSBLvlBJq1+FyzUIFf/F2g1MaHdOpa6G9150YQ=="], + "needle": ["needle@2.9.1", "", { "dependencies": { "debug": "^3.2.6", "iconv-lite": "^0.4.4", "sax": "^1.2.4" }, "bin": { "needle": "./bin/needle" } }, "sha512-6R9fqJ5Zcmf+uYaFgdIHmLwNldn5HbK8L5ybn7Uz+ylX/rnOsSp1AHcvQSrCaFN+qNM1wpymHqD7mVasEOlHGQ=="], "negotiator": ["negotiator@1.0.0", "", {}, "sha512-8Ofs/AUQh8MaEcrlq5xOX0CQ9ypTF5dl78mjlMNfOK08fzpgTHQRQPBxcPlEtIw0yRpws+Zo/3r+5WRby7u3Gg=="],