From 87948e956a14bb3fe53da0557bc66de3b459115e Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 8 Mar 2026 16:36:05 +0700 Subject: [PATCH] refactor keyboard shortcut tests into table-driven suites --- .../src/keyboard/keyboard-shortcuts.test.ts | 582 ++++++++---------- 1 file changed, 244 insertions(+), 338 deletions(-) diff --git a/packages/app/src/keyboard/keyboard-shortcuts.test.ts b/packages/app/src/keyboard/keyboard-shortcuts.test.ts index f9af3a86e..2c1d9c19b 100644 --- a/packages/app/src/keyboard/keyboard-shortcuts.test.ts +++ b/packages/app/src/keyboard/keyboard-shortcuts.test.ts @@ -31,306 +31,221 @@ function shortcutContext( }; } +function expectShortcutResolution(input: { + event: Partial; + context?: Partial; + action: string; + payload?: unknown; + preventDefault?: boolean; + stopPropagation?: boolean; +}) { + const match = resolveKeyboardShortcut({ + event: keyboardEvent(input.event), + context: shortcutContext(input.context), + }); + + expect(match?.action).toBe(input.action); + if ("payload" in input) { + expect(match?.payload).toEqual(input.payload); + } + expect(match?.preventDefault).toBe(input.preventDefault ?? true); + expect(match?.stopPropagation).toBe(input.stopPropagation ?? true); +} + +function expectNoShortcutResolution(input: { + event: Partial; + context?: Partial; +}) { + const match = resolveKeyboardShortcut({ + event: keyboardEvent(input.event), + context: shortcutContext(input.context), + }); + + expect(match).toBeNull(); +} + +type MatchingShortcutCase = { + name: string; + event: Partial; + context?: Partial; + action: string; + payload?: unknown; + preventDefault?: boolean; + stopPropagation?: boolean; +}; + +type NonMatchingShortcutCase = { + name: string; + event: Partial; + context?: Partial; +}; + +type HelpSectionCase = { + name: string; + context: { + isMac: boolean; + isTauri: boolean; + }; + expectedKeys: Record; +}; + describe("keyboard-shortcuts", () => { - it("matches Mod+Shift+O to create new agent", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "O", - code: "KeyO", - metaKey: true, - shiftKey: true, - }), - context: shortcutContext({ isMac: true }), - }); + const matchingCases: MatchingShortcutCase[] = [ + { + name: "matches Mod+Shift+O to create new agent", + event: { key: "O", code: "KeyO", metaKey: true, shiftKey: true }, + context: { isMac: true }, + action: "agent.new", + }, + { + name: "matches question-mark shortcut to toggle the shortcuts dialog", + event: { key: "?", code: "Slash", shiftKey: true }, + context: { focusScope: "other" }, + action: "shortcuts.dialog.toggle", + }, + { + name: "matches workspace index jump on web via Alt+digit", + event: { key: "2", code: "Digit2", altKey: true }, + context: { isTauri: false }, + action: "workspace.navigate.index", + payload: { index: 2 }, + }, + { + name: "matches workspace index jump on tauri via Mod+digit", + event: { key: "2", code: "Digit2", metaKey: true }, + context: { isMac: true, isTauri: true }, + action: "workspace.navigate.index", + payload: { index: 2 }, + }, + { + name: "matches tab index jump on tauri via Alt+digit", + event: { key: "2", code: "Digit2", altKey: true }, + context: { isTauri: true }, + action: "workspace.tab.navigate.index", + payload: { index: 2 }, + }, + { + name: "matches tab index jump on web via Alt+Shift+digit", + event: { key: "@", code: "Digit2", altKey: true, shiftKey: true }, + context: { isTauri: false }, + action: "workspace.tab.navigate.index", + payload: { index: 2 }, + }, + { + name: "matches workspace relative navigation on web via Alt+[", + event: { key: "[", code: "BracketLeft", altKey: true }, + context: { isTauri: false }, + action: "workspace.navigate.relative", + payload: { delta: -1 }, + }, + { + name: "matches workspace relative navigation on tauri via Mod+]", + event: { key: "]", code: "BracketRight", ctrlKey: true }, + context: { isTauri: true }, + action: "workspace.navigate.relative", + payload: { delta: 1 }, + }, + { + name: "matches tab relative navigation via Alt+Shift+]", + event: { key: "}", code: "BracketRight", altKey: true, shiftKey: true }, + action: "workspace.tab.navigate.relative", + payload: { delta: 1 }, + }, + { + name: "matches Alt+Shift+T to open new tab", + event: { key: "T", code: "KeyT", altKey: true, shiftKey: true }, + action: "workspace.tab.new", + }, + { + name: "matches Alt+Shift+W to close current tab on web", + event: { key: "W", code: "KeyW", altKey: true, shiftKey: true }, + context: { isTauri: false }, + action: "workspace.tab.close.current", + }, + { + name: "matches Mod+W to close current tab on tauri", + event: { key: "w", code: "KeyW", metaKey: true }, + context: { isMac: true, isTauri: true }, + action: "workspace.tab.close.current", + }, + { + name: "matches Cmd+B sidebar toggle on macOS", + event: { key: "b", code: "KeyB", metaKey: true }, + context: { isMac: true }, + action: "sidebar.toggle.left", + }, + { + name: "keeps Mod+. as sidebar toggle fallback", + event: { key: ".", code: "Period", ctrlKey: true }, + context: { isMac: false }, + action: "sidebar.toggle.left", + }, + { + name: "routes Mod+D to message-input action outside terminal", + event: { key: "d", code: "KeyD", metaKey: true }, + context: { isMac: true, focusScope: "message-input" }, + action: "message-input.action", + payload: { kind: "dictation-toggle" }, + }, + { + name: "routes space to voice mute toggle outside editable scopes", + event: { key: " ", code: "Space" }, + context: { focusScope: "other" }, + action: "message-input.action", + payload: { kind: "voice-mute-toggle" }, + }, + { + name: "lets Escape continue to local handlers while routing dictation cancel", + event: { key: "Escape", code: "Escape" }, + context: { focusScope: "message-input" }, + action: "message-input.action", + payload: { kind: "dictation-cancel" }, + preventDefault: false, + stopPropagation: false, + }, + ]; - expect(match?.action).toBe("agent.new"); + it.each(matchingCases)("$name", ({ event, context, action, payload, preventDefault, stopPropagation }) => { + expectShortcutResolution({ + event, + context, + action, + ...(payload !== undefined ? { payload } : {}), + ...(preventDefault !== undefined ? { preventDefault } : {}), + ...(stopPropagation !== undefined ? { stopPropagation } : {}), + }); }); - it("does not keep old Mod+Alt+N binding", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "n", - code: "KeyN", - metaKey: true, - altKey: true, - }), - context: shortcutContext({ isMac: true }), - }); + const nonMatchingCases: NonMatchingShortcutCase[] = [ + { + name: "does not keep old Mod+Alt+N binding", + event: { key: "n", code: "KeyN", metaKey: true, altKey: true }, + context: { isMac: true }, + }, + { + name: "does not match question-mark shortcut inside editable scopes", + event: { key: "?", code: "Slash", shiftKey: true }, + context: { focusScope: "message-input" }, + }, + { + name: "does not bind Ctrl+B on non-mac", + event: { key: "b", code: "KeyB", ctrlKey: true }, + context: { isMac: false }, + }, + { + name: "does not route message-input actions when terminal is focused", + event: { key: "d", code: "KeyD", metaKey: true }, + context: { isMac: true, focusScope: "terminal" }, + }, + { + name: "keeps space typing available in message input", + event: { key: " ", code: "Space" }, + context: { focusScope: "message-input" }, + }, + ]; - expect(match).toBeNull(); - }); - - it("matches question-mark shortcut to toggle the shortcuts dialog", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "?", - code: "Slash", - shiftKey: true, - }), - context: shortcutContext({ focusScope: "other" }), - }); - - expect(match?.action).toBe("shortcuts.dialog.toggle"); - }); - - it("does not match question-mark shortcut inside editable scopes", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "?", - code: "Slash", - shiftKey: true, - }), - context: shortcutContext({ focusScope: "message-input" }), - }); - - expect(match).toBeNull(); - }); - - it("matches workspace index jump on web via Alt+digit", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "2", - code: "Digit2", - altKey: true, - }), - context: shortcutContext({ isTauri: false }), - }); - - expect(match?.action).toBe("workspace.navigate.index"); - expect(match?.payload).toEqual({ index: 2 }); - }); - - it("matches workspace index jump on tauri via Mod+digit", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "2", - code: "Digit2", - metaKey: true, - }), - context: shortcutContext({ isMac: true, isTauri: true }), - }); - - expect(match?.action).toBe("workspace.navigate.index"); - expect(match?.payload).toEqual({ index: 2 }); - }); - - it("matches tab index jump on tauri via Alt+digit", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "2", - code: "Digit2", - altKey: true, - }), - context: shortcutContext({ isTauri: true }), - }); - - expect(match?.action).toBe("workspace.tab.navigate.index"); - expect(match?.payload).toEqual({ index: 2 }); - }); - - it("matches tab index jump on web via Alt+Shift+digit", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "@", - code: "Digit2", - altKey: true, - shiftKey: true, - }), - context: shortcutContext({ isTauri: false }), - }); - - expect(match?.action).toBe("workspace.tab.navigate.index"); - expect(match?.payload).toEqual({ index: 2 }); - }); - - it("matches workspace relative navigation on web via Alt+[", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "[", - code: "BracketLeft", - altKey: true, - }), - context: shortcutContext({ isTauri: false }), - }); - - expect(match?.action).toBe("workspace.navigate.relative"); - expect(match?.payload).toEqual({ delta: -1 }); - }); - - it("matches workspace relative navigation on tauri via Mod+]", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "]", - code: "BracketRight", - ctrlKey: true, - }), - context: shortcutContext({ isTauri: true }), - }); - - expect(match?.action).toBe("workspace.navigate.relative"); - expect(match?.payload).toEqual({ delta: 1 }); - }); - - it("matches tab relative navigation via Alt+Shift+]", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "}", - code: "BracketRight", - altKey: true, - shiftKey: true, - }), - context: shortcutContext(), - }); - - expect(match?.action).toBe("workspace.tab.navigate.relative"); - expect(match?.payload).toEqual({ delta: 1 }); - }); - - it("matches Alt+Shift+T to open new tab", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "T", - code: "KeyT", - altKey: true, - shiftKey: true, - }), - context: shortcutContext(), - }); - - expect(match?.action).toBe("workspace.tab.new"); - }); - - it("matches Alt+Shift+W to close current tab on web", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "W", - code: "KeyW", - altKey: true, - shiftKey: true, - }), - context: shortcutContext({ isTauri: false }), - }); - - expect(match?.action).toBe("workspace.tab.close.current"); - }); - - it("matches Mod+W to close current tab on tauri", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "w", - code: "KeyW", - metaKey: true, - }), - context: shortcutContext({ isMac: true, isTauri: true }), - }); - - expect(match?.action).toBe("workspace.tab.close.current"); - }); - - it("matches Cmd+B sidebar toggle on macOS", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "b", - code: "KeyB", - metaKey: true, - }), - context: shortcutContext({ isMac: true }), - }); - - expect(match?.action).toBe("sidebar.toggle.left"); - }); - - it("does not bind Ctrl+B on non-mac", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "b", - code: "KeyB", - ctrlKey: true, - }), - context: shortcutContext({ isMac: false }), - }); - - expect(match).toBeNull(); - }); - - it("keeps Mod+. as sidebar toggle fallback", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: ".", - code: "Period", - ctrlKey: true, - }), - context: shortcutContext({ isMac: false }), - }); - - expect(match?.action).toBe("sidebar.toggle.left"); - }); - - it("routes Mod+D to message-input action outside terminal", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "d", - code: "KeyD", - metaKey: true, - }), - context: shortcutContext({ isMac: true, focusScope: "message-input" }), - }); - - expect(match?.action).toBe("message-input.action"); - expect(match?.payload).toEqual({ kind: "dictation-toggle" }); - }); - - it("does not route message-input actions when terminal is focused", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "d", - code: "KeyD", - metaKey: true, - }), - context: shortcutContext({ isMac: true, focusScope: "terminal" }), - }); - - expect(match).toBeNull(); - }); - - it("keeps space typing available in message input", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: " ", - code: "Space", - }), - context: shortcutContext({ focusScope: "message-input" }), - }); - - expect(match).toBeNull(); - }); - - it("routes space to voice mute toggle outside editable scopes", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: " ", - code: "Space", - }), - context: shortcutContext({ focusScope: "other" }), - }); - - expect(match?.action).toBe("message-input.action"); - expect(match?.payload).toEqual({ kind: "voice-mute-toggle" }); - }); - - it("lets Escape continue to local handlers while routing dictation cancel", () => { - const match = resolveKeyboardShortcut({ - event: keyboardEvent({ - key: "Escape", - code: "Escape", - }), - context: shortcutContext({ focusScope: "message-input" }), - }); - - expect(match?.action).toBe("message-input.action"); - expect(match?.payload).toEqual({ kind: "dictation-cancel" }); - expect(match?.preventDefault).toBe(false); - expect(match?.stopPropagation).toBe(false); + it.each(nonMatchingCases)("$name", ({ event, context }) => { + expectNoShortcutResolution({ event, context }); }); }); @@ -348,50 +263,41 @@ describe("keyboard-shortcut help sections", () => { return null; } - it("uses web defaults for workspace and tab jump", () => { - const sections = buildKeyboardShortcutHelpSections({ - isMac: true, - isTauri: false, - }); + const helpCases: HelpSectionCase[] = [ + { + name: "uses web defaults for workspace and tab jump", + context: { isMac: true, isTauri: false }, + expectedKeys: { + "new-agent": ["mod", "shift", "O"], + "workspace-jump-index": ["alt", "1-9"], + "workspace-tab-jump-index": ["alt", "shift", "1-9"], + "workspace-tab-close-current": ["alt", "shift", "W"], + }, + }, + { + name: "uses tauri defaults for workspace and tab jump", + context: { isMac: true, isTauri: true }, + expectedKeys: { + "new-agent": ["mod", "shift", "O"], + "workspace-jump-index": ["mod", "1-9"], + "workspace-tab-jump-index": ["alt", "1-9"], + "workspace-tab-close-current": ["mod", "W"], + }, + }, + { + name: "uses mod+period as non-mac left sidebar shortcut", + context: { isMac: false, isTauri: false }, + expectedKeys: { + "toggle-left-sidebar": ["mod", "."], + }, + }, + ]; - expect(findRow(sections, "new-agent")?.keys).toEqual(["mod", "shift", "O"]); - expect(findRow(sections, "workspace-jump-index")?.keys).toEqual(["alt", "1-9"]); - expect(findRow(sections, "workspace-tab-jump-index")?.keys).toEqual([ - "alt", - "shift", - "1-9", - ]); - expect(findRow(sections, "workspace-tab-close-current")?.keys).toEqual([ - "alt", - "shift", - "W", - ]); - }); + it.each(helpCases)("$name", ({ context, expectedKeys }) => { + const sections = buildKeyboardShortcutHelpSections(context); - it("uses tauri defaults for workspace and tab jump", () => { - const sections = buildKeyboardShortcutHelpSections({ - isMac: true, - isTauri: true, - }); - - expect(findRow(sections, "new-agent")?.keys).toEqual(["mod", "shift", "O"]); - expect(findRow(sections, "workspace-jump-index")?.keys).toEqual(["mod", "1-9"]); - expect(findRow(sections, "workspace-tab-jump-index")?.keys).toEqual(["alt", "1-9"]); - expect(findRow(sections, "workspace-tab-close-current")?.keys).toEqual([ - "mod", - "W", - ]); - }); - - it("uses mod+period as non-mac left sidebar shortcut", () => { - const sections = buildKeyboardShortcutHelpSections({ - isMac: false, - isTauri: false, - }); - - expect(findRow(sections, "toggle-left-sidebar")?.keys).toEqual([ - "mod", - ".", - ]); + for (const [id, keys] of Object.entries(expectedKeys)) { + expect(findRow(sections, id)?.keys).toEqual(keys); + } }); });