From 74cf1931b13b3f1397c3cbb0454cd87d3414a976 Mon Sep 17 00:00:00 2001 From: Christoph Leiter Date: Sun, 9 Aug 2026 01:56:32 +0200 Subject: [PATCH] fix(shortcuts): show effective bindings in the cheat sheet (#2985) Render and search the shortcut chord that actually fires, including rebinding default-only display tokens such as question mark and index wildcards. Unassigned rows show no empty badge, and named-key overrides use the shared display vocabulary. --- .../components/keyboard-shortcuts-dialog.tsx | 69 +++------ .../src/keyboard/keyboard-shortcuts.test.ts | 135 ++++++++++++++++-- .../app/src/keyboard/keyboard-shortcuts.ts | 129 +++++++---------- .../src/keyboard/shortcut-help-search.test.ts | 113 +++++++++++++++ .../app/src/keyboard/shortcut-help-search.ts | 90 ++++++++++++ .../app/src/keyboard/shortcut-string.test.ts | 40 ++++++ packages/app/src/keyboard/shortcut-string.ts | 32 ++++- .../e2e/keyboard-shortcut-unassign.spec.ts | 53 +++++-- 8 files changed, 501 insertions(+), 160 deletions(-) create mode 100644 packages/app/src/keyboard/shortcut-help-search.test.ts create mode 100644 packages/app/src/keyboard/shortcut-help-search.ts diff --git a/packages/app/src/components/keyboard-shortcuts-dialog.tsx b/packages/app/src/components/keyboard-shortcuts-dialog.tsx index b671416f6..6e2e6db26 100644 --- a/packages/app/src/components/keyboard-shortcuts-dialog.tsx +++ b/packages/app/src/components/keyboard-shortcuts-dialog.tsx @@ -6,37 +6,16 @@ import { getIsElectronRuntime } from "@/constants/layout"; import { AdaptiveModalSheet, type SheetHeader } from "@/components/adaptive-modal-sheet"; import { Shortcut } from "@/components/ui/shortcut"; import { useKeyboardShortcutsStore } from "@/stores/keyboard-shortcuts-store"; -import { formatShortcut } from "@/utils/format-shortcut"; import { getShortcutOs } from "@/utils/shortcut-platform"; import { buildEffectiveBindings, buildKeyboardShortcutHelpSections, } from "@/keyboard/keyboard-shortcuts"; +import { filterShortcutHelpSections } from "@/keyboard/shortcut-help-search"; import { useKeyboardShortcutOverrides } from "@/hooks/use-keyboard-shortcut-overrides"; const SNAP_POINTS: string[] = ["70%", "92%"]; -function shortcutSearchAliases(keys: string[], shortcutOs: "mac" | "non-mac"): string { - const aliases = keys.map((key) => { - if (shortcutOs === "mac") { - if (key === "mod" || key === "meta") return ["cmd", "command"]; - if (key === "alt") return ["alt", "option"]; - } else { - if (key === "mod" || key === "ctrl") return ["ctrl", "control"]; - if (key === "meta") return ["win", "windows"]; - } - return [key]; - }); - const combinations = aliases.reduce( - (prefixes, choices) => - prefixes.flatMap((prefix) => choices.map((choice) => [...prefix, choice])), - [[]], - ); - return combinations - .flatMap((combination) => [combination.join(" "), combination.join("+")]) - .join(" "); -} - export function KeyboardShortcutsDialog() { const { t } = useTranslation(); const open = useKeyboardShortcutsStore((s) => s.shortcutsDialogOpen); @@ -54,33 +33,10 @@ export function KeyboardShortcutsDialog() { () => buildKeyboardShortcutHelpSections({ isMac, isDesktop: isDesktopApp }, bindings), [bindings, isDesktopApp, isMac], ); - const visibleSections = useMemo(() => { - const normalizedQuery = query.trim().toLocaleLowerCase(); - if (!normalizedQuery) return sections; - - return sections.flatMap((section) => { - const sectionTitle = t(section.titleKey); - if (sectionTitle.toLocaleLowerCase().includes(normalizedQuery)) { - return [section]; - } - - const rows = section.rows.filter((row) => { - const searchText = [ - t(row.labelKey), - row.noteKey ? t(row.noteKey) : row.note, - row.keys.join(" "), - formatShortcut(row.keys, shortcutOs), - shortcutSearchAliases(row.keys, shortcutOs), - ] - .filter(Boolean) - .join(" ") - .toLocaleLowerCase(); - return searchText.includes(normalizedQuery); - }); - - return rows.length > 0 ? [{ ...section, rows }] : []; - }); - }, [query, sections, shortcutOs, t]); + const visibleSections = useMemo( + () => filterShortcutHelpSections({ sections, query, translate: t, shortcutOs }), + [query, sections, shortcutOs, t], + ); useEffect(() => { if (!open) setQuery(""); @@ -114,14 +70,18 @@ export function KeyboardShortcutsDialog() { {t(section.titleKey)} {section.rows.map((row) => ( - + {t(row.labelKey)} {row.note ? ( {row.noteKey ? t(row.noteKey) : row.note} ) : null} - + {row.chord === null ? ( + {t("settings.shortcuts.unassigned")} + ) : ( + + )} ))} @@ -179,6 +139,13 @@ const styles = StyleSheet.create((theme) => ({ rowShortcut: { alignSelf: "flex-start", }, + // Matches the settings row's unassigned label, so the two screens describe the + // same state the same way. + rowUnassigned: { + alignSelf: "flex-start", + fontSize: theme.fontSize.sm, + color: theme.colors.foregroundMuted, + }, empty: { paddingVertical: theme.spacing[6], textAlign: "center", diff --git a/packages/app/src/keyboard/keyboard-shortcuts.test.ts b/packages/app/src/keyboard/keyboard-shortcuts.test.ts index a3accef6c..cf12f2c5d 100644 --- a/packages/app/src/keyboard/keyboard-shortcuts.test.ts +++ b/packages/app/src/keyboard/keyboard-shortcuts.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { formatShortcut } from "@/utils/format-shortcut"; import { buildKeyboardShortcutHelpSections, buildEffectiveBindings, @@ -645,7 +646,11 @@ describe("keyboard-shortcut help sections", () => { "workspace-tab-new": ["mod", "T"], "workspace-jump-index": ["mod", "1-9"], "workspace-tab-jump-index": ["mod", "alt", "1-9"], - "workspace-tab-close-current": ["meta", "W"], + // Derived from `combo: "Cmd+W"`, so the token is `mod` where the row + // used to be hand-authored as `meta`. This binding is mac-only and + // `formatShortcut` renders both as ⌘, so the badge is unchanged — see + // the render assertion below. + "workspace-tab-close-current": ["mod", "W"], "workspace-pane-split-right": ["mod", "\\"], "workspace-pane-close": ["mod", "shift", "W"], }, @@ -659,11 +664,15 @@ describe("keyboard-shortcut help sections", () => { }, }, { - name: "uses mod+b for the left sidebar and mod+period for both sidebars on non-mac", + name: "uses ctrl+b for the left sidebar and ctrl+period for both sidebars on non-mac", context: { isMac: false, isDesktop: false }, + // Derived from `combo: "Ctrl+B"` / `"Ctrl+."`, so the token is `ctrl` where + // these rows used to be hand-authored as `mod`. Both bindings are non-mac + // only and `formatShortcut` labels either token "Ctrl" there, so the badge + // is unchanged — see the render assertion below. expectedKeys: { - "toggle-left-sidebar": ["mod", "B"], - "toggle-both-sidebars": ["mod", "."], + "toggle-left-sidebar": ["ctrl", "B"], + "toggle-both-sidebars": ["ctrl", "."], }, }, ]; @@ -672,10 +681,104 @@ describe("keyboard-shortcut help sections", () => { const sections = buildKeyboardShortcutHelpSections(context); for (const [id, keys] of Object.entries(expectedKeys)) { - expect(findRow(sections, id)?.keys).toEqual(keys); + expect(findRow(sections, id)?.chord).toEqual([keys]); } }); + describe("rows derive their keys from the binding that fires", () => { + const macDesktop = { isMac: true, isDesktop: true }; + const NEW_WORKSPACE_BINDING = "workspace-new-cmd-n-mac"; + const MAC_INDEX_BINDING = "workspace-navigate-index-cmd-digit-mac"; + const PANE_FOCUS_LEFT_BINDING = "workspace-pane-focus-left-cmd-shift-left"; + const SHOW_SHORTCUTS_BINDING = "shortcuts-dialog-toggle-question-mark"; + + function rowChord(overrides: ShortcutOverrides, id: string) { + const sections = buildKeyboardShortcutHelpSections( + macDesktop, + buildEffectiveBindings(overrides), + ); + return findRow(sections, id)?.chord ?? null; + } + + // The reported bug: the cheat sheet advertised the shipped default no matter + // what the user had rebound the shortcut to. + it("shows the override, not the shipped default", () => { + expect(rowChord({}, "new-workspace")).toEqual([["mod", "N"]]); + expect(rowChord({ [NEW_WORKSPACE_BINDING]: "Cmd+Shift+K" }, "new-workspace")).toEqual([ + ["mod", "shift", "K"], + ]); + }); + + it("shows every step of a multi-step chord override", () => { + expect(rowChord({ [NEW_WORKSPACE_BINDING]: "Cmd+K Cmd+N" }, "new-workspace")).toEqual([ + ["mod", "K"], + ["mod", "N"], + ]); + }); + + it("reports an unassigned row as having no keys at all", () => { + expect(rowChord({ [NEW_WORKSPACE_BINDING]: UNASSIGNED_COMBO }, "new-workspace")).toBeNull(); + }); + + it("returns to the shipped default once the override is cleared", () => { + expect(rowChord({ [NEW_WORKSPACE_BINDING]: "Cmd+Shift+K" }, "new-workspace")).toEqual([ + ["mod", "shift", "K"], + ]); + expect(rowChord({}, "new-workspace")).toEqual([["mod", "N"]]); + }); + + // An arrow override used to render as the raw `ARROWLEFT` code, because the + // display path uppercased key names past the table that maps them to arrows. + it("renders an arrow override as an arrow", () => { + const chord = rowChord( + { [PANE_FOCUS_LEFT_BINDING]: "Cmd+Alt+ArrowLeft" }, + "workspace-pane-focus-left", + ); + expect(chord).toEqual([["mod", "alt", "Left"]]); + expect(formatShortcut(chord?.[0] ?? [], "mac")).toBe("⌥⌘←"); + }); + + // `1-9` and `?` are display-only tokens no combo string can spell, so these + // two rows opt out of default derivation via `help.defaultDisplayKeys`. + it("keeps the wildcard token on the index-jump row", () => { + expect(rowChord({}, "workspace-jump-index")).toEqual([["mod", "1-9"]]); + }); + + it("keeps the bare ? on the show-shortcuts row rather than deriving Shift+?", () => { + expect(rowChord({}, "show-shortcuts")).toEqual([["?"]]); + }); + + it("replaces the default-only wildcard when the index jump is rebound", () => { + expect(rowChord({ [MAC_INDEX_BINDING]: "Ctrl+Digit" }, "workspace-jump-index")).toEqual([ + ["ctrl", "Digit"], + ]); + }); + + it("replaces the default-only ? when show shortcuts is rebound", () => { + expect(rowChord({ [SHOW_SHORTCUTS_BINDING]: "Cmd+K" }, "show-shortcuts")).toEqual([ + ["mod", "K"], + ]); + }); + + // The authored row said `meta`, the derived row says `mod`. Both render ⌘ on + // mac and this binding is mac-only, so nothing the user sees moved. + it("still renders close-tab as ⌘W after switching to the derived token", () => { + const sections = buildKeyboardShortcutHelpSections(macDesktop); + const chord = findRow(sections, "workspace-tab-close-current")?.chord; + expect(formatShortcut(chord?.[0] ?? [], "mac")).toBe("⌘W"); + }); + + // Same story on the other side: these rows said `mod`, the non-mac combos + // say `Ctrl`, and non-mac labels both as "Ctrl". + it("still renders the sidebar toggles as Ctrl+B and Ctrl+. on non-mac", () => { + const sections = buildKeyboardShortcutHelpSections({ isMac: false, isDesktop: false }); + const left = findRow(sections, "toggle-left-sidebar")?.chord; + const both = findRow(sections, "toggle-both-sidebars")?.chord; + expect(formatShortcut(left?.[0] ?? [], "non-mac")).toBe("Ctrl+B"); + expect(formatShortcut(both?.[0] ?? [], "non-mac")).toBe("Ctrl+."); + }); + }); + it("returns stable i18n keys for section titles and help rows", () => { const sections = buildKeyboardShortcutHelpSections({ isMac: true, isDesktop: true }); const projects = sections.find((section) => section.id === "projects"); @@ -889,9 +992,12 @@ describe("unassigned shortcuts", () => { }); describe("resolveShortcutKeysForAction", () => { + // `ctrl`, not `mod`: these keys are derived from the non-mac binding's + // `combo: "Ctrl+T"` rather than hand-authored. `formatShortcut` labels both + // tokens "Ctrl" off mac, and this binding is non-mac only. it("returns the default keys when there is no override", () => { expect(resolveShortcutKeysForAction("workspace-tab-new", {}, desktopNonMac)).toEqual([ - ["mod", "T"], + ["ctrl", "T"], ]); }); @@ -924,13 +1030,13 @@ describe("unassigned shortcuts", () => { { [TAB_NEW_BINDING]: "Ctrl+Nonsense" }, desktopNonMac, ), - ).toEqual([["mod", "T"]]); + ).toEqual([["ctrl", "T"]]); }); it("falls back to the default keys for a non-string stored value", () => { const overrides = { [TAB_NEW_BINDING]: 42 } as unknown as ShortcutOverrides; expect(resolveShortcutKeysForAction("workspace-tab-new", overrides, desktopNonMac)).toEqual([ - ["mod", "T"], + ["ctrl", "T"], ]); }); @@ -944,7 +1050,7 @@ describe("unassigned shortcuts", () => { const bindings = buildEffectiveBindings({ [TAB_NEW_BINDING]: UNASSIGNED_COMBO }); const sections = buildKeyboardShortcutHelpSections(desktopNonMac, bindings); - expect(findRow(sections, "workspace-tab-new")?.keys).toEqual([]); + expect(findRow(sections, "workspace-tab-new")?.chord).toBeNull(); }); it("keeps the row so the shortcut stays rebindable", () => { @@ -958,15 +1064,14 @@ describe("unassigned shortcuts", () => { it("still lists the default keys when nothing is overridden", () => { const sections = buildKeyboardShortcutHelpSections(desktopNonMac); - expect(findRow(sections, "workspace-tab-new")?.keys).toEqual(["mod", "T"]); - expect(getDefaultKeysForAction("workspace-tab-new", desktopNonMac)).toEqual(["mod", "T"]); + expect(findRow(sections, "workspace-tab-new")?.chord).toEqual([["ctrl", "T"]]); + expect(getDefaultKeysForAction("workspace-tab-new", desktopNonMac)).toEqual([["ctrl", "T"]]); }); // A binding authored with `combo: ""` has no default. The settings row uses // this to hide Reset, which would otherwise be a dead button landing on the - // same "Not set" state Clear already produced. `help.keys` stays populated - // on purpose: it is hand-authored, so the empty parsed chord alone has to be - // what marks the binding default-less. + // same "Not set" state Clear already produced. The empty parsed chord is what + // marks the binding default-less. it("reports no default keys for a binding that ships without a combo", () => { const bindings = withoutDefaultCombo("workspace-tab-new"); @@ -977,7 +1082,7 @@ describe("unassigned shortcuts", () => { const bindings = withoutDefaultCombo("workspace-tab-new"); const sections = buildKeyboardShortcutHelpSections(desktopNonMac, bindings); - expect(findRow(sections, "workspace-tab-new")?.keys).toEqual([]); + expect(findRow(sections, "workspace-tab-new")?.chord).toBeNull(); }); }); }); diff --git a/packages/app/src/keyboard/keyboard-shortcuts.ts b/packages/app/src/keyboard/keyboard-shortcuts.ts index e7e4032b0..6ca660c37 100644 --- a/packages/app/src/keyboard/keyboard-shortcuts.ts +++ b/packages/app/src/keyboard/keyboard-shortcuts.ts @@ -43,7 +43,8 @@ export interface KeyboardShortcutHelpRow { id: string; label: string; labelKey: string; - keys: ShortcutKey[]; + /** The keys that actually fire this action, or `null` when it has none. */ + chord: ShortcutKey[][] | null; note?: string; noteKey?: string; } @@ -88,7 +89,14 @@ interface ShortcutHelp { id: string; section: ShortcutSectionId; label: string; - keys: ShortcutKey[]; + /** + * Display keys to show instead of the combo. Set this only when the combo + * cannot express what the row means — the `Digit` wildcard, which stands for + * any of 1-9, and `Shift+?`, whose Shift is implied by the character itself. + * Every other row derives its keys from `combo`, so a rebound shortcut shows + * what it now does rather than what it shipped as. + */ + defaultDisplayKeys?: ShortcutKey[]; note?: string; } @@ -194,7 +202,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "new-agent", section: "projects", label: "Open project", - keys: ["mod", "O"], }, }, { @@ -206,7 +213,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "new-agent", section: "projects", label: "Open project", - keys: ["mod", "O"], }, }, @@ -220,7 +226,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "new-workspace", section: "projects", label: "New workspace", - keys: ["mod", "N"], }, }, { @@ -232,7 +237,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "new-workspace", section: "projects", label: "New workspace", - keys: ["mod", "N"], }, }, @@ -246,7 +250,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "switch-project", section: "projects", label: "Switch project", - keys: ["mod", "P"], }, }, { @@ -258,7 +261,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "switch-project", section: "projects", label: "Switch project", - keys: ["mod", "P"], }, }, @@ -274,7 +276,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "archive-workspace", section: "projects", label: "Archive workspace", - keys: ["mod", "shift", "Backspace"], }, }, { @@ -288,7 +289,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "archive-workspace", section: "projects", label: "Archive workspace", - keys: ["mod", "shift", "Backspace"], }, }, @@ -302,7 +302,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "pin-workspace", section: "projects", label: "Pin chat", - keys: ["mod", "shift", "P"], }, }, { @@ -314,7 +313,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "pin-workspace", section: "projects", label: "Pin chat", - keys: ["mod", "shift", "P"], }, }, @@ -328,7 +326,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-new", section: "tabs-panes", label: "New tab", - keys: ["mod", "T"], }, }, { @@ -340,7 +337,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-new", section: "tabs-panes", label: "New tab", - keys: ["mod", "T"], }, }, { @@ -352,7 +348,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-close-current", section: "tabs-panes", label: "Close current tab", - keys: ["meta", "W"], }, }, { @@ -364,7 +359,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-close-current", section: "tabs-panes", label: "Close current tab", - keys: ["ctrl", "W"], }, }, { @@ -376,7 +370,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-close-current", section: "tabs-panes", label: "Close current tab", - keys: ["alt", "shift", "W"], }, }, @@ -391,7 +384,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-jump-index", section: "navigation", label: "Jump to workspace", - keys: ["mod", "1-9"], + defaultDisplayKeys: ["mod", "1-9"], }, }, { @@ -404,7 +397,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-jump-index", section: "navigation", label: "Jump to workspace", - keys: ["mod", "1-9"], + defaultDisplayKeys: ["mod", "1-9"], }, }, { @@ -417,7 +410,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-jump-index", section: "navigation", label: "Jump to workspace", - keys: ["alt", "1-9"], + defaultDisplayKeys: ["alt", "1-9"], }, }, @@ -432,7 +425,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-jump-index", section: "navigation", label: "Jump to tab", - keys: ["mod", "alt", "1-9"], + defaultDisplayKeys: ["mod", "alt", "1-9"], }, }, { @@ -445,7 +438,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-jump-index", section: "navigation", label: "Jump to tab", - keys: ["alt", "1-9"], + defaultDisplayKeys: ["alt", "1-9"], }, }, { @@ -458,7 +451,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-jump-index", section: "navigation", label: "Jump to tab", - keys: ["alt", "shift", "1-9"], + defaultDisplayKeys: ["alt", "shift", "1-9"], }, }, @@ -473,7 +466,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-prev", section: "navigation", label: "Previous workspace", - keys: ["mod", "["], }, }, { @@ -486,7 +478,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-prev", section: "navigation", label: "Previous workspace", - keys: ["mod", "["], }, }, { @@ -499,7 +490,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-next", section: "navigation", label: "Next workspace", - keys: ["mod", "]"], }, }, { @@ -512,7 +502,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-next", section: "navigation", label: "Next workspace", - keys: ["mod", "]"], }, }, { @@ -525,7 +514,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-prev", section: "navigation", label: "Previous workspace", - keys: ["alt", "["], }, }, { @@ -538,7 +526,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-next", section: "navigation", label: "Next workspace", - keys: ["alt", "]"], }, }, @@ -553,7 +540,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-prev", section: "navigation", label: "Previous tab", - keys: ["alt", "shift", "["], }, }, { @@ -566,7 +552,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-tab-next", section: "navigation", label: "Next tab", - keys: ["alt", "shift", "]"], }, }, @@ -580,7 +565,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-split-right", section: "tabs-panes", label: "Split pane right", - keys: ["mod", "\\"], }, }, { @@ -592,7 +576,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-split-down", section: "tabs-panes", label: "Split pane down", - keys: ["mod", "shift", "\\"], }, }, { @@ -604,7 +587,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-focus-left", section: "tabs-panes", label: "Focus pane left", - keys: ["mod", "shift", "Left"], }, }, { @@ -616,7 +598,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-focus-right", section: "tabs-panes", label: "Focus pane right", - keys: ["mod", "shift", "Right"], }, }, { @@ -628,7 +609,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-focus-up", section: "tabs-panes", label: "Focus pane up", - keys: ["mod", "shift", "Up"], }, }, { @@ -640,7 +620,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-focus-down", section: "tabs-panes", label: "Focus pane down", - keys: ["mod", "shift", "Down"], }, }, { @@ -652,7 +631,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-move-tab-left", section: "tabs-panes", label: "Move tab left", - keys: ["mod", "shift", "alt", "Left"], }, }, { @@ -664,7 +642,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-move-tab-right", section: "tabs-panes", label: "Move tab right", - keys: ["mod", "shift", "alt", "Right"], }, }, { @@ -676,7 +653,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-move-tab-up", section: "tabs-panes", label: "Move tab up", - keys: ["mod", "shift", "alt", "Up"], }, }, { @@ -688,7 +664,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-move-tab-down", section: "tabs-panes", label: "Move tab down", - keys: ["mod", "shift", "alt", "Down"], }, }, { @@ -700,7 +675,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-pane-close", section: "tabs-panes", label: "Close pane", - keys: ["mod", "shift", "W"], }, }, @@ -714,7 +688,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-terminal-new", section: "panels", label: "New terminal", - keys: ["mod", "shift", "T"], }, }, { @@ -726,7 +699,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "workspace-terminal-new", section: "panels", label: "New terminal", - keys: ["mod", "shift", "T"], }, }, @@ -740,7 +712,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-command-center", section: "panels", label: "Toggle command center", - keys: ["mod", "K"], }, }, { @@ -752,7 +723,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-command-center", section: "panels", label: "Toggle command center", - keys: ["mod", "K"], }, }, @@ -767,7 +737,7 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "show-shortcuts", section: "panels", label: "Show keyboard shortcuts", - keys: ["?"], + defaultDisplayKeys: ["?"], note: "Available when focus is not in a text field or terminal.", }, }, @@ -782,7 +752,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-left-sidebar", section: "panels", label: "Toggle left sidebar", - keys: ["mod", "B"], }, }, { @@ -794,7 +763,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-left-sidebar", section: "panels", label: "Toggle left sidebar", - keys: ["mod", "B"], }, }, { @@ -806,7 +774,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-right-sidebar", section: "panels", label: "Toggle right sidebar", - keys: ["mod", "E"], }, }, { @@ -818,7 +785,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-right-sidebar", section: "panels", label: "Toggle right sidebar", - keys: ["mod", "E"], }, }, { @@ -838,7 +804,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-both-sidebars", section: "panels", label: "Toggle both sidebars", - keys: ["mod", "."], }, }, { @@ -850,7 +815,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-both-sidebars", section: "panels", label: "Toggle both sidebars", - keys: ["mod", "."], }, }, @@ -864,7 +828,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-settings", section: "panels", label: "Toggle settings", - keys: ["mod", ","], }, }, { @@ -876,7 +839,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-settings", section: "panels", label: "Toggle settings", - keys: ["mod", ","], }, }, @@ -890,7 +852,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-focus", section: "panels", label: "Toggle focus mode", - keys: ["mod", "shift", "F"], }, }, { @@ -902,7 +863,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "toggle-focus", section: "panels", label: "Toggle focus mode", - keys: ["mod", "shift", "F"], }, }, @@ -916,7 +876,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "cycle-theme", section: "panels", label: "Cycle theme", - keys: ["mod", "alt", "T"], }, }, { @@ -928,7 +887,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "cycle-theme", section: "panels", label: "Cycle theme", - keys: ["mod", "alt", "T"], }, }, @@ -943,7 +901,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "focus-message-input", section: "agent-input", label: "Focus message input", - keys: ["mod", "L"], }, }, { @@ -956,7 +913,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "focus-message-input", section: "agent-input", label: "Focus message input", - keys: ["mod", "L"], }, }, { @@ -970,7 +926,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "cycle-agent-mode", section: "agent-input", label: "Cycle agent mode", - keys: ["shift", "Tab"], }, }, { @@ -984,7 +939,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "voice-toggle", section: "agent-input", label: "Toggle voice mode", - keys: ["mod", "shift", "D"], }, }, { @@ -998,7 +952,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "voice-toggle", section: "agent-input", label: "Toggle voice mode", - keys: ["mod", "shift", "D"], }, }, { @@ -1011,7 +964,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "dictation-toggle", section: "agent-input", label: "Start/stop dictation", - keys: ["mod", "D"], }, }, { @@ -1024,7 +976,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "dictation-toggle", section: "agent-input", label: "Start/stop dictation", - keys: ["mod", "D"], }, }, { @@ -1038,7 +989,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "agent-interrupt", section: "agent-input", label: "Interrupt agent", - keys: ["Esc"], }, }, { @@ -1060,7 +1010,6 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ id: "voice-mute-toggle", section: "agent-input", label: "Mute/unmute voice mode", - keys: ["Space"], }, }, ]; @@ -1123,7 +1072,11 @@ export function buildEffectiveBindings(overrides: ShortcutOverrides): ParsedShor if (binding.repeat === false && lastCombo) { lastCombo.repeat = false; } - return { ...binding, combo: override, parsedChord }; + if (!binding.help?.defaultDisplayKeys) { + return { ...binding, combo: override, parsedChord }; + } + const { defaultDisplayKeys: _defaultDisplayKeys, ...help } = binding.help; + return { ...binding, combo: override, parsedChord, help }; }); } @@ -1422,11 +1375,33 @@ export function getBindingIdForAction( return null; } +/** + * The keys to display for one binding, derived from the combo that actually + * fires so a rebound shortcut never advertises the keys it shipped with. + * + * `help.defaultDisplayKeys` wins where it is set, but only on the shipped + * binding. Effective bindings discard it when an override replaces the combo, + * so rebound rows still derive the keys that now fire. + * `parsedChord` is the single source of truth for "has no keys": it is derived + * from `combo`, so it covers both a user-unassigned shortcut and a binding + * authored without a default. + */ +function displayChordForBinding(binding: ParsedShortcutBinding): ShortcutKey[][] | null { + if (binding.parsedChord.length === 0) { + return null; + } + const displayKeys = binding.help?.defaultDisplayKeys; + if (displayKeys) { + return [displayKeys]; + } + return chordStringToShortcutKeys(binding.combo); +} + export function getDefaultKeysForAction( actionId: string, platform: { isMac: boolean; isDesktop: boolean }, bindings: readonly ParsedShortcutBinding[] = DEFAULT_BINDINGS, -): ShortcutKey[] | null { +): ShortcutKey[][] | null { for (const binding of bindings) { if (binding.help?.id !== actionId) { continue; @@ -1434,13 +1409,7 @@ export function getDefaultKeysForAction( if (!helpMatchesPlatform(binding.when, platform)) { continue; } - // `help.keys` is hand-authored, so it cannot be trusted to be empty for a - // binding that ships without a default combo. The parsed chord is derived - // from `combo`, so it is the single source of truth for "has no keys". - if (binding.parsedChord.length === 0) { - return null; - } - return binding.help.keys; + return displayChordForBinding(binding); } return null; } @@ -1464,8 +1433,7 @@ export function resolveShortcutKeysForAction( return null; } - const defaultKeys = getDefaultKeysForAction(actionId, platform); - const defaultChord = defaultKeys ? [defaultKeys] : null; + const defaultChord = getDefaultKeysForAction(actionId, platform); const override = overrides[bindingId]; if (override === UNASSIGNED_COMBO || override === "") { @@ -1566,8 +1534,7 @@ export function buildKeyboardShortcutHelpSections( id: help.id, label: help.label, labelKey: SHORTCUT_HELP_LABEL_KEYS[help.id] ?? help.label, - // An empty chord has no keys to show, whatever `help.keys` was authored as. - keys: binding.parsedChord.length === 0 ? [] : help.keys, + chord: displayChordForBinding(binding), ...(help.note ? { note: help.note } : {}), ...(SHORTCUT_HELP_NOTE_KEYS[help.id] ? { noteKey: SHORTCUT_HELP_NOTE_KEYS[help.id] } : {}), }); diff --git a/packages/app/src/keyboard/shortcut-help-search.test.ts b/packages/app/src/keyboard/shortcut-help-search.test.ts new file mode 100644 index 000000000..c365a09b0 --- /dev/null +++ b/packages/app/src/keyboard/shortcut-help-search.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "vitest"; +import { + buildEffectiveBindings, + buildKeyboardShortcutHelpSections, + UNASSIGNED_COMBO, + type ShortcutOverrides, +} from "./keyboard-shortcuts"; +import { filterShortcutHelpSections, shortcutSearchText } from "./shortcut-help-search"; + +const MAC_DESKTOP = { isMac: true, isDesktop: true }; +const NEW_WORKSPACE_BINDING = "workspace-new-cmd-n-mac"; + +/** + * The i18n port, filled with the real English strings the rows use. Translation + * is the only dependency the filter cannot compute, so it is injected rather + * than mocked. + */ +const EN: Record = { + "settings.shortcuts.sections.projects": "Projects", + "settings.shortcuts.help.newWorkspace": "New workspace", + "settings.shortcuts.help.openProject": "Open project", +}; + +function translate(key: string): string { + return EN[key] ?? key; +} + +function filter(query: string, overrides: ShortcutOverrides = {}) { + return filterShortcutHelpSections({ + sections: buildKeyboardShortcutHelpSections(MAC_DESKTOP, buildEffectiveBindings(overrides)), + query, + translate, + shortcutOs: "mac", + }); +} + +function matchedRowIds(query: string, overrides: ShortcutOverrides = {}): string[] { + return filter(query, overrides).flatMap((section) => section.rows.map((row) => row.id)); +} + +describe("shortcutSearchText", () => { + it("covers the tokens, the rendered badge, and the modifier aliases", () => { + const text = shortcutSearchText([["mod", "N"]], "mac"); + + expect(text).toContain("mod N"); + expect(text).toContain("⌘N"); + expect(text).toContain("cmd+N"); + expect(text).toContain("command+N"); + }); + + // The alias expansion is combinatorial, so a flattened chord would multiply + // the two combos together and invent spellings for keys never pressed at once. + it("keeps a multi-step chord's combos apart", () => { + const text = shortcutSearchText( + [ + ["mod", "K"], + ["mod", "N"], + ], + "mac", + ); + + expect(text).toContain("cmd+K"); + expect(text).toContain("cmd+N"); + expect(text).not.toContain("cmd+K+cmd+N"); + }); +}); + +describe("filterShortcutHelpSections", () => { + it("returns every section for a blank query", () => { + expect(filter(" ")).toHaveLength(buildKeyboardShortcutHelpSections(MAC_DESKTOP).length); + }); + + it("matches a row by its label", () => { + expect(matchedRowIds("new workspace")).toContain("new-workspace"); + }); + + it("matches a row by the rendered badge", () => { + expect(matchedRowIds("⌘n")).toContain("new-workspace"); + }); + + it("matches a row by a spelled-out modifier", () => { + expect(matchedRowIds("command+n")).toContain("new-workspace"); + }); + + // The reported bug, from the search side: the cheat sheet has to stop finding + // a row by keys the user has rebound away from. + it("finds a rebound row by its new keys and not by the superseded default", () => { + const overrides = { [NEW_WORKSPACE_BINDING]: "Cmd+Shift+K" }; + + expect(matchedRowIds("cmd+shift+k", overrides)).toContain("new-workspace"); + expect(matchedRowIds("cmd+n", overrides)).not.toContain("new-workspace"); + }); + + it("still finds an unassigned row by its label", () => { + const overrides = { [NEW_WORKSPACE_BINDING]: UNASSIGNED_COMBO }; + + expect(matchedRowIds("new workspace", overrides)).toContain("new-workspace"); + expect(matchedRowIds("cmd+n", overrides)).not.toContain("new-workspace"); + }); + + it("keeps every row of a section whose own title matches", () => { + const projects = filter("projects").find((section) => section.id === "projects"); + const allProjects = buildKeyboardShortcutHelpSections(MAC_DESKTOP).find( + (section) => section.id === "projects", + ); + + expect(projects?.rows).toHaveLength(allProjects?.rows.length ?? 0); + }); + + it("drops a section whose rows all miss", () => { + expect(filter("no shortcut spells this")).toEqual([]); + }); +}); diff --git a/packages/app/src/keyboard/shortcut-help-search.ts b/packages/app/src/keyboard/shortcut-help-search.ts new file mode 100644 index 000000000..04061ee2d --- /dev/null +++ b/packages/app/src/keyboard/shortcut-help-search.ts @@ -0,0 +1,90 @@ +import type { KeyboardShortcutHelpSection } from "@/keyboard/keyboard-shortcuts"; +import { formatShortcut, type ShortcutKey, type ShortcutOs } from "@/utils/format-shortcut"; + +/** Resolves an i18n key to display text. The caller's `t`, injected. */ +export type TranslateHelpKey = (key: string) => string; + +/** + * Alternative spellings for one combo, so searching "cmd k" and "command+k" both + * find ⌘K. The modifier a user types rarely matches the token the binding + * stores, and never matches the symbol the badge renders. + */ +export function shortcutSearchAliases( + keys: readonly ShortcutKey[], + shortcutOs: ShortcutOs, +): string { + const aliases = keys.map((key) => { + if (shortcutOs === "mac") { + if (key === "mod" || key === "meta") return ["cmd", "command"]; + if (key === "alt") return ["alt", "option"]; + } else { + if (key === "mod" || key === "ctrl") return ["ctrl", "control"]; + if (key === "meta") return ["win", "windows"]; + } + return [key]; + }); + const combinations = aliases.reduce( + (prefixes, choices) => + prefixes.flatMap((prefix) => choices.map((choice) => [...prefix, choice])), + [[]], + ); + return combinations + .flatMap((combination) => [combination.join(" "), combination.join("+")]) + .join(" "); +} + +/** + * Search text for one row's keys. Built per combo and joined, never over a + * flattened chord: the alias expansion is combinatorial, so flattening a + * multi-step chord into one key list both explodes the combination count and + * invents aliases for combos the user never has to press together. + */ +export function shortcutSearchText(chord: ShortcutKey[][], shortcutOs: ShortcutOs): string { + return chord + .flatMap((combo) => [ + combo.join(" "), + formatShortcut(combo, shortcutOs), + shortcutSearchAliases(combo, shortcutOs), + ]) + .join(" "); +} + +/** + * The cheat sheet's sections narrowed to a query, matching on the row's label, + * its note, and every spelling of the keys that actually fire it. A section + * whose own title matches keeps all of its rows. + */ +export function filterShortcutHelpSections({ + sections, + query, + translate, + shortcutOs, +}: { + sections: readonly KeyboardShortcutHelpSection[]; + query: string; + translate: TranslateHelpKey; + shortcutOs: ShortcutOs; +}): KeyboardShortcutHelpSection[] { + const normalizedQuery = query.trim().toLocaleLowerCase(); + if (!normalizedQuery) return [...sections]; + + return sections.flatMap((section) => { + if (translate(section.titleKey).toLocaleLowerCase().includes(normalizedQuery)) { + return [section]; + } + + const rows = section.rows.filter((row) => { + const searchText = [ + translate(row.labelKey), + row.noteKey ? translate(row.noteKey) : row.note, + row.chord ? shortcutSearchText(row.chord, shortcutOs) : null, + ] + .filter(Boolean) + .join(" ") + .toLocaleLowerCase(); + return searchText.includes(normalizedQuery); + }); + + return rows.length > 0 ? [{ ...section, rows }] : []; + }); +} diff --git a/packages/app/src/keyboard/shortcut-string.test.ts b/packages/app/src/keyboard/shortcut-string.test.ts index 345a2b73e..f7e5c70df 100644 --- a/packages/app/src/keyboard/shortcut-string.test.ts +++ b/packages/app/src/keyboard/shortcut-string.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it } from "vitest"; +import { formatShortcut } from "@/utils/format-shortcut"; import { + comboStringToShortcutKeys, keyComboToString, keyboardEventToComboString, parseShortcutString, + SHORTCUT_KEY_NAMES, } from "./shortcut-string"; function keyboardEvent(overrides: Partial): KeyboardEvent { @@ -68,3 +71,40 @@ describe("parseShortcutString round-trips punctuation keys", () => { expect(keyComboToString(combo)).toBe("Cmd+-"); }); }); + +describe("comboStringToShortcutKeys", () => { + it("maps every modifier spelling onto its display token", () => { + expect(comboStringToShortcutKeys("Cmd+Ctrl+Alt+Shift+K")).toEqual([ + "mod", + "ctrl", + "alt", + "shift", + "K", + ]); + // `parseShortcutString` accepts `Mod` as well, so the display path has to. + expect(comboStringToShortcutKeys("Mod+K")).toEqual(["mod", "K"]); + }); + + it("hands named keys to the display table under the names it uses", () => { + expect(comboStringToShortcutKeys("Cmd+Shift+ArrowLeft")).toEqual(["mod", "shift", "Left"]); + expect(formatShortcut(comboStringToShortcutKeys("Cmd+Shift+ArrowLeft"), "mac")).toBe( + "Shift+⌘+←", + ); + expect(formatShortcut(comboStringToShortcutKeys("Escape"), "mac")).toBe("Esc"); + expect(formatShortcut(comboStringToShortcutKeys("Backspace"), "mac")).toBe("⌫"); + }); + + // The whole class, not just the arrows: uppercasing key names used to push + // every multi-character name past `KEY_DISPLAY`, so an override on one of them + // rendered as a shouty raw code — `ARROWLEFT`, `ESCAPE`, `PAGEDOWN`. + it("never emits a shouty raw code for any key the capture path can produce", () => { + for (const name of SHORTCUT_KEY_NAMES) { + // `F1` and the digits are already all-caps, so only names that contain a + // lowercase letter can exhibit the bug. + if (name === name.toUpperCase()) continue; + const [key] = comboStringToShortcutKeys(name); + const rendered = formatShortcut([key], "non-mac"); + expect(rendered, `${name} renders as ${rendered}`).not.toBe(name.toUpperCase()); + } + }); +}); diff --git a/packages/app/src/keyboard/shortcut-string.ts b/packages/app/src/keyboard/shortcut-string.ts index eb0f8b4bc..06a25bd37 100644 --- a/packages/app/src/keyboard/shortcut-string.ts +++ b/packages/app/src/keyboard/shortcut-string.ts @@ -74,6 +74,13 @@ for (let i = 1; i <= 12; i++) { KEY_MAP[`F${i}`] = { code: `F${i}` }; } +/** + * Every key name a combo string can carry. Exported so the display path can be + * driven over the whole vocabulary instead of a hand-copied sample that goes + * stale the moment `KEY_MAP` gains an entry. + */ +export const SHORTCUT_KEY_NAMES: readonly string[] = Object.keys(KEY_MAP); + const CODE_TO_KEY: Record = {}; for (const [humanKey, mapping] of Object.entries(KEY_MAP)) { if (!CODE_TO_KEY[mapping.code]) { @@ -177,6 +184,21 @@ const MODIFIER_CODES = new Set([ "ShiftRight", ]); +/** + * Key names as combo strings spell them, mapped to the names `KEY_DISPLAY` in + * `format-shortcut` is keyed on. The two vocabularies were never reconciled: + * `keyboardEventToComboString` emits `ArrowLeft` and `Escape` (from `KEY_MAP`), + * while the display table only knows `Left` and `Esc`. Without this bridge an + * override on a named key renders as a raw code — `ARROWLEFT`, not `←`. + */ +const DISPLAY_KEY_ALIASES: Record = { + ArrowLeft: "Left", + ArrowRight: "Right", + ArrowUp: "Up", + ArrowDown: "Down", + Escape: "Esc", +}; + export function comboStringToShortcutKeys(comboString: string): ShortcutKey[] { const parts = comboString.split("+"); const keys: ShortcutKey[] = []; @@ -185,6 +207,11 @@ export function comboStringToShortcutKeys(comboString: string): ShortcutKey[] { case "Cmd": keys.push("mod"); break; + // `parseShortcutString` accepts `Mod`, so the display path has to as well + // or a combo it can match would render the literal word. + case "Mod": + keys.push("mod"); + break; case "Ctrl": keys.push("ctrl"); break; @@ -195,7 +222,10 @@ export function comboStringToShortcutKeys(comboString: string): ShortcutKey[] { keys.push("shift"); break; default: - keys.push(part.toUpperCase()); + // Pass the key through unchanged. `normalizeKey` already uppercases + // single characters, and uppercasing here is what used to push + // multi-character names past `KEY_DISPLAY` into raw output. + keys.push(DISPLAY_KEY_ALIASES[part] ?? part); break; } } diff --git a/packages/desktop/e2e/keyboard-shortcut-unassign.spec.ts b/packages/desktop/e2e/keyboard-shortcut-unassign.spec.ts index 1d70dfca5..0744c780d 100644 --- a/packages/desktop/e2e/keyboard-shortcut-unassign.spec.ts +++ b/packages/desktop/e2e/keyboard-shortcut-unassign.spec.ts @@ -51,6 +51,17 @@ async function openRowMenu(page: Page) { await expect(page.getByTestId(`shortcut-bind-${SHORTCUTS_ROW}`)).toBeVisible(); } +/** Reachable from the sidebar even when the cheat sheet's own shortcut is gone. */ +async function openCheatSheet(page: Page) { + await gotoAppShell(page); + await page.getByTestId("sidebar-help").click(); + await expect(page.getByTestId("sidebar-help-menu")).toBeVisible(); + await page.getByTestId("sidebar-help-shortcuts").click(); + const dialog = page.getByTestId("keyboard-shortcuts-dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + return dialog; +} + async function closeRowMenu(page: Page) { await page.keyboard.press("Escape"); await expect(page.getByTestId(`shortcut-bind-${SHORTCUTS_ROW}`)).toHaveCount(0); @@ -120,20 +131,38 @@ test("an unassigned shortcut lists no keys in the shortcuts cheat sheet", async await page.getByTestId(`shortcut-clear-${SHORTCUTS_ROW}`).click(); await expect(page.getByText("Not set", { exact: true })).toBeVisible(); - // Reachable from the sidebar even with its own shortcut unassigned. - await gotoAppShell(page); - await page.getByTestId("sidebar-help").click(); - await expect(page.getByTestId("sidebar-help-menu")).toBeVisible(); - await page.getByTestId("sidebar-help-shortcuts").click(); + const dialog = await openCheatSheet(page); - const dialog = page.getByTestId("keyboard-shortcuts-dialog"); - await expect(dialog).toBeVisible({ timeout: 10_000 }); - - const row = dialog - .locator("div") - .filter({ hasText: /^Show keyboard shortcuts$/ }) - .first(); + // Addressed by testID rather than by filtering on its own text: an anchored + // `hasText` matches only the innermost node holding exactly that label, so a + // row-scoped assertion about anything *else* in the row finds nothing. + const row = dialog.getByTestId(`shortcut-help-row-${SHORTCUTS_ROW}`); await expect(row).toBeVisible(); // No badge pill, blank or otherwise, for a shortcut with no keys. await expect(dialog.getByText("?", { exact: true })).toHaveCount(0); + // It says so, rather than leaving a silent gap where the keys were. Same words + // the settings row uses for the same state. + await expect(row.getByText("Not set", { exact: true })).toBeVisible(); +}); + +test("a rebound shortcut lists its new keys in the shortcuts cheat sheet", async ({ page }) => { + await openShortcutsSettings(page); + + const defaultKeys = page.getByText("?", { exact: true }); + await expect(defaultKeys).toBeVisible(); + + await page.getByTestId(`shortcut-actions-${SHORTCUTS_ROW}`).click(); + await page.getByTestId(`shortcut-bind-${SHORTCUTS_ROW}`).click(); + await page.keyboard.press("Alt+Shift+K"); + await page.getByText("Done", { exact: true }).click(); + + const reboundKeys = page.getByText("⌥+Shift+K", { exact: true }); + await expect(reboundKeys).toBeVisible(); + await expect(defaultKeys).toHaveCount(0); + + const dialog = await openCheatSheet(page); + const row = dialog.getByTestId(`shortcut-help-row-${SHORTCUTS_ROW}`); + await expect(row.getByText("⌥+Shift+K", { exact: true })).toBeVisible(); + // The whole point: the cheat sheet stops advertising the shipped default. + await expect(row.getByText("?", { exact: true })).toHaveCount(0); });