diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index d128fd184..f26b3b104 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -7,6 +7,7 @@ import { setupAutoUpdater } from "./updater"; import { setupDaemonManager } from "./daemon-manager"; import { openExternalSafely, downloadURLSafely } from "./external-url"; import { installContextMenu } from "./context-menu"; +import { handleAppShortcut } from "./keyboard-shortcuts"; import { getAppVersion } from "./app-version"; import { loadRuntimeConfig } from "./runtime-config-loader"; import type { RuntimeConfigResult } from "../shared/runtime-config"; @@ -189,19 +190,13 @@ function createWindow(): void { return { action: "deny" }; }); - // Prevent Cmd+R / Ctrl+R / Shift+Cmd+R / Shift+Ctrl+R / F5 from - // reloading the page. In a desktop app an accidental reload destroys - // in-memory state (tabs, drafts, WS connections) with no URL bar to - // navigate back. DevTools refresh (via the DevTools UI) still works. - mainWindow.webContents.on("before-input-event", (_event, input) => { - if (input.type !== "keyDown") return; - const cmdOrCtrl = - process.platform === "darwin" ? input.meta : input.control; - if ( - (cmdOrCtrl && input.key.toLowerCase() === "r") || - input.key === "F5" - ) { - _event.preventDefault(); + // Window-level keyboard shortcuts. Calling preventDefault here prevents + // both the renderer keydown AND the application menu accelerator, so + // anything we own here (reload-block, zoom) is the sole handler for + // that combination — no double-fire with the macOS default View menu. + mainWindow.webContents.on("before-input-event", (event, input) => { + if (handleAppShortcut(input, mainWindow!.webContents)) { + event.preventDefault(); } }); diff --git a/apps/desktop/src/main/keyboard-shortcuts.test.ts b/apps/desktop/src/main/keyboard-shortcuts.test.ts new file mode 100644 index 000000000..53e04d682 --- /dev/null +++ b/apps/desktop/src/main/keyboard-shortcuts.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it, vi } from "vitest"; +import { handleAppShortcut, type ShortcutInput } from "./keyboard-shortcuts"; + +function makeWc(initialLevel = 0) { + let level = initialLevel; + return { + getZoomLevel: vi.fn(() => level), + setZoomLevel: vi.fn((next: number) => { + level = next; + }), + currentLevel: () => level, + }; +} + +function key( + k: string, + mods: Partial> = {}, +): ShortcutInput { + return { + type: "keyDown", + key: k, + control: false, + meta: false, + ...mods, + }; +} + +describe("handleAppShortcut — reload blocking", () => { + it("swallows Cmd+R on macOS", () => { + const wc = makeWc(); + expect(handleAppShortcut(key("r", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.setZoomLevel).not.toHaveBeenCalled(); + }); + + it("swallows Ctrl+R on Linux/Windows", () => { + const wc = makeWc(); + expect(handleAppShortcut(key("r", { control: true }), wc, "linux")).toBe(true); + expect(handleAppShortcut(key("R", { control: true }), wc, "win32")).toBe(true); + }); + + it("swallows F5 regardless of modifier", () => { + const wc = makeWc(); + expect(handleAppShortcut(key("F5"), wc, "darwin")).toBe(true); + }); + + it("ignores non-keyDown events", () => { + const wc = makeWc(); + expect( + handleAppShortcut({ ...key("r", { meta: true }), type: "keyUp" }, wc, "darwin"), + ).toBe(false); + }); +}); + +describe("handleAppShortcut — zoom in", () => { + it("zooms in on Cmd+= (unshifted)", () => { + const wc = makeWc(0); + expect(handleAppShortcut(key("=", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("zooms in on Cmd++ (Shift+=)", () => { + const wc = makeWc(0); + expect(handleAppShortcut(key("+", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("zooms in on Ctrl+= on non-mac", () => { + const wc = makeWc(0); + expect(handleAppShortcut(key("=", { control: true }), wc, "linux")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("does nothing without Cmd/Ctrl", () => { + const wc = makeWc(0); + expect(handleAppShortcut(key("="), wc, "darwin")).toBe(false); + expect(wc.setZoomLevel).not.toHaveBeenCalled(); + }); + + it("clamps zoom-in at the upper bound", () => { + const wc = makeWc(4.5); + expect(handleAppShortcut(key("=", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(4.5); + }); +}); + +describe("handleAppShortcut — zoom out (regression: MUL-2354)", () => { + it("zooms out on Cmd+- (unshifted)", () => { + const wc = makeWc(1); + expect(handleAppShortcut(key("-", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("zooms out on Cmd+_ (Shift+-)", () => { + const wc = makeWc(1); + expect(handleAppShortcut(key("_", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("zooms out on Ctrl+- on non-mac", () => { + const wc = makeWc(1); + expect(handleAppShortcut(key("-", { control: true }), wc, "win32")).toBe(true); + expect(wc.currentLevel()).toBe(0.5); + }); + + it("undoes a prior Cmd+= so the user can return to 100%", () => { + const wc = makeWc(0); + handleAppShortcut(key("=", { meta: true }), wc, "darwin"); + expect(wc.currentLevel()).toBe(0.5); + handleAppShortcut(key("-", { meta: true }), wc, "darwin"); + expect(wc.currentLevel()).toBe(0); + }); + + it("clamps zoom-out at the lower bound", () => { + const wc = makeWc(-3); + expect(handleAppShortcut(key("-", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(-3); + }); + + it("does nothing without Cmd/Ctrl", () => { + const wc = makeWc(1); + expect(handleAppShortcut(key("-"), wc, "darwin")).toBe(false); + expect(wc.setZoomLevel).not.toHaveBeenCalled(); + }); +}); + +describe("handleAppShortcut — reset zoom", () => { + it("resets to 0 on Cmd+0", () => { + const wc = makeWc(2); + expect(handleAppShortcut(key("0", { meta: true }), wc, "darwin")).toBe(true); + expect(wc.currentLevel()).toBe(0); + }); + + it("resets to 0 on Ctrl+0", () => { + const wc = makeWc(-1.5); + expect(handleAppShortcut(key("0", { control: true }), wc, "linux")).toBe(true); + expect(wc.currentLevel()).toBe(0); + }); + + it("ignores plain 0 without modifier", () => { + const wc = makeWc(2); + expect(handleAppShortcut(key("0"), wc, "darwin")).toBe(false); + expect(wc.setZoomLevel).not.toHaveBeenCalled(); + }); +}); + +describe("handleAppShortcut — unrelated keys pass through", () => { + it("does not capture plain letters", () => { + const wc = makeWc(); + expect(handleAppShortcut(key("a", { meta: true }), wc, "darwin")).toBe(false); + expect(handleAppShortcut(key("k", { meta: true }), wc, "darwin")).toBe(false); + }); +}); diff --git a/apps/desktop/src/main/keyboard-shortcuts.ts b/apps/desktop/src/main/keyboard-shortcuts.ts new file mode 100644 index 000000000..ce9458399 --- /dev/null +++ b/apps/desktop/src/main/keyboard-shortcuts.ts @@ -0,0 +1,74 @@ +import type { WebContents } from "electron"; + +// Shape of the input subset we read from Electron's `before-input-event`. +// Modeled as a structural type so the handler is unit-testable without a +// real Electron Input instance. +export type ShortcutInput = { + type: string; + key: string; + control: boolean; + meta: boolean; +}; + +// Subset of WebContents the zoom handler needs. Keeps the test mock tiny. +export type ZoomTarget = Pick; + +// Match Electron's built-in zoomIn/zoomOut roles (Chromium default of 0.5 +// per step). Clamp to a range that keeps the UI legible — values outside +// this band turn the workspace into either confetti or a microfiche. +const ZOOM_STEP = 0.5; +const ZOOM_MIN = -3; +const ZOOM_MAX = 4.5; + +/** + * Inspect a `before-input-event` key and apply (or block) the matching + * window-level shortcut. Returns `true` when the caller should call + * `event.preventDefault()` — that both swallows the renderer keydown and + * prevents the application menu accelerator from firing, so we don't + * double-trigger zoom on macOS where the default menu also binds these + * keys. + * + * Why we don't rely on the menu's `zoomIn` / `zoomOut` roles: on macOS the + * default `Cmd+-` accelerator does not fire reliably across keyboard + * layouts (issue MUL-2354 — Cmd+= zooms in but Cmd+- doesn't undo it). + * Handling the shortcuts here gives identical behavior on every platform + * and every layout. + */ +export function handleAppShortcut( + input: ShortcutInput, + webContents: ZoomTarget, + platform: NodeJS.Platform = process.platform, +): boolean { + if (input.type !== "keyDown") return false; + const cmdOrCtrl = platform === "darwin" ? input.meta : input.control; + + // Block reload — accidental Cmd+R / Ctrl+R / F5 destroys in-memory state + // (tabs, drafts, WS connections) with no URL bar to recover from. + if ((cmdOrCtrl && input.key.toLowerCase() === "r") || input.key === "F5") { + return true; + } + + if (!cmdOrCtrl) return false; + + // Cmd/Ctrl + "=" (unshifted) or "+" (Shift+=) → zoom in. + if (input.key === "=" || input.key === "+") { + const next = Math.min(webContents.getZoomLevel() + ZOOM_STEP, ZOOM_MAX); + webContents.setZoomLevel(next); + return true; + } + + // Cmd/Ctrl + "-" (unshifted) or "_" (Shift+-) → zoom out. + if (input.key === "-" || input.key === "_") { + const next = Math.max(webContents.getZoomLevel() - ZOOM_STEP, ZOOM_MIN); + webContents.setZoomLevel(next); + return true; + } + + // Cmd/Ctrl + 0 → reset zoom to 100%. + if (input.key === "0") { + webContents.setZoomLevel(0); + return true; + } + + return false; +}