Skip to content

fix(desktop): persist zoom level across routes - #38908

Closed
katanumahotori wants to merge 1 commit into
NousResearch:mainfrom
katanumahotori:codex/persist-desktop-zoom
Closed

katanumahotori wants to merge 1 commit into
NousResearch:mainfrom
katanumahotori:codex/persist-desktop-zoom

Conversation

@katanumahotori

Copy link
Copy Markdown

Summary

Fixes Desktop zoom persistence so the selected zoom level behaves as an app-wide preference instead of a per-route Chromium value.

Chromium stores zoom for file://...index.html#/route entries separately, so Hermes Desktop could reset or vary zoom when switching sessions, opening settings, or landing on a different hash route. This PR stores the Desktop zoom level under Electron userData and reapplies it on window load/navigation events.

Changes

  • Add zoom.json under Electron userData with { zoomLevel }.
  • Clamp zoom level to Electron's supported [-9, 9] range.
  • Route all menu and keyboard zoom changes through one persisted helper.
  • Reapply the saved zoom level on dom-ready, did-finish-load, did-navigate, and did-navigate-in-page.
  • Reapply on the next tick as well so Chromium's per-URL zoom restore cannot win a timing race.

Related

Fixes #38854.

Validation

  • node --check apps/desktop/electron/main.cjs

I did not run the packaged Windows desktop build for this PR because Hermes Desktop is currently running locally, and the Windows package build can fail when release/win-unpacked DLLs are locked by the running app.

Co-Authored-By: OpenAI <support@openai.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the route-level zoom problem. The underlying gap remains relevant, but this draft needs a current-architecture salvage rather than a direct application.

Problems

  • The only changed file, apps/desktop/electron/main.cjs, was removed by the TypeScript migration (39d09453f95e8aefc0c97e5d9b30ff341cae9ed8). Current zoom ownership is in apps/desktop/electron/main.ts:4754-4825.
  • Current main deliberately scopes global UI zoom away from the pet overlay (apps/desktop/electron/main.ts:6968-6981). The draft's all-BrowserWindow application path would need to preserve that exclusion.
  • The draft adds navigation-time behavior without a regression test. Current main still restores only on first load plus show/restore (apps/desktop/electron/main.ts:6975-6980) while the renderer uses HashRouter (apps/desktop/src/main.tsx:42).

Suggested changes

  • Rework this through wireCommonWindowHandlers() and the existing zoom helpers, covering chat/session windows without enabling zoom for the pet overlay.
  • Add a focused in-page navigation regression test.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Closing as superseded — both halves of this design have since landed on main via other PRs:

You had the right architecture — main-process file as the source of truth — before either of those landed; it just arrived via parallel salvages. Thanks for the work, and sorry it sat in draft long enough to be overtaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop zoom level is persisted per hash route instead of app-wide

3 participants