diff --git a/apps/desktop/src/lib/__tests__/editor/keyboardShortcuts.spec.ts b/apps/desktop/src/lib/__tests__/editor/keyboardShortcuts.spec.ts index 698b315d5e..93e2c23c92 100644 --- a/apps/desktop/src/lib/__tests__/editor/keyboardShortcuts.spec.ts +++ b/apps/desktop/src/lib/__tests__/editor/keyboardShortcuts.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { eventToModifierOnlyShortcut, eventToShortcut, isExecuteSqlInNewResultTabShortcut, matchesModifierOnlyShortcut, matchesShortcut } from "@/lib/editor/keyboardShortcuts"; +import { formatShortcutDisplay, isMacShortcutPlatform } from "@/lib/editor/shortcutDisplay"; describe("keyboard shortcut matching", () => { it("records modifier-only mouse shortcut settings", () => { @@ -23,23 +24,62 @@ describe("keyboard shortcut matching", () => { }); it("records the plus key without losing it to the separator", () => { - expect(eventToShortcut({ key: "+", ctrlKey: true })).toBe("Mod+Plus"); - expect(eventToShortcut({ key: "+", ctrlKey: true, shiftKey: true })).toBe("Shift+Mod+Plus"); + expect(eventToShortcut({ key: "+", ctrlKey: true }, "Win32")).toBe("Mod+Plus"); + expect(eventToShortcut({ key: "+", ctrlKey: true, shiftKey: true }, "Win32")).toBe("Shift+Mod+Plus"); + }); + + it("keeps Control distinct from Command when recording macOS shortcuts", () => { + const controlShortcut = eventToShortcut({ key: "b", ctrlKey: true }, "MacIntel"); + + expect(controlShortcut).toBe("Ctrl+B"); + expect(formatShortcutDisplay(controlShortcut!, "MacIntel")).toBe("⌃ B"); + expect(matchesShortcut({ key: "b", ctrlKey: true }, controlShortcut!, "MacIntel")).toBe(true); + expect(matchesShortcut({ key: "b", metaKey: true }, controlShortcut!, "MacIntel")).toBe(false); + expect(eventToShortcut({ key: "b", metaKey: true }, "MacIntel")).toBe("Mod+B"); + expect(matchesShortcut({ key: "b", metaKey: true }, "Mod+B", "MacIntel")).toBe(true); + expect(matchesShortcut({ key: "b", ctrlKey: true }, "Mod+B", "MacIntel")).toBe(false); + expect(matchesShortcut({ key: "b", ctrlKey: true, metaKey: true }, "Mod+B", "MacIntel")).toBe(false); + }); + + it("preserves non-macOS Mod recording compatibility", () => { + expect(eventToShortcut({ key: "b", ctrlKey: true }, "Win32")).toBe("Mod+B"); + expect(eventToShortcut({ key: "b", metaKey: true }, "Win32")).toBe("Mod+B"); + expect(matchesShortcut({ key: "b", ctrlKey: true }, "Mod+B", "Win32")).toBe(true); + expect(matchesShortcut({ key: "b", metaKey: true }, "Mod+B", "Win32")).toBe(true); + }); + + it("preserves combined platform modifiers", () => { + const combinedShortcut = eventToShortcut({ key: "b", ctrlKey: true, metaKey: true, shiftKey: true, altKey: true }, "MacIntel"); + + expect(combinedShortcut).toBe("Shift+Ctrl+Mod+Alt+B"); + expect(matchesShortcut({ key: "b", ctrlKey: true, metaKey: true, shiftKey: true, altKey: true }, combinedShortcut!, "MacIntel")).toBe(true); + expect(matchesShortcut({ key: "b", ctrlKey: true, shiftKey: true, altKey: true }, combinedShortcut!, "MacIntel")).toBe(false); + expect(matchesShortcut({ key: "b", metaKey: true, shiftKey: true, altKey: true }, combinedShortcut!, "MacIntel")).toBe(false); + + const nonMacCombinedShortcut = eventToShortcut({ key: "b", ctrlKey: true, metaKey: true, shiftKey: true, altKey: true }, "Win32"); + expect(nonMacCombinedShortcut).toBe("Shift+Mod+Meta+Alt+B"); + expect(matchesShortcut({ key: "b", ctrlKey: true, metaKey: true, shiftKey: true, altKey: true }, nonMacCombinedShortcut!, "Win32")).toBe(true); + expect(matchesShortcut({ key: "b", ctrlKey: true, shiftKey: true, altKey: true }, nonMacCombinedShortcut!, "Win32")).toBe(false); + expect(matchesShortcut({ key: "b", metaKey: true, shiftKey: true, altKey: true }, nonMacCombinedShortcut!, "Win32")).toBe(false); }); it("matches canonical plus-key shortcuts", () => { - expect(matchesShortcut({ key: "+", ctrlKey: true }, "Mod+Plus")).toBe(true); - expect(matchesShortcut({ key: "+", ctrlKey: true, shiftKey: true }, "Shift+Mod+Plus")).toBe(true); + expect(matchesShortcut({ key: "+", ctrlKey: true }, "Mod+Plus", "Win32")).toBe(true); + expect(matchesShortcut({ key: "+", ctrlKey: true, shiftKey: true }, "Shift+Mod+Plus", "Win32")).toBe(true); }); it("matches the configurable execute-in-new-result-tab shortcut", () => { - expect(isExecuteSqlInNewResultTabShortcut({ key: "\\", ctrlKey: true }, { executeSqlInNewResultTab: "Mod+\\" })).toBe(true); + const isMac = isMacShortcutPlatform(); + const platformModEvent = isMac ? { key: "\\", metaKey: true } : { key: "\\", ctrlKey: true }; + + expect(isExecuteSqlInNewResultTabShortcut(platformModEvent, { executeSqlInNewResultTab: "Mod+\\" })).toBe(true); + expect(isExecuteSqlInNewResultTabShortcut({ ...platformModEvent, shiftKey: true }, { executeSqlInNewResultTab: "Mod+\\" })).toBe(false); + expect(isExecuteSqlInNewResultTabShortcut({ key: "\\", ctrlKey: true }, { executeSqlInNewResultTab: "Mod+\\" })).toBe(!isMac); expect(isExecuteSqlInNewResultTabShortcut({ key: "\\", metaKey: true }, { executeSqlInNewResultTab: "Mod+\\" })).toBe(true); - expect(isExecuteSqlInNewResultTabShortcut({ key: "\\", ctrlKey: true, shiftKey: true }, { executeSqlInNewResultTab: "Mod+\\" })).toBe(false); }); it("matches legacy plus-key shortcuts saved with plus as a separator", () => { - expect(matchesShortcut({ key: "+", ctrlKey: true }, "Mod++")).toBe(true); - expect(matchesShortcut({ key: "+", ctrlKey: true, shiftKey: true }, "Shift+Mod++")).toBe(true); + expect(matchesShortcut({ key: "+", ctrlKey: true }, "Mod++", "Win32")).toBe(true); + expect(matchesShortcut({ key: "+", ctrlKey: true, shiftKey: true }, "Shift+Mod++", "Win32")).toBe(true); }); }); diff --git a/apps/desktop/src/lib/editor/keyboardShortcuts.ts b/apps/desktop/src/lib/editor/keyboardShortcuts.ts index 33d87349b9..eaa5eb09c3 100644 --- a/apps/desktop/src/lib/editor/keyboardShortcuts.ts +++ b/apps/desktop/src/lib/editor/keyboardShortcuts.ts @@ -37,7 +37,7 @@ function shortcutKeyName(key: string): string | null { return key; } -export function eventToShortcut(event: ShortcutLikeEvent): string | null { +export function eventToShortcut(event: ShortcutLikeEvent, platform = globalThis.navigator?.platform || ""): string | null { if (event.isComposing) return null; const key = shortcutKeyName(event.key); @@ -46,9 +46,17 @@ export function eventToShortcut(event: ShortcutLikeEvent): string | null { const hasModifier = !!event.metaKey || !!event.ctrlKey || !!event.altKey || !!event.shiftKey; if (!hasModifier && event.key.length === 1 && event.key !== " ") return null; + const isMac = isMacShortcutPlatform(platform); const parts: string[] = []; if (event.shiftKey) parts.push("Shift"); - if (event.metaKey || event.ctrlKey) parts.push("Mod"); + if (isMac) { + if (event.ctrlKey) parts.push("Ctrl"); + if (event.metaKey) parts.push("Mod"); + } else if (event.ctrlKey && event.metaKey) { + parts.push("Mod", "Meta"); + } else if (event.ctrlKey || event.metaKey) { + parts.push("Mod"); + } if (event.altKey) parts.push("Alt"); parts.push(key); return parts.join("+"); @@ -87,7 +95,13 @@ export function matchesShortcut(event: ShortcutLikeEvent, shortcut: string, plat const usesCtrl = modifiers.has("Ctrl"); if (usesMod) { - if (!event.metaKey && !event.ctrlKey) return false; + if (isMacShortcutPlatform(platform)) { + if (!event.metaKey || !!event.ctrlKey !== usesCtrl) return false; + } else { + if (!event.metaKey && !event.ctrlKey) return false; + if (usesCtrl && (!event.ctrlKey || !event.metaKey)) return false; + if (usesMeta && (!event.metaKey || !event.ctrlKey)) return false; + } } else { if (!!event.metaKey !== usesMeta) return false; if (!!event.ctrlKey !== usesCtrl) return false; diff --git a/packages/app-tests/keyboardShortcuts.test.ts b/packages/app-tests/keyboardShortcuts.test.ts index 9e1b2d0b42..df9d7ed19f 100644 --- a/packages/app-tests/keyboardShortcuts.test.ts +++ b/packages/app-tests/keyboardShortcuts.test.ts @@ -1,5 +1,5 @@ import { strict as assert } from "node:assert"; -import { test } from "vitest"; +import { afterAll, beforeAll, test, vi } from "vitest"; import { eventToShortcut, isBrowserReloadShortcut, @@ -30,6 +30,9 @@ import { } from "../../apps/desktop/src/lib/editor/keyboardShortcuts.ts"; import { shortcutToCodeMirrorKey } from "../../apps/desktop/src/lib/editor/shortcutRegistry.ts"; +beforeAll(() => vi.stubGlobal("navigator", { platform: "Linux x86_64" })); +afterAll(() => vi.unstubAllGlobals()); + test("matches Cmd+Enter for SQL execution", () => { assert.equal(isExecuteSqlShortcut({ key: "Enter", metaKey: true }), true); }); @@ -48,7 +51,11 @@ test("matches custom shortcut settings for SQL execution", () => { }); test("records custom shortcuts from keydown events", () => { - assert.equal(eventToShortcut({ key: "r", metaKey: true, shiftKey: true } as any), "Shift+Mod+R"); + assert.equal(eventToShortcut({ key: "r", metaKey: true, shiftKey: true } as any, "MacIntel"), "Shift+Mod+R"); + assert.equal(eventToShortcut({ key: "r", ctrlKey: true, shiftKey: true } as any, "MacIntel"), "Shift+Ctrl+R"); + assert.equal(eventToShortcut({ key: "r", ctrlKey: true, shiftKey: true } as any, "Win32"), "Shift+Mod+R"); + assert.equal(eventToShortcut({ key: "r", metaKey: true, shiftKey: true } as any, "Win32"), "Shift+Mod+R"); + assert.equal(eventToShortcut({ key: "r", ctrlKey: true, metaKey: true, shiftKey: true } as any, "Win32"), "Shift+Mod+Meta+R"); assert.equal(eventToShortcut({ key: "F2" } as any), "F2"); assert.equal(eventToShortcut({ key: "Control", ctrlKey: true } as any), null); });