diff --git a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts index 1ebda67fc62..8f7cdb0a4a7 100644 --- a/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts +++ b/apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts @@ -22,7 +22,13 @@ export function captureHotkeyFromEvent(event: KeyboardEvent): string | null { if (isIgnorableKey(key)) return null; const isFKey = /^f([1-9]|1[0-2])$/.test(key); - if (!isFKey && !event.ctrlKey && !event.metaKey) return null; + // 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. + const altIsAppModifier = PLATFORM === "mac" && event.altKey; + if (!isFKey && !event.ctrlKey && !event.metaKey && !altIsAppModifier) { + return null; + } const modifiers = new Set(); if (event.metaKey) modifiers.add("meta"); @@ -55,6 +61,11 @@ const OS_RESERVED: Record> = { linux: new Set(["alt+f4", "alt+tab"].map(canonicalizeChord)), }; +function isMacAltOnlyChord(canonical: string): boolean { + const mods = new Set(canonical.split("+").slice(0, -1)); + return mods.has("alt") && !mods.has("meta") && !mods.has("ctrl"); +} + function checkReserved( keys: string, ): { reason: string; severity: "error" | "warning" } | null { @@ -63,6 +74,11 @@ function checkReserved( return { reason: "Reserved by terminal", severity: "error" }; if (OS_RESERVED[PLATFORM].has(canonical)) return { reason: "Reserved by OS", severity: "warning" }; + if (PLATFORM === "mac" && isMacAltOnlyChord(canonical)) + return { + reason: "Option shortcuts may prevent typing special characters", + severity: "warning", + }; return null; } diff --git a/apps/desktop/src/renderer/hotkeys/migrate.ts b/apps/desktop/src/renderer/hotkeys/migrate.ts index 4658bfb0a52..1fc3ac9d90b 100644 --- a/apps/desktop/src/renderer/hotkeys/migrate.ts +++ b/apps/desktop/src/renderer/hotkeys/migrate.ts @@ -8,6 +8,7 @@ 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"; @@ -18,10 +19,71 @@ const PLATFORM_MAP = { linux: "linux", } as const; +function sanitizeOverridesMap( + raw: Record, + assumeUSMacLayout: boolean, +): { cleaned: Record; dropped: number } { + const cleaned: Record = {}; + let dropped = 0; + for (const [id, value] of Object.entries(raw)) { + const sanitized = sanitizeOverride(value, { assumeUSMacLayout }); + if (sanitized === undefined) { + dropped++; + continue; + } + cleaned[id] = sanitized; + } + return { cleaned, dropped }; +} + export async function migrateHotkeyOverrides(): Promise { if (localStorage.getItem(MIGRATION_MARKER_KEY)) return; try { + const assumeUSMacLayout = + PLATFORM === "mac" ? await isUSCompatibleLayout() : true; + + // FORK NOTE: If the user already has a hotkey-overrides entry in + // localStorage from an earlier migration, don't overwrite it with the + // potentially stale legacy tRPC store value — but still re-run the + // sanitizer over those entries. The -v2 marker bump specifically exists + // so users whose pre-sanitizer localStorage contains corrupt overrides + // get them re-sanitized (or dropped) once. + const existingRaw = localStorage.getItem("hotkey-overrides"); + if (existingRaw) { + try { + const parsed = JSON.parse(existingRaw) as { + state?: { overrides?: Record }; + }; + const overrides = parsed?.state?.overrides; + if (overrides && Object.keys(overrides).length > 0) { + const { cleaned, dropped } = sanitizeOverridesMap( + overrides, + assumeUSMacLayout, + ); + localStorage.setItem( + "hotkey-overrides", + JSON.stringify({ state: { overrides: cleaned }, version: 0 }), + ); + console.log( + `[hotkeys] Re-sanitized ${Object.keys(cleaned).length} localStorage override(s)` + + (dropped > 0 ? `, dropped ${dropped} invalid` : ""), + ); + } else { + console.log( + "[hotkeys] Migration skipped — localStorage overrides empty", + ); + } + } catch (parseError) { + console.log( + "[hotkeys] Failed to parse existing localStorage overrides, leaving untouched:", + parseError, + ); + } + localStorage.setItem(MIGRATION_MARKER_KEY, "1"); + return; + } + const oldState = await electronTrpcClient.uiState.hotkeys.get.query(); const oldPlatformKey = PLATFORM_MAP[PLATFORM]; const oldOverrides = oldState?.byPlatform?.[oldPlatformKey]; @@ -31,28 +93,10 @@ export async function migrateHotkeyOverrides(): Promise { return; } - // If the user already has a hotkey-overrides entry in localStorage (set - // after the v1 migration), preserve it rather than overwriting with the - // potentially stale legacy tRPC store value. - const existingRaw = localStorage.getItem("hotkey-overrides"); - if (existingRaw) { - localStorage.setItem(MIGRATION_MARKER_KEY, "1"); - console.log( - "[hotkeys] Migration skipped — localStorage overrides already present", - ); - return; - } - - const cleaned: Record = {}; - let dropped = 0; - for (const [id, raw] of Object.entries(oldOverrides)) { - const sanitized = sanitizeOverride(raw); - if (sanitized === undefined) { - dropped++; - continue; - } - cleaned[id] = sanitized; - } + const { cleaned, dropped } = sanitizeOverridesMap( + oldOverrides, + assumeUSMacLayout, + ); localStorage.setItem( "hotkey-overrides", diff --git a/apps/desktop/src/renderer/hotkeys/registry.ts b/apps/desktop/src/renderer/hotkeys/registry.ts index c387ac2086c..1f62eed1229 100644 --- a/apps/desktop/src/renderer/hotkeys/registry.ts +++ b/apps/desktop/src/renderer/hotkeys/registry.ts @@ -102,13 +102,21 @@ export const HOTKEYS_REGISTRY = { category: "Workspace", }, PREV_WORKSPACE: { - key: { mac: null, windows: null, linux: null }, + key: { + mac: "meta+alt+up", + windows: "ctrl+shift+alt+up", + linux: "ctrl+shift+alt+up", + }, label: "Previous Workspace", category: "Workspace", description: "Navigate to the previous workspace in the sidebar", }, NEXT_WORKSPACE: { - key: { mac: null, windows: null, linux: null }, + key: { + mac: "meta+alt+down", + windows: "ctrl+shift+alt+down", + linux: "ctrl+shift+alt+down", + }, label: "Next Workspace", category: "Workspace", description: "Navigate to the next workspace in the sidebar", @@ -321,8 +329,8 @@ export const HOTKEYS_REGISTRY = { SCROLL_TO_BOTTOM: { key: { mac: "meta+shift+down", - windows: "ctrl+shift+alt+down", - linux: "ctrl+shift+alt+down", + windows: "ctrl+end", + linux: "ctrl+end", }, label: "Scroll to Bottom", category: "Terminal", @@ -343,53 +351,45 @@ export const HOTKEYS_REGISTRY = { category: "Terminal", }, PREV_TAB: { - key: { mac: null, windows: null, linux: null }, + key: { + mac: "meta+alt+left", + windows: "ctrl+shift+alt+left", + linux: "ctrl+shift+alt+left", + }, label: "Previous Tab", category: "Terminal", description: "Focus the previous tab in the active workspace", }, NEXT_TAB: { - key: { mac: null, windows: null, linux: null }, + key: { + mac: "meta+alt+right", + windows: "ctrl+shift+alt+right", + linux: "ctrl+shift+alt+right", + }, label: "Next Tab", category: "Terminal", description: "Focus the next tab in the active workspace", }, FOCUS_PANE_LEFT: { - key: { - mac: "meta+alt+left", - windows: "ctrl+shift+alt+left", - linux: "ctrl+shift+alt+left", - }, + key: { mac: null, windows: null, linux: null }, label: "Focus Pane Left", category: "Terminal", description: "Focus the pane to the left of the active pane", }, FOCUS_PANE_RIGHT: { - key: { - mac: "meta+alt+right", - windows: "ctrl+shift+alt+right", - linux: "ctrl+shift+alt+right", - }, + key: { mac: null, windows: null, linux: null }, label: "Focus Pane Right", category: "Terminal", description: "Focus the pane to the right of the active pane", }, FOCUS_PANE_UP: { - key: { - mac: "meta+alt+up", - windows: "ctrl+shift+alt+up", - linux: "ctrl+shift+alt+up", - }, + key: { mac: null, windows: null, linux: null }, label: "Focus Pane Up", category: "Terminal", description: "Focus the pane above the active pane", }, FOCUS_PANE_DOWN: { - key: { - mac: "meta+alt+down", - windows: "ctrl+shift+alt+down", - linux: "ctrl+shift+alt+down", - }, + key: { mac: null, windows: null, linux: null }, label: "Focus Pane Down", category: "Terminal", description: "Focus the pane below the active pane", diff --git a/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts b/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts new file mode 100644 index 00000000000..e19117ccc0e --- /dev/null +++ b/apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts @@ -0,0 +1,48 @@ +// 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/overrideSanitizer.test.ts b/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts index 1ac907cc779..8b17835f767 100644 --- a/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts +++ b/apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "bun:test"; import { sanitizeOverride } from "./sanitizeOverride"; -describe("sanitizeOverride (migration validation)", () => { +describe("sanitizeOverride — input shape", () => { it("preserves an explicit unassignment (null)", () => { expect(sanitizeOverride(null)).toBeNull(); }); @@ -14,29 +14,253 @@ describe("sanitizeOverride (migration validation)", () => { expect(sanitizeOverride({})).toBeUndefined(); }); - it("canonicalizes valid chords", () => { + 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 key tokens (bracketleft, f12)", () => { + 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"); }); +}); - it("drops pre-fix `ctrl+control` garbage (no real key)", () => { - expect(sanitizeOverride("ctrl+control")).toBeUndefined(); +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("drops chords with single-char punctuation keys (pre-fix event.key output)", () => { - expect(sanitizeOverride("ctrl+shift+@")).toBeUndefined(); - expect(sanitizeOverride("meta+[")).toBeUndefined(); - expect(sanitizeOverride("alt+¬")).toBeUndefined(); + it("rewrites minus / equal", () => { + expect(sanitizeOverride("ctrl+-")).toBe("ctrl+minus"); + expect(sanitizeOverride("meta+-")).toBe("meta+minus"); + expect(sanitizeOverride("meta+=")).toBe("meta+equal"); }); - it("drops chords with only modifiers", () => { - expect(sanitizeOverride("ctrl+shift")).toBeUndefined(); - expect(sanitizeOverride("meta")).toBeUndefined(); + 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/sanitizeOverride.ts b/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts index 217ee469c38..8993ab9f589 100644 --- a/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts +++ b/apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts @@ -1,18 +1,120 @@ 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`, `ctrl+shift+@`, `meta+[`) that the old recorder could - * produce and that would never match `event.code`-based dispatch. + * (`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): string | null | undefined { +export function sanitizeOverride( + value: unknown, + options: SanitizeOverrideOptions = {}, +): string | null | undefined { if (value === null) return null; if (typeof value !== "string" || !value.trim()) return undefined; - const canonical = canonicalizeChord(value); + 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/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx index aeac99dfe2f..c3b8e620aef 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx @@ -56,8 +56,10 @@ import type { Tab } from "renderer/stores/tabs/types"; import { useTabsWithPresets } from "renderer/stores/tabs/useTabsWithPresets"; import { extractPaneIdsFromLayout, + type FocusDirection, findPanePath, getFirstPaneId, + getSpatialNeighborMosaicPaneId, resolveActiveTabIdForWorkspace, } from "renderer/stores/tabs/utils"; import { @@ -801,6 +803,37 @@ export function WorkspacePage({ { enabled: isActive }, ); + // FORK NOTE: upstream #3460 introduces v1 directional pane focus via + // FOCUS_PANE_{LEFT,RIGHT,UP,DOWN}. The default bindings are unbound after + // #3472 (Cmd+Alt+Arrow goes back to prev/next tab/workspace), so this only + // fires when the user explicitly rebinds the FOCUS_PANE_* ids in Settings. + // `enabled: isActive` is required so inactive WorkspacePage instances + // kept mounted by KeepAliveWorkspaces don't also register the hotkey. + const moveFocusDirectional = useCallback( + (dir: FocusDirection) => { + if (!activeTabId || !activeTab?.layout || !focusedPaneId) return; + const neighbor = getSpatialNeighborMosaicPaneId( + activeTab.layout, + focusedPaneId, + dir, + ); + if (neighbor) setFocusedPane(activeTabId, neighbor); + }, + [activeTabId, activeTab?.layout, focusedPaneId, setFocusedPane], + ); + useHotkey("FOCUS_PANE_LEFT", () => moveFocusDirectional("left"), { + enabled: isActive, + }); + useHotkey("FOCUS_PANE_RIGHT", () => moveFocusDirectional("right"), { + enabled: isActive, + }); + useHotkey("FOCUS_PANE_UP", () => moveFocusDirectional("up"), { + enabled: isActive, + }); + useHotkey("FOCUS_PANE_DOWN", () => moveFocusDirectional("down"), { + enabled: isActive, + }); + // FORK NOTE: v1 workspace uses tRPC-based prev/next workspace navigation. // Upstream removed these handlers in #3403 (they use DashboardSidebar's // flattenedWorkspaces instead). Fork keeps tRPC approach for v1. 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 91695f383af..adeafc8ac5c 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx @@ -25,6 +25,7 @@ import { } from "renderer/hotkeys"; const CATEGORY_ORDER: HotkeyCategory[] = [ + "Navigation", "Browser", "Workspace", "Terminal", diff --git a/apps/desktop/src/renderer/stores/tabs/utils.ts b/apps/desktop/src/renderer/stores/tabs/utils.ts index 6a1b3d78a25..ae697442db0 100644 --- a/apps/desktop/src/renderer/stores/tabs/utils.ts +++ b/apps/desktop/src/renderer/stores/tabs/utils.ts @@ -676,6 +676,70 @@ export const findPanePath = ( return null; }; +export type FocusDirection = "left" | "right" | "up" | "down"; + +const findEdgeMosaicPaneId = ( + node: MosaicNode, + dir: FocusDirection, + alignmentPath: MosaicBranch[] = [], +): string => { + if (typeof node === "string") return node; + const axis: "row" | "column" = + dir === "left" || dir === "right" ? "row" : "column"; + if (node.direction === axis) { + const nearEdge: MosaicBranch = + dir === "right" || dir === "down" ? "first" : "second"; + return findEdgeMosaicPaneId(node[nearEdge], dir, alignmentPath); + } + const [alignedBranch = "first", ...rest] = alignmentPath; + return findEdgeMosaicPaneId(node[alignedBranch], dir, rest); +}; + +const getMosaicNodeAtPath = ( + node: MosaicNode, + path: MosaicBranch[], +): MosaicNode | null => { + let current: MosaicNode = node; + for (const branch of path) { + if (typeof current === "string") return null; + current = current[branch]; + } + return current; +}; + +/** + * Visually adjacent pane in `dir`, or null at the outer edge of the grid. + * Preserves cross-axis alignment when descending through perpendicular splits. + */ +export const getSpatialNeighborMosaicPaneId = ( + root: MosaicNode, + paneId: string, + dir: FocusDirection, +): string | null => { + const path = findPanePath(root, paneId); + if (!path) return null; + + const axis: "row" | "column" = + dir === "left" || dir === "right" ? "row" : "column"; + const wantSecond = dir === "right" || dir === "down"; + + for (let i = path.length - 1; i >= 0; i--) { + const ancestor = getMosaicNodeAtPath(root, path.slice(0, i)); + if (!ancestor || typeof ancestor === "string") continue; + if (ancestor.direction !== axis) continue; + const cameFrom = path[i]; + if (wantSecond && cameFrom !== "first") continue; + if (!wantSecond && cameFrom !== "second") continue; + const siblingBranch: MosaicBranch = wantSecond ? "second" : "first"; + return findEdgeMosaicPaneId( + ancestor[siblingBranch], + dir, + path.slice(i + 1), + ); + } + return null; +}; + /** * Adds a pane to an existing layout by creating a split */ diff --git a/bun.lock b/bun.lock index 2b125d3aac4..9c8e506cc00 100644 --- a/bun.lock +++ b/bun.lock @@ -110,7 +110,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.5.1", + "version": "1.5.3", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36",