From 06404107261cae24394d6abe39984fb445c5aff5 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 8 Aug 2026 05:24:38 -0400 Subject: [PATCH] fix(desktop): zoom shortcuts no longer die when the preview browser has focus (#5691) Co-authored-by: Claude Fable 5 --- apps/desktop/src/app/DesktopLifecycle.test.ts | 1 + .../src/backend/DesktopBackendPool.test.ts | 1 + .../src/window/DesktopApplicationMenu.test.ts | 90 ++++++++++++++----- .../src/window/DesktopApplicationMenu.ts | 29 +++++- apps/desktop/src/window/DesktopWindow.ts | 20 +++++ 5 files changed, 113 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/app/DesktopLifecycle.test.ts b/apps/desktop/src/app/DesktopLifecycle.test.ts index be9d7f3451f1263118a5be53654fdf42d58a7917..45e1c82460c8c130f6a6da50e0fd7f3f44bcdd65 100644 --- a/apps/desktop/src/app/DesktopLifecycle.test.ts +++ b/apps/desktop/src/app/DesktopLifecycle.test.ts @@ -78,6 +78,7 @@ describe("DesktopLifecycle", () => { handleBackendNotReady: Effect.void, flushMainWindowBounds: Effect.void, dispatchMenuAction: () => Effect.void, + zoomMain: () => Effect.void, syncAppearance: Effect.void, }); diff --git a/apps/desktop/src/backend/DesktopBackendPool.test.ts b/apps/desktop/src/backend/DesktopBackendPool.test.ts index 523e8764697bdf3311a20dc572e61d67dad32c1e..98bd4065fbeed1b42eb99b80a05596fd24eb04f2 100644 --- a/apps/desktop/src/backend/DesktopBackendPool.test.ts +++ b/apps/desktop/src/backend/DesktopBackendPool.test.ts @@ -91,6 +91,7 @@ function makePoolLayer( handleBackendNotReady: Effect.void, flushMainWindowBounds: Effect.void, dispatchMenuAction: () => Effect.die("unexpected menu action"), + zoomMain: () => Effect.die("unexpected zoom"), syncAppearance: Effect.void, } satisfies DesktopWindow.DesktopWindow["Service"]), ), diff --git a/apps/desktop/src/window/DesktopApplicationMenu.test.ts b/apps/desktop/src/window/DesktopApplicationMenu.test.ts index 22a24b908b62d085ba797495bbf5580d5af43fd6..0c826e36dd96b6ee5d0f6af4b3ad875c690c8e70 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.test.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.test.ts @@ -81,6 +81,8 @@ const makeDesktopWindowLayer = (selectedAction: Deferred.Deferred) => handleBackendNotReady: Effect.void, flushMainWindowBounds: Effect.void, dispatchMenuAction: (action) => Deferred.succeed(selectedAction, action).pipe(Effect.asVoid), + zoomMain: (direction) => + Deferred.succeed(selectedAction, `zoom-${direction}`).pipe(Effect.asVoid), syncAppearance: Effect.void, } satisfies DesktopWindow.DesktopWindow["Service"]); @@ -94,32 +96,38 @@ const makeElectronMenuLayer = ( showContextMenu: () => Effect.succeed(Option.none()), } satisfies ElectronMenu.ElectronMenu["Service"]); -describe("DesktopApplicationMenu", () => { - it.effect("installs the native menu and routes Settings through DesktopWindow", () => - Effect.gen(function* () { - const selectedAction = yield* Deferred.make(); - const applicationMenuTemplate = - yield* Deferred.make(); - - yield* Effect.gen(function* () { - const menu = yield* DesktopApplicationMenu.DesktopApplicationMenu; - yield* menu.configure; - }).pipe( - Effect.provide( - DesktopApplicationMenu.layer.pipe( - Layer.provideMerge(makeElectronMenuLayer(applicationMenuTemplate)), - Layer.provideMerge(makeDesktopWindowLayer(selectedAction)), - Layer.provideMerge(desktopUpdatesLayer), - Layer.provideMerge(electronDialogLayer), - Layer.provideMerge(electronAppLayer), - Layer.provideMerge( - DesktopEnvironment.layer(environmentInput).pipe( - Layer.provide(Layer.mergeAll(NodeServices.layer, DesktopConfig.layerTest({}))), - ), - ), +const configureMenu = ( + selectedAction: Deferred.Deferred, + applicationMenuTemplate: Deferred.Deferred, +) => + Effect.gen(function* () { + const menu = yield* DesktopApplicationMenu.DesktopApplicationMenu; + yield* menu.configure; + }).pipe( + Effect.provide( + DesktopApplicationMenu.layer.pipe( + Layer.provideMerge(makeElectronMenuLayer(applicationMenuTemplate)), + Layer.provideMerge(makeDesktopWindowLayer(selectedAction)), + Layer.provideMerge(desktopUpdatesLayer), + Layer.provideMerge(electronDialogLayer), + Layer.provideMerge(electronAppLayer), + Layer.provideMerge( + DesktopEnvironment.layer(environmentInput).pipe( + Layer.provide(Layer.mergeAll(NodeServices.layer, DesktopConfig.layerTest({}))), ), ), - ); + ), + ), + ); + +describe("DesktopApplicationMenu", () => { + it.effect("installs the native menu and routes Settings through DesktopWindow", () => + Effect.gen(function* () { + const selectedAction = yield* Deferred.make(); + const applicationMenuTemplate = + yield* Deferred.make(); + + yield* configureMenu(selectedAction, applicationMenuTemplate); const template = yield* Deferred.await(applicationMenuTemplate); const fileMenu = template.find((item) => item.label === "File"); @@ -138,4 +146,38 @@ describe("DesktopApplicationMenu", () => { assert.equal(yield* Deferred.await(selectedAction), "open-settings"); }), ); + + // Zoom must route through DesktopWindow.zoomMain instead of the Electron + // zoom roles: the roles zoom whichever webContents has focus, which breaks + // app zoom while an embedded preview WebContentsView holds focus. + it.effect("routes View menu zoom to the main window instead of zoom roles", () => + Effect.gen(function* () { + const selectedAction = yield* Deferred.make(); + const applicationMenuTemplate = + yield* Deferred.make(); + + yield* configureMenu(selectedAction, applicationMenuTemplate); + + const template = yield* Deferred.await(applicationMenuTemplate); + const viewMenu = template.find((item) => item.label === "View"); + assert.isDefined(viewMenu); + if (!Array.isArray(viewMenu.submenu)) { + throw new Error("Expected View menu submenu to be an array."); + } + + assert.isUndefined( + viewMenu.submenu.find((item) => item.role?.toLowerCase().includes("zoom")), + ); + + const zoomIn = viewMenu.submenu.find((item) => item.label === "Zoom In"); + assert.isDefined(zoomIn); + assert.equal(zoomIn.accelerator, "CmdOrCtrl+="); + if (typeof zoomIn.click !== "function") { + throw new Error("Expected Zoom In menu item to have a click handler."); + } + + zoomIn.click({} as Electron.MenuItem, {} as Electron.BrowserWindow, {} as KeyboardEvent); + assert.equal(yield* Deferred.await(selectedAction), "zoom-in"); + }), + ); }); diff --git a/apps/desktop/src/window/DesktopApplicationMenu.ts b/apps/desktop/src/window/DesktopApplicationMenu.ts index a52707627b0a459bf4a79e6676a11c6e0bc648e3..66244534debf42812d757bdcc46fcb4844fdc6b0 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.ts @@ -49,6 +49,13 @@ const dispatchMenuAction = Effect.fn("desktop.menu.dispatchMenuAction")(function yield* desktopWindow.dispatchMenuAction(action); }); +const zoomMainWindow = Effect.fn("desktop.menu.zoomMainWindow")(function* ( + direction: DesktopWindow.MainWindowZoomDirection, +): Effect.fn.Return { + const desktopWindow = yield* DesktopWindow.DesktopWindow; + yield* desktopWindow.zoomMain(direction); +}); + const checkForUpdatesFromMenu = Effect.gen(function* () { const updates = yield* DesktopUpdates.DesktopUpdates; const electronDialog = yield* ElectronDialog.ElectronDialog; @@ -127,6 +134,9 @@ export const make = Effect.gen(function* () { const settingsClick = () => { runMenuEffect("open-settings", dispatchMenuAction("open-settings")); }; + const zoomClick = (direction: DesktopWindow.MainWindowZoomDirection) => () => { + runMenuEffect(`zoom-${direction}`, zoomMainWindow(direction)); + }; const template: Electron.MenuItemConstructorOptions[] = []; if (environment.platform === "darwin") { @@ -181,10 +191,21 @@ export const make = Effect.gen(function* () { { role: "forceReload" }, { role: "toggleDevTools" }, { type: "separator" }, - { role: "resetZoom" }, - { role: "zoomIn", accelerator: "CmdOrCtrl+=" }, - { role: "zoomIn", accelerator: "CmdOrCtrl+Plus", visible: false }, - { role: "zoomOut" }, + /* + Not the zoom roles: those act on the focused webContents, so with + an embedded preview WebContentsView focused they zoom the guest + page and the app UI appears stuck. These always zoom the main + window (see DesktopWindow.zoomMain). + */ + { label: "Actual Size", accelerator: "CmdOrCtrl+0", click: zoomClick("reset") }, + { label: "Zoom In", accelerator: "CmdOrCtrl+=", click: zoomClick("in") }, + { + label: "Zoom In", + accelerator: "CmdOrCtrl+Plus", + visible: false, + click: zoomClick("in"), + }, + { label: "Zoom Out", accelerator: "CmdOrCtrl+-", click: zoomClick("out") }, { type: "separator" }, { role: "togglefullscreen" }, ], diff --git a/apps/desktop/src/window/DesktopWindow.ts b/apps/desktop/src/window/DesktopWindow.ts index 3bf746a8e9b639c4295dc630f965da8063bf45b5..bf8c681448fe7a6c95ae433a8cf5a1b614e92891 100644 --- a/apps/desktop/src/window/DesktopWindow.ts +++ b/apps/desktop/src/window/DesktopWindow.ts @@ -61,6 +61,8 @@ export type DesktopWindowError = | ElectronWindow.ElectronWindowCreateError | PreviewManager.PreviewManagerError; +export type MainWindowZoomDirection = "in" | "out" | "reset"; + export class DesktopWindow extends Context.Service< DesktopWindow, { @@ -87,6 +89,12 @@ export class DesktopWindow extends Context.Service< readonly handleBackendNotReady: Effect.Effect; readonly flushMainWindowBounds: Effect.Effect; readonly dispatchMenuAction: (action: string) => Effect.Effect; + // Zooms the main window's own webContents. The Electron `zoomIn`/`zoomOut` + // menu roles act on whichever webContents has keyboard focus, so with an + // embedded preview WebContentsView (or DevTools) focused they zoom the + // guest page instead of the app UI. The menu routes here to always target + // the main window. + readonly zoomMain: (direction: MainWindowZoomDirection) => Effect.Effect; readonly syncAppearance: Effect.Effect; } >()("@t3tools/desktop/window/DesktopWindow") {} @@ -836,6 +844,18 @@ export const make = Effect.gen(function* () { send(); }), + zoomMain: Effect.fn("desktop.window.zoomMain")(function* (direction) { + yield* Effect.annotateCurrentSpan({ direction }); + const window = yield* focusedMainWindow; + if (Option.isNone(window) || window.value.isDestroyed()) { + return; + } + const webContents = window.value.webContents; + // Same step size as the Electron zoomIn/zoomOut menu roles. + webContents.setZoomLevel( + direction === "reset" ? 0 : webContents.getZoomLevel() + (direction === "in" ? 0.5 : -0.5), + ); + }), syncAppearance: Effect.gen(function* () { const shouldUseDarkColors = yield* electronTheme.shouldUseDarkColors; yield* electronWindow.syncAllAppearance((window) => -- 2.54.0