Skip to content

feat(window): persist UI zoom level across restarts#3017

Merged
pauldambra merged 3 commits into
mainfrom
posthog-code/persist-zoom-level
Jun 30, 2026
Merged

feat(window): persist UI zoom level across restarts#3017
pauldambra merged 3 commits into
mainfrom
posthog-code/persist-zoom-level

Conversation

@pauldambra

Copy link
Copy Markdown
Member

Problem

Zooming the UI with CMD++ / CMD+- / CMD+0 worked during a session, but the zoom reset to 100% on every relaunch.

The View-menu zoom items used Electron's built-in zoomIn/zoomOut/resetZoom roles, which only adjust Chromium's in-memory per-webContents zoom and expose no click hook to persist it. Nothing saved the level.

Change

Give zoom the same treatment as the existing window-state persistence (bounds/maximized), all in apps/code/src/main/:

  • utils/store.ts — add zoomLevel to WindowStateSchema and the windowStateStore defaults (0 = 100%).
  • window.ts — read the saved level in getSavedWindowState(); add saveZoomLevel(); re-apply the level on did-finish-load (covers fresh launches and in-app reloads, which reset Chromium's zoom); persist wheel/pinch zoom via zoom-changed.
  • menu.ts — replace the built-in zoom roles with custom items backed by a shared applyZoom() helper that sets and persists the level. Keeps CMD++, CMD+= (hidden duplicate, matching the built-in role's dual accelerator), CMD+-, and CMD+0.

Verification

  • pnpm --filter code typecheck passes.
  • Biome lint clean on the three files.
  • Manual: zoom in, quit fully, relaunch → level restored; zoomLevel is written to <userData>/window-state.json; survives CMD+Shift+R reload.

Created with PostHog Code

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit f94aa6d.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f473a6bf8f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/code/src/main/window.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "feat(window): persist UI zoom level acro..." | Re-trigger Greptile

Comment thread apps/code/src/main/window.ts
Comment thread apps/code/src/main/menu.ts Outdated

@pauldambra pauldambra left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: paul-reviewer, xp-reviewer, security-audit (qa-team skipped — not installed in this repo). See inline comments.

Comment thread apps/code/src/main/window.ts Outdated
Comment thread apps/code/src/main/menu.ts Outdated
Comment thread apps/code/src/main/menu.ts Outdated
Comment thread apps/code/src/main/menu.ts
@pauldambra

Copy link
Copy Markdown
Member Author

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: paul-reviewer, xp-reviewer, security-audit (qa-team skipped — not installed in this repo)

Verdict: 💬 APPROVE WITH NITS

Clean, focused change. No security or blocking correctness issues. One genuine bug worth fixing: the in-app-reload restore re-applies the zoom level captured at window-create time, so it doesn't actually preserve session zooming across a reload.

Key findings

  • 🟡 MEDIUMwindow.ts did-finish-load re-applies savedState.zoomLevel (create-time snapshot), not the latest persisted value → a reload discards in-session zooming. Read from the store in the handler.
  • 🟢 LOW — no clamp on the zoom level; a runaway Cmd+Plus persists an extreme level (the built-in role capped this).
  • 🟢 LOW0.5 / -0.5 / 0 magic numbers want a named ZOOM_STEP.
  • 🟢 LOW — zoom state-mutation split across menu.ts (applyZoom) and window.ts (saveZoomLevel); consider consolidating.

Convergence

  • did-finish-load handler flagged independently by paul (stale create-time snapshot) and xp (restore→re-save coupling) — highest confidence.
  • Zoom-logic-split flagged by both paul and xp.

Reviewer summaries

Reviewer Assessment
👤 paul approve-with-caveats; wants a clamp and to double-check the did-finish-load intent
📐 xp solid, does one thing well; consolidate zoom logic + name the step constant
🛡 security-audit 0 findings — local-only main-process path, no untrusted input reaches setZoomLevel

Automated by QA Swarm — not a human review

pauldambra added a commit that referenced this pull request Jun 30, 2026
Address qa-swarm review on #3017:
- did-finish-load now reads the latest persisted zoomLevel from the store
  instead of the create-time snapshot, so zooming done during a session
  survives in-app reloads.
- Clamp the zoom level (ZOOM_MIN/ZOOM_MAX) so a runaway accelerator can't
  persist an unusable zoom across restarts.
- Extract the ZOOM_STEP constant in place of the repeated 0.5 literals.

Generated-By: PostHog Code
Task-Id: 20eca4a9-7e69-4258-b681-5b60caae5a47
The View-menu zoom items used Electron's built-in zoomIn/zoomOut/resetZoom
roles, which only adjust Chromium's in-memory per-webContents zoom and expose
no click hook to persist it. Zoom therefore reset to 100% on every relaunch.

Store the zoom level alongside the existing window state, re-apply it on
did-finish-load (covering fresh launches and in-app reloads), and persist
both menu-driven and wheel/pinch zoom changes.

Generated-By: PostHog Code
Task-Id: 20eca4a9-7e69-4258-b681-5b60caae5a47
Address qa-swarm review on #3017:
- did-finish-load now reads the latest persisted zoomLevel from the store
  instead of the create-time snapshot, so zooming done during a session
  survives in-app reloads.
- Clamp the zoom level (ZOOM_MIN/ZOOM_MAX) so a runaway accelerator can't
  persist an unusable zoom across restarts.
- Extract the ZOOM_STEP constant in place of the repeated 0.5 literals.

Generated-By: PostHog Code
Task-Id: 20eca4a9-7e69-4258-b681-5b60caae5a47
@pauldambra pauldambra force-pushed the posthog-code/persist-zoom-level branch from 0f59fb8 to 0ba211e Compare June 30, 2026 09:53
@pauldambra pauldambra added the Stamphog This will request an autostamp by stamphog on small changes label Jun 30, 2026
github-actions[bot]
github-actions Bot previously approved these changes Jun 30, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No showstoppers: all substantive reviewer concerns (reload-reset bug, missing zoom clamp, unnamed step constant) are addressed in the current diff. Remaining comments are low-severity style suggestions. The circular dep between menu.ts and window.ts is real but runtime-safe for function-only imports in a bundled Electron main process.

menu.ts importing saveZoomLevel from window.ts created a circular
dependency (window.ts already imports buildApplicationMenu from menu.ts).
saveZoomLevel only wraps windowStateStore.set, so it belongs beside the
store; both menu.ts and window.ts now import it from utils/store. Resolves
greptile's circular-dependency note on #3017.

Generated-By: PostHog Code
Task-Id: 20eca4a9-7e69-4258-b681-5b60caae5a47
@pauldambra pauldambra removed the Stamphog This will request an autostamp by stamphog on small changes label Jun 30, 2026
@github-actions github-actions Bot dismissed their stale review June 30, 2026 09:57

New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.

@pauldambra pauldambra added the Stamphog This will request an autostamp by stamphog on small changes label Jun 30, 2026
@pauldambra

Copy link
Copy Markdown
Member Author

Note

🤖 Automated comment by PR Shepherd — not written by a human

Triaged the automated review threads against the latest commits:

  • Codex P2 / greptile P1 — "use current persisted zoom on reload" (window.ts did-finish-load): already fixed in 0ba211e8 — the handler now reads windowStateStore.get("zoomLevel", 0) instead of the create-time snapshot, so in-session zooming survives reloads. (Both bots reviewed a pre-fix snapshot.)
  • greptile P2 — circular import menu.ts ↔ window.ts: resolved in f94aa6df by moving saveZoomLevel into utils/store.ts (next to the store it wraps); menu.ts no longer imports from window.ts.
  • Also from the qa-swarm pass: added a zoom clamp (ZOOM_MIN/ZOOM_MAX) and a ZOOM_STEP constant.

Deferred (your call): the broader "consolidate all zoom logic into a dedicated module" suggestion — the current split is small and clear, so left as-is.

pnpm --filter code typecheck and Biome lint are clean on all three files.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All substantive reviewer concerns (reload-reset bug, missing zoom clamp, unnamed step constant) are addressed in the current diff. The unresolved greptile comment describes an earlier version; the actual code reads zoom from the live store on reload, not a stale snapshot. Changes are correctly scoped to the Electron host layer.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No showstoppers. The substantive reviewer concerns (reload-reset bug, missing clamp, unnamed constant) are all addressed in the current diff. The circular-dependency risk is gone because both files now import through store.ts. The change is correctly confined to the Electron host layer.

@pauldambra pauldambra merged commit 53b85fe into main Jun 30, 2026
26 checks passed
@pauldambra pauldambra deleted the posthog-code/persist-zoom-level branch June 30, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants