From c0b801b80590d6bc75a72ca6de501d065c193cc7 Mon Sep 17 00:00:00 2001 From: OnCloud Date: Tue, 14 Jul 2026 19:30:32 +0800 Subject: [PATCH] Fix shortcut capture ignoring - = ; ' keys (#2047) * Fix shortcut capture ignoring - = ; ' keys KEY_MAP was missing Minus/Equal/Semicolon/Quote, so keyboardEventToComboString returned null for those physical keys and the capture UI silently dropped them. Add the four entries with their shifted variants. Also suppress the Electron View > Zoom accelerators (Cmd+-/=/0) while capturing, so those combos reach the renderer instead of zooming the window. Driven by the existing capturingShortcut flag via a new paseo:menu:set-capturing-shortcut IPC. * fix: address review feedback from PR #2047 Trim the capturing useEffect so it only fires when capture is active (removes the redundant on-mount false and the double-fire on true->false), and reset the main-process capturingShortcut flag on renderer reload via browser-window-created + did-finish-load so a Cmd+R mid-capture can't leave the zoom accelerators disabled. --------- Co-authored-by: Mohamed Boudra --- packages/app/src/desktop/host.ts | 1 + .../app/src/keyboard/shortcut-string.test.ts | 70 +++++++++++++++++++ packages/app/src/keyboard/shortcut-string.ts | 4 ++ .../settings/keyboard-shortcuts-section.tsx | 13 ++++ packages/desktop/src/features/menu.ts | 39 ++++++++++- packages/desktop/src/preload.ts | 2 + 6 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 packages/app/src/keyboard/shortcut-string.test.ts diff --git a/packages/app/src/desktop/host.ts b/packages/app/src/desktop/host.ts index 6b3f0832b..f76a2c528 100644 --- a/packages/app/src/desktop/host.ts +++ b/packages/app/src/desktop/host.ts @@ -85,6 +85,7 @@ export interface DesktopWebUtilsBridge { export interface DesktopMenuBridge { showContextMenu?: (input?: { kind?: "terminal"; hasSelection?: boolean }) => Promise; + setCapturingShortcut?: (capturing: boolean) => Promise; } export interface DesktopWindowControlsOverlayUpdate { diff --git a/packages/app/src/keyboard/shortcut-string.test.ts b/packages/app/src/keyboard/shortcut-string.test.ts new file mode 100644 index 000000000..345a2b73e --- /dev/null +++ b/packages/app/src/keyboard/shortcut-string.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import { + keyComboToString, + keyboardEventToComboString, + parseShortcutString, +} from "./shortcut-string"; + +function keyboardEvent(overrides: Partial): KeyboardEvent { + return { + key: "", + code: "", + altKey: false, + ctrlKey: false, + metaKey: false, + shiftKey: false, + repeat: false, + ...overrides, + } as KeyboardEvent; +} + +describe("keyboardEventToComboString", () => { + it("captures the minus key (regression: previously returned null)", () => { + expect(keyboardEventToComboString(keyboardEvent({ key: "-", code: "Minus" }))).toBe("-"); + }); + + it("captures minus with modifiers", () => { + expect( + keyboardEventToComboString(keyboardEvent({ key: "-", code: "Minus", metaKey: true })), + ).toBe("Cmd+-"); + expect( + keyboardEventToComboString(keyboardEvent({ key: "_", code: "Minus", shiftKey: true })), + ).toBe("Shift+-"); + }); + + it("captures equal, semicolon, and quote", () => { + expect(keyboardEventToComboString(keyboardEvent({ key: "=", code: "Equal" }))).toBe("="); + expect(keyboardEventToComboString(keyboardEvent({ key: ";", code: "Semicolon" }))).toBe(";"); + expect(keyboardEventToComboString(keyboardEvent({ key: "'", code: "Quote" }))).toBe("'"); + }); + + it("still returns null for modifier-only presses", () => { + expect(keyboardEventToComboString(keyboardEvent({ code: "ShiftLeft" }))).toBeNull(); + }); +}); + +describe("parseShortcutString round-trips punctuation keys", () => { + const cases: ReadonlyArray<[string, string, string, string]> = [ + ["-", "Minus", "-", "_"], + ["=", "Equal", "=", "+"], + [";", "Semicolon", ";", ":"], + ["'", "Quote", "'", '"'], + ]; + + for (const [humanKey, code, key, shiftedKey] of cases) { + it(`round-trips ${humanKey}`, () => { + const combo = parseShortcutString(humanKey); + expect(combo.code).toBe(code); + expect(combo.key).toBe(key); + expect(combo.shiftedKey).toBe(shiftedKey); + expect(keyComboToString(combo)).toBe(humanKey); + }); + } + + it("round-trips a modifier combo with minus", () => { + const combo = parseShortcutString("Cmd+-"); + expect(combo.meta).toBe(true); + expect(combo.code).toBe("Minus"); + expect(keyComboToString(combo)).toBe("Cmd+-"); + }); +}); diff --git a/packages/app/src/keyboard/shortcut-string.ts b/packages/app/src/keyboard/shortcut-string.ts index c7448a74e..eb0f8b4bc 100644 --- a/packages/app/src/keyboard/shortcut-string.ts +++ b/packages/app/src/keyboard/shortcut-string.ts @@ -42,9 +42,13 @@ KEY_MAP["9"].shiftedKey = "("; KEY_MAP["0"].shiftedKey = ")"; KEY_MAP["Digit"] = { code: "Digit" }; +KEY_MAP["-"] = { code: "Minus", key: "-", shiftedKey: "_" }; +KEY_MAP["="] = { code: "Equal", key: "=", shiftedKey: "+" }; KEY_MAP["\\"] = { code: "Backslash", key: "\\", shiftedKey: "|" }; KEY_MAP["["] = { code: "BracketLeft", key: "[", shiftedKey: "{" }; KEY_MAP["]"] = { code: "BracketRight", key: "]", shiftedKey: "}" }; +KEY_MAP[";"] = { code: "Semicolon", key: ";", shiftedKey: ":" }; +KEY_MAP["'"] = { code: "Quote", key: "'", shiftedKey: '"' }; KEY_MAP[","] = { code: "Comma", key: ",", shiftedKey: "<" }; KEY_MAP["."] = { code: "Period", key: ".", shiftedKey: ">" }; KEY_MAP["`"] = { code: "Backquote", key: "`", shiftedKey: "~" }; diff --git a/packages/app/src/screens/settings/keyboard-shortcuts-section.tsx b/packages/app/src/screens/settings/keyboard-shortcuts-section.tsx index eaa4bf08e..de1323df5 100644 --- a/packages/app/src/screens/settings/keyboard-shortcuts-section.tsx +++ b/packages/app/src/screens/settings/keyboard-shortcuts-section.tsx @@ -23,6 +23,7 @@ import { useKeyboardShortcutsStore } from "@/stores/keyboard-shortcuts-store"; import { getShortcutOs } from "@/utils/shortcut-platform"; import { getIsElectronRuntime } from "@/constants/layout"; import { isNative } from "@/constants/platform"; +import { getDesktopHost } from "@/desktop/host"; const EMPTY_CAPTURED_COMBOS: string[] = []; @@ -169,6 +170,7 @@ export function KeyboardShortcutsSection() { const { overrides, hasOverrides, setOverride, removeOverride, resetAll } = useKeyboardShortcutOverrides(); const setCapturingShortcut = useKeyboardShortcutsStore((s) => s.setCapturingShortcut); + const capturing = useKeyboardShortcutsStore((s) => s.capturingShortcut); const isFocused = useIsFocused(); const isMac = getShortcutOs() === "mac"; @@ -242,6 +244,17 @@ export function KeyboardShortcutsSection() { }; }, [setCapturingShortcut]); + // Suppress desktop zoom accelerators while capturing so combos like Cmd+- are + // recorded instead of zooming the window. No-op outside Electron. + useEffect(() => { + if (isNative || !capturing) return; + const menu = getDesktopHost()?.menu; + void menu?.setCapturingShortcut?.(true); + return () => { + void menu?.setCapturingShortcut?.(false); + }; + }, [capturing]); + const handleResetAll = useCallback(() => void resetAll(), [resetAll]); const handleRemoveOverride = useCallback( (bindingId: string) => void removeOverride(bindingId), diff --git a/packages/desktop/src/features/menu.ts b/packages/desktop/src/features/menu.ts index c086c2a24..d55f053a5 100644 --- a/packages/desktop/src/features/menu.ts +++ b/packages/desktop/src/features/menu.ts @@ -47,8 +47,10 @@ function reloadFocusedContentsOrWindow(win: BrowserWindow, options?: { ignoreCac function buildApplicationMenuTemplate( options: ApplicationMenuOptions, + capturing: boolean, ): Electron.MenuItemConstructorOptions[] { const isMac = process.platform === "darwin"; + const zoomEnabled = !capturing; return [ ...(isMac @@ -99,6 +101,7 @@ function buildApplicationMenuTemplate( { label: "Zoom In", accelerator: "CmdOrCtrl+=", + enabled: zoomEnabled, click: withBrowserWindow((win) => { win.webContents.setZoomLevel(win.webContents.getZoomLevel() + 0.5); }), @@ -106,6 +109,7 @@ function buildApplicationMenuTemplate( { label: "Zoom Out", accelerator: "CmdOrCtrl+-", + enabled: zoomEnabled, click: withBrowserWindow((win) => { win.webContents.setZoomLevel(win.webContents.getZoomLevel() - 0.5); }), @@ -113,6 +117,7 @@ function buildApplicationMenuTemplate( { label: "Actual Size", accelerator: "CmdOrCtrl+0", + enabled: zoomEnabled, click: withBrowserWindow((win) => { win.webContents.setZoomLevel(0); }), @@ -150,9 +155,20 @@ function buildApplicationMenuTemplate( ]; } -export function setupApplicationMenu(options: ApplicationMenuOptions): void { - const menu = Menu.buildFromTemplate(buildApplicationMenuTemplate(options)); +let applicationMenuOptions: ApplicationMenuOptions | null = null; +let capturingShortcut = false; + +function rebuildApplicationMenu(): void { + if (!applicationMenuOptions) return; + const menu = Menu.buildFromTemplate( + buildApplicationMenuTemplate(applicationMenuOptions, capturingShortcut), + ); Menu.setApplicationMenu(menu); +} + +export function setupApplicationMenu(options: ApplicationMenuOptions): void { + applicationMenuOptions = options; + rebuildApplicationMenu(); ipcMain.handle("paseo:menu:showContextMenu", (event, input?: ShowContextMenuInput) => { const win = BrowserWindow.fromWebContents(event.sender); @@ -185,4 +201,23 @@ export function setupApplicationMenu(options: ApplicationMenuOptions): void { contextMenu.popup({ window: win }); }); + + // Disable the zoom accelerators while capturing a shortcut so combos like + // Cmd+- / Cmd+= reach the renderer instead of zooming the window. + ipcMain.handle("paseo:menu:set-capturing-shortcut", (_event, capturing?: boolean) => { + capturingShortcut = capturing === true; + rebuildApplicationMenu(); + }); + + // If the renderer reloads mid-capture (e.g. Cmd+R) the renderer-side effect + // never gets to send `false`, so reset the flag from the main process when a + // main window finishes loading. Workspace browser webviews are not + // BrowserWindows, so they don't trigger this. + app.on("browser-window-created", (_event, win) => { + win.webContents.on("did-finish-load", () => { + if (!capturingShortcut) return; + capturingShortcut = false; + rebuildApplicationMenu(); + }); + }); } diff --git a/packages/desktop/src/preload.ts b/packages/desktop/src/preload.ts index 84e5dc8bd..1ce664be2 100644 --- a/packages/desktop/src/preload.ts +++ b/packages/desktop/src/preload.ts @@ -72,6 +72,8 @@ contextBridge.exposeInMainWorld("paseoDesktop", { menu: { showContextMenu: (input?: Record) => ipcRenderer.invoke("paseo:menu:showContextMenu", input), + setCapturingShortcut: (capturing: boolean) => + ipcRenderer.invoke("paseo:menu:set-capturing-shortcut", capturing), }, browser: { registerWorkspaceBrowser: (input: { browserId: string; workspaceId: string }) =>