Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions apps/desktop/plans/20260505-hotkey-adaptive-toggle-smoke-test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
# Hotkey adaptive-layout toggle — manual smoke test

Verifies the full adaptive-layout pipeline after this branch:

- **A.** Toggle gates *every* dispatch consumer (registration, resolver,
display, recorder conflict detector, imperative `getDispatchChord`).
- **B.** Shipped defaults are authored as `mode: "logical"`, so the toggle
actually moves them on non-US layouts (previously it was a no-op for
defaults because they were physical strings).
- **C.** `adaptiveLayoutEnabled` defaults to **true** for new installs.
Matches macOS / VS Code / Chrome convention: `⌘Z` fires on the key
labeled "Z" regardless of layout.

The original PR (#4078) wired the toggle through the resolver and display
hooks but missed the registration hook and the recorder; even with that
fixed, defaults were stored as physical strings so the toggle had no
effect on shipped bindings. This branch closes both gaps.

## Setup

1. Build & launch the desktop app: `cd apps/desktop && bun dev`.
2. Open DevTools (View → Toggle Developer Tools).
3. Run `localStorage.removeItem("keyboard-preferences")` and reload —
guarantees you start on the **new default (toggle ON)**.

You'll need a non-US layout to exercise the bug. macOS:
System Settings → Keyboard → Input Sources → add **German – Standard
(QWERTZ)**. Switch via the menu-bar flag or Ctrl+Space.

QWERTZ swap to keep in mind: physical KeyZ prints **Y**, physical KeyY
prints **Z**.

## A. Toggle ON (new default) — labels rule

> Expected: every shortcut fires on the key whose **printed label**
> matches the binding, regardless of physical position.

1. Switch input source to **German (QWERTZ)**.
2. In the app:
- Press `⌘Z` (the key labeled "Z" — physical KeyY) → bound `UNDO`-style
hotkey fires. Pressing the key labeled "Y" (physical KeyZ) does
**not** fire it.
- Press `⌘P` (Open Command Palette) — opens. The labeled-P key works.
- Settings → Keyboard: glyphs reflect QWERTZ (the printed character on
the current layout).
3. Open the recorder for any shortcut, press `⌘Z` (labeled-Z key). The
captured chord shows `⌘Z`. Saving doesn't flag a phantom conflict.
4. In DevTools console, dispatch synthesized events:
```js
// physical KeyY (labeled Z on QWERTZ) — should fire
document.activeElement.dispatchEvent(
new KeyboardEvent("keydown", { code: "KeyY", metaKey: true, bubbles: true })
);
// physical KeyZ (labeled Y on QWERTZ) — should NOT fire
document.activeElement.dispatchEvent(
new KeyboardEvent("keydown", { code: "KeyZ", metaKey: true, bubbles: true })
);
```

## B. Toggle OFF — positions rule

1. Settings → Keyboard → flip **Adaptive layout mapping** off.
2. Still on QWERTZ:
- Press the key labeled "Y" (physical KeyZ) with ⌘ → `UNDO` fires.
- Press the key labeled "Z" (physical KeyY) with ⌘ → does NOT fire.
- Settings → Keyboard glyphs render exactly as authored ("⌘Z" stays
"⌘Z"; no QWERTZ remap).
3. Recorder: press the key labeled "Y" (physical KeyZ) + ⌘ → captured
chord shows `⌘Z`, saved as physical KeyZ internally.

## C. Live toggle flip

1. Toggle OFF → confirm physical KeyZ (labeled Y) fires Undo.
2. Flip toggle ON without reloading → physical KeyZ stops firing it,
physical KeyY (labeled Z) starts firing it.
3. Flip back OFF → original positional behavior returns. No reload needed.

The resolver index, registration (`useHotkey`), display, and recorder
all subscribe to the preferences store, so a toggle flip propagates
through every consumer immediately.

## D. Default-direction migration

For a brand-new install (no persisted `keyboard-preferences` value), the
toggle initializes to **true**. Verify:

1. `localStorage.removeItem("keyboard-preferences")` + reload.
2. Open Settings → Keyboard. The Adaptive layout mapping switch is **on**.
3. On QWERTZ, Undo fires on the labeled-Z key out of the box.

Existing users who explicitly toggled OFF before this change keep their
saved value (persist middleware writes only on explicit `set()`). Users
who never opened the keyboard settings page get the new default ON on
upgrade — this is intentional (matches OS convention) and the only
behavior change they'll notice is on non-US layouts.

## E. Terminal reservation parity

With the toggle in either state, in a v2 terminal pane:

1. Press `Ctrl+C` — sent to the PTY (interrupts the running process).
2. Press the app's bound chord (e.g. `⌘P`) — opens the palette, doesn't
leak into the terminal buffer.
3. On QWERTZ + toggle ON: press `⌘Z` on the labeled-Z key (physical
KeyY). Undo should fire; the keystroke should not also leak to the PTY.

## Expected file touchpoints

If any of the above fails, the bug is almost certainly in one of:

- `apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts`
(registration — main consumer; gated via `useActiveLayoutMap`)
- `apps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.ts`
(reverse index used by terminal reservation)
- `apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts`
(display — `useHotkeyDisplay` + `useFormatBinding`)
- `apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts`
(`getDispatchChord`)
- `apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts`
(`getHotkeyConflict`)
- `apps/desktop/src/renderer/hotkeys/registry.ts` (defaults must use
`L()` for printable chords, bare strings only for named keys)
- `apps/desktop/src/renderer/hotkeys/stores/keyboardPreferencesStore.ts`
(default `adaptiveLayoutEnabled: true`)

All five hook/util consumers must read the layout map through the
`adaptiveLayoutEnabled` gate. If a future consumer reads
`useKeyboardLayoutStore` directly it must do the same — consider a
`useEffectiveLayoutMap()` helper as a follow-up to make this
unmissable.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { HOTKEYS, type HotkeyId } from "../../registry";
import { useHotkeyOverridesStore } from "../../stores/hotkeyOverridesStore";
import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore";
import { useKeyboardPreferencesStore } from "../../stores/keyboardPreferencesStore";
import { getEffectiveLayoutMap } from "../../stores/keyboardPreferencesStore";
import type { ShortcutBinding } from "../../types";
import { bindingToDispatchChord } from "../../utils/binding";

Expand Down Expand Up @@ -34,7 +33,5 @@ export function getBinding(id: HotkeyId): ShortcutBinding | null {
* the bound handler on non-US layouts.
*/
export function getDispatchChord(id: HotkeyId): string | null {
const adaptive = useKeyboardPreferencesStore.getState().adaptiveLayoutEnabled;
const layoutMap = adaptive ? useKeyboardLayoutStore.getState().map : null;
return bindingToDispatchChord(getBinding(id), layoutMap);
return bindingToDispatchChord(getBinding(id), getEffectiveLayoutMap());
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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 { useEffectiveLayoutMap } from "../../stores/keyboardPreferencesStore";
import type { HotkeyDisplay } from "../../types";
import { bindingToDispatchChord } from "../../utils/binding";
import { useBinding } from "../useBinding";
Expand All @@ -24,7 +24,7 @@ export function useHotkey(
options?: Options,
): HotkeyDisplay {
const binding = useBinding(id);
const layoutMap = useKeyboardLayoutStore((s) => s.map);
const layoutMap = useEffectiveLayoutMap();
const chord = bindingToDispatchChord(binding, layoutMap);
const callbackRef = useRef(callback);
callbackRef.current = callback;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
import { useMemo } from "react";
import { formatHotkeyDisplay } from "../../display";
import { PLATFORM } from "../../registry";
import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore";
import { useKeyboardPreferencesStore } from "../../stores/keyboardPreferencesStore";
import { useEffectiveLayoutMap } from "../../stores/keyboardPreferencesStore";
import type { HotkeyDisplay, ShortcutBinding } from "../../types";
import { bindingToDispatchChord } from "../../utils/binding";
import { useBinding } from "../useBinding";

/** Effective layout map for display: null when the user has disabled
* adaptive layout mapping, so glyphs match the authored chord. */
function useActiveLayoutMap(): ReadonlyMap<string, string> | null {
const layoutMap = useKeyboardLayoutStore((s) => s.map);
const adaptive = useKeyboardPreferencesStore((s) => s.adaptiveLayoutEnabled);
return adaptive ? layoutMap : null;
}

export function useHotkeyDisplay(id: string): HotkeyDisplay {
const binding = useBinding(id as Parameters<typeof useBinding>[0]);
const layoutMap = useActiveLayoutMap();
const layoutMap = useEffectiveLayoutMap();
const chord = bindingToDispatchChord(binding, layoutMap);
return useMemo(
() => formatHotkeyDisplay(chord, PLATFORM, layoutMap),
Expand All @@ -34,7 +25,7 @@ export function useHotkeyDisplay(id: string): HotkeyDisplay {
export function useFormatBinding(
binding: ShortcutBinding | null,
): HotkeyDisplay {
const layoutMap = useActiveLayoutMap();
const layoutMap = useEffectiveLayoutMap();
const chord = bindingToDispatchChord(binding, layoutMap);
return useMemo(
() => formatHotkeyDisplay(chord, PLATFORM, layoutMap),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect, useRef } from "react";
import { HOTKEYS, type HotkeyId, PLATFORM } from "../../registry";
import { useHotkeyOverridesStore } from "../../stores/hotkeyOverridesStore";
import { useKeyboardLayoutStore } from "../../stores/keyboardLayoutStore";
import { getEffectiveLayoutMap } from "../../stores/keyboardPreferencesStore";
import type {
BindingMode,
ParsedBinding,
Expand Down Expand Up @@ -141,7 +141,7 @@ function getHotkeyConflict(
excludeId: HotkeyId,
): HotkeyId | null {
const { overrides } = useHotkeyOverridesStore.getState();
const layoutMap = useKeyboardLayoutStore.getState().map;
const layoutMap = getEffectiveLayoutMap();
const candidateDispatch = bindingToDispatchChord(candidate, layoutMap);
if (!candidateDispatch) return null;
const target = canonicalizeChord(candidateDispatch);
Expand Down
117 changes: 117 additions & 0 deletions apps/desktop/src/renderer/hotkeys/registry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { describe, expect, it } from "bun:test";
import { HOTKEYS_REGISTRY } from "./registry";

// Locks in the shape of shipped defaults so the toggle keeps doing
// something. The original "Adaptive layout mapping" toggle was decorative
// for shipped defaults because every entry was a bare-string (physical)
// chord, and bindingToDispatchChord short-circuits physical mode. Keeping
// printable defaults as `mode: "logical"` is what makes the toggle move
// them on non-US layouts.

const NAMED_TERMINAL_TOKENS = new Set([
"enter",
"escape",
"backspace",
"delete",
"tab",
"space",
"up",
"down",
"left",
"right",
"arrowup",
"arrowdown",
"arrowleft",
"arrowright",
"home",
"end",
"pageup",
"pagedown",
"insert",
]);

function isFunctionKey(token: string): boolean {
return /^f([1-9]|1[0-2])$/.test(token);
}

function terminalToken(chord: string): string {
const parts = chord.split("+");
return parts[parts.length - 1] ?? "";
}

function* allBindings(): Generator<{
id: string;
platform: "mac" | "windows" | "linux";
binding: unknown;
}> {
for (const [id, def] of Object.entries(HOTKEYS_REGISTRY)) {
for (const platform of ["mac", "windows", "linux"] as const) {
yield { id, platform, binding: def.key[platform] };
}
}
}

describe("HOTKEYS_REGISTRY shape", () => {
it("authors printable defaults as mode: 'logical'", () => {
const offenders: string[] = [];
for (const { id, platform, binding } of allBindings()) {
if (binding === null) continue;
if (typeof binding !== "string") continue; // v2 objects checked below

const token = terminalToken(binding);
const isLayoutStable =
NAMED_TERMINAL_TOKENS.has(token) || isFunctionKey(token);
if (!isLayoutStable) {
offenders.push(`${id}.${platform}=${binding}`);
}
}
// If this fires: a printable chord ('meta+t', 'ctrl+shift+0', 'meta+slash')
// was authored as a bare string, which parses as physical mode and bypasses
// the adaptive-layout toggle. Wrap it with the L() helper in registry.ts.
expect(offenders).toEqual([]);
});

it("keeps every logical entry pinned to v2 / logical mode", () => {
for (const { id, platform, binding } of allBindings()) {
if (binding === null) continue;
if (typeof binding === "string") continue;
expect({ id, platform, binding }).toMatchObject({
binding: { version: 2, mode: "logical" },
});
}
});

it("keeps named-key chords as bare strings (layout-stable, no L() needed)", () => {
// Chords whose terminal is a named key gain nothing from logical mode —
// translateLogicalChord short-circuits named keys. Authoring them as
// bare strings keeps the registry terse.
for (const { id, platform, binding } of allBindings()) {
if (typeof binding !== "string") continue;
const token = terminalToken(binding);
const isLayoutStable =
NAMED_TERMINAL_TOKENS.has(token) || isFunctionKey(token);
if (!isLayoutStable) {
throw new Error(
`${id}.${platform}=${binding} is a bare string but its terminal token is not a named key — wrap with L() in registry.ts`,
);
}
}
});

it("includes a canary letter, digit, and punctuation default in logical mode", () => {
// Sample three different terminal-token shapes so a partial regression
// (e.g. only digits revert to physical) gets caught here too.
expect(HOTKEYS_REGISTRY.QUICK_OPEN.key.mac).toMatchObject({
mode: "logical",
chord: "meta+p",
});
expect(HOTKEYS_REGISTRY.JUMP_TO_WORKSPACE_1.key.mac).toMatchObject({
mode: "logical",
chord: "meta+1",
});
expect(HOTKEYS_REGISTRY.OPEN_SETTINGS.key.mac).toMatchObject({
mode: "logical",
chord: "meta+comma",
});
});
});
Loading
Loading