Skip to content

fix(desktop): persist zoom level across conversations - #46429

Closed
SongotenU wants to merge 1 commit into
NousResearch:mainfrom
SongotenU:fix/desktop-zoom-persist-across-conversations
Closed

SongotenU wants to merge 1 commit into
NousResearch:mainfrom
SongotenU:fix/desktop-zoom-persist-across-conversations

Conversation

@SongotenU

Copy link
Copy Markdown

Problem

When a user zooms in (Cmd+=) in one conversation and switches to another via the sidebar, the zoom level resets to default. This happens because Chromium stores per-URL zoom levels in per_host_zoom_levels, and each conversation has a unique hash route (...#/session_a vs ...#/session_b). The unseen URL gets a fresh 1.0 zoom.

Root Cause

The app already has zoom persistence infrastructure:

  • installZoomShortcuts() — intercepts Cmd+=/Cmd+-/Cmd+0 and writes the zoom level into both webContents.setZoomLevel() and localStorage
  • setAndPersistZoomLevel() / restorePersistedZoomLevel() — round-trip through localStorage so zoom survives restarts

But restorePersistedZoomLevel was called only from a once(did-finish-load) on mainWindow. It was not wired into:

  • Session windows (detached chats)
  • Hash-route navigations inside the SPA (same window, new URL = new per-host entry)

Fix

Move restorePersistedZoomLevel from the one-shot mainWindow callback into wireCommonWindowHandlers, which is already shared by every window (main + session). Now every did-finish-load — cold start, session window open, reload, and hash-route transitions — re-applies the user persisted zoom level.

Changes

  • apps/desktop/electron/main.cjs: +8 −1
  • Move restorePersistedZoomLevel(win) into wireCommonWindowHandlers as a did-finish-load listener
  • Remove the now-redundant call from mainWindow one-shot callback

Testing

  • Zoom in → switch conversation → zoom persists
  • Zoom in → close & reopen window → zoom persists
  • Zoom in → open session window → zoom is restored
  • Cmd+0 resets zoom → persists across navigation

When a user zooms in (Cmd+=) in one conversation and switches to
another via the sidebar, Chromium's per-URL per_host_zoom_levels
resets the zoom to 1.0 for the unseen URL.  The app already persists
the zoom level in localStorage and restores it on mainWindow's
did-finish-load, but session windows and hash-route navigations
within the same window did not re-apply the persisted zoom.

Move restorePersistedZoomLevel from the one-shot mainWindow callback
into wireCommonWindowHandlers so it runs on every did-finish-load for
every window — cold start, session window open, reload, and
hash-route transitions inside the SPA all re-apply the user's chosen
zoom level.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jun 15, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Simple, focused fix: persists zoom level across desktop conversations. Single file, minimal change.

Looks Good

  • Clean, well-scoped
  • No security concerns

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused zoom-persistence investigation. The issue remains relevant, but this patch needs a small rework before it can cover its stated route-switch path.

Problems

  • Desktop uses HashRouter at apps/desktop/src/main.tsx:42. Electron documents that an in-page navigation does not reload and emits did-navigate-in-page; therefore the PR's did-finish-load listener does not run for the hash-route conversation switch it describes.
  • The patch targets apps/desktop/electron/main.cjs, but current main moved this wiring to apps/desktop/electron/main.ts:6972-6981; it will not apply cleanly.
  • There is no route-navigation regression test. Current zoom tests cover show/restore only (apps/desktop/electron/zoom.test.ts:67-102).

Suggested changes

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 pushed a commit that referenced this pull request Jul 18, 2026
…e first (#46429)

Surgical reapply of PR #46429 by @SongotenU (the branch predates the
ts-ify migration; main.cjs no longer exists so a direct cherry-pick
cannot apply).

Main already adopted half of the PR's fix when zoom restore moved into
wireCommonWindowHandlers (57dfebe) — session windows are covered. The
surviving delta is the listener lifetime: 'once' spends the listener on
the first did-finish-load, so any later full load in the same
webContents — the crash-recovery webContents.reload(), a manual reload,
or an in-place navigation that lands on a fresh per-host zoom entry —
came up at default zoom while the settings Scale control still showed
the persisted percentage. 'on' re-applies the persisted level after
every completed load; restorePersistedZoomLevel routes through the
applyZoomLevel funnel so the renderer stays in sync.
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #66989 (#66989) — your fix is on main with your authorship preserved. Thanks for the clean root-cause analysis on the per-host zoom levels and the once-spent listener!

Your branch targeted the pre-TypeScript main.cjs (since migrated), so the change was surgically reapplied onto current main under your authorship rather than cherry-picked. Half of your original fix — moving zoom restore into wireCommonWindowHandlers so session windows are covered — had already been independently adopted on main; the merged PR ships the remaining half (once('did-finish-load') → on(...)) so zoom now survives crash-recovery reloads and in-place navigations too.

Closing this PR in favor of the merged salvage. Thanks again!

@teknium1 teknium1 closed this Jul 18, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…e first (NousResearch#46429)

Surgical reapply of PR NousResearch#46429 by @SongotenU (the branch predates the
ts-ify migration; main.cjs no longer exists so a direct cherry-pick
cannot apply).

Main already adopted half of the PR's fix when zoom restore moved into
wireCommonWindowHandlers (1d97112) — session windows are covered. The
surviving delta is the listener lifetime: 'once' spends the listener on
the first did-finish-load, so any later full load in the same
webContents — the crash-recovery webContents.reload(), a manual reload,
or an in-place navigation that lands on a fresh per-host zoom entry —
came up at default zoom while the settings Scale control still showed
the persisted percentage. 'on' re-applies the persisted level after
every completed load; restorePersistedZoomLevel routes through the
applyZoomLevel funnel so the renderer stays in sync.
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
…e first (NousResearch#46429)

Surgical reapply of PR NousResearch#46429 by @SongotenU (the branch predates the
ts-ify migration; main.cjs no longer exists so a direct cherry-pick
cannot apply).

Main already adopted half of the PR's fix when zoom restore moved into
wireCommonWindowHandlers (385e920) — session windows are covered. The
surviving delta is the listener lifetime: 'once' spends the listener on
the first did-finish-load, so any later full load in the same
webContents — the crash-recovery webContents.reload(), a manual reload,
or an in-place navigation that lands on a fresh per-host zoom entry —
came up at default zoom while the settings Scale control still showed
the persisted percentage. 'on' re-applies the persisted level after
every completed load; restorePersistedZoomLevel routes through the
applyZoomLevel funnel so the renderer stays in sync.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…e first (NousResearch#46429)

Surgical reapply of PR NousResearch#46429 by @SongotenU (the branch predates the
ts-ify migration; main.cjs no longer exists so a direct cherry-pick
cannot apply).

Main already adopted half of the PR's fix when zoom restore moved into
wireCommonWindowHandlers (57dfebe) — session windows are covered. The
surviving delta is the listener lifetime: 'once' spends the listener on
the first did-finish-load, so any later full load in the same
webContents — the crash-recovery webContents.reload(), a manual reload,
or an in-place navigation that lands on a fresh per-host zoom entry —
came up at default zoom while the settings Scale control still showed
the persisted percentage. 'on' re-applies the persisted level after
every completed load; restorePersistedZoomLevel routes through the
applyZoomLevel funnel so the renderer stays in sync.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) 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.

4 participants