fix(desktop): publish native menu zoom changes - #46773
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness/robustness nits in the changed code paths (notably an unhandled rejection risk on renderer startup) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes desktop zoom state desynchronization by routing native menu zoom actions through a single main-process zoom setter that emits WindowZoomChanged, and by seeding the renderer’s webviewZoom signal from the main process on reload so layout-dependent signals reflect the actual persisted zoom.
Changes:
- Add renderer startup seeding of
webviewZoomusingapi.getZoomFactor()to preserve zoom state across renderer reloads. - Introduce
setZoomFactor(win, factor)in the main window appearance module to clamp, apply, and publish zoom changes. - Update native menu zoom handlers to call
setZoomFactor(instead of directly mutatingwebContentsand only updating the titlebar).
File summaries
| File | Description |
|---|---|
| packages/desktop/src/renderer/window/zoom.ts | Seed webviewZoom from the main-process zoom factor on renderer start to avoid stale zoom-derived layout after reload. |
| packages/desktop/src/main/windows/index.ts | Re-export setZoomFactor from the windows module for use by other main-process code (menu actions). |
| packages/desktop/src/main/windows/appearance.ts | Add setZoomFactor helper that clamps + applies zoom and publishes WindowZoomChanged via updateZoom. |
| packages/desktop/src/main/native/menu-actions.ts | Route View → Zoom menu actions through setZoomFactor so zoom changes emit the IPC event and keep renderer state in sync. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+49
to
+54
| // A renderer reload keeps the window's zoom, so seed the signal from main. | ||
| void api.getZoomFactor().then((factor) => { | ||
| if (requestedZoom !== 1) return | ||
| requestedZoom = clamp(factor) | ||
| setWebviewZoom(requestedZoom) | ||
| }) |
Comment on lines
1
to
+3
| import { BrowserWindow } from "electron" | ||
| import type { DesktopMenuAction } from "@opencode-ai/app/desktop-menu" | ||
| import { updateTitlebar } from "../windows" | ||
| import { setZoomFactor } from "../windows" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WindowZoomChanged, so the renderer'swebviewZoomsignal stayed stale. Anything laid out from that signal (titlebar insets, native view bounds) drifted from the real zoom until the next keyboard or pinch zoom.setZoomFactor(win, factor)inwindows/appearance.tsthat clamps, applies, updates the titlebar overlay, and publishes the event.webviewZoomfromgetZoomFactor()on renderer start so a reload while zoomed does not reset the signal to1.flowchart LR Menu[Native View menu] --> Set[setZoomFactor] Keys[Ctrl +/- keys] --> Set Pinch[zoom-changed] --> Set Set --> Titlebar[updateTitlebar] Set --> Event[WindowZoomChanged] Event --> Signal[renderer webviewZoom]