Skip to content

fix(web): preview viewports keep their size when the main window is zoomed - #14039

Open
Ta-noshii wants to merge 5 commits into
pingdotgg:mainfrom
Ta-noshii:fix/preview-viewport-main-zoom
Open

Ta-noshii wants to merge 5 commits into
pingdotgg:mainfrom
Ta-noshii:fix/preview-viewport-main-zoom

Conversation

@Ta-noshii

@Ta-noshii Ta-noshii commented Sep 28, 2026 •

Copy link
Copy Markdown

Fixed preview viewports grow with the app window's zoom even though the guest retains its own tab zoom. This sizes local Electron webviews with the ratio of guest zoom to host zoom, and uses the same units for responsive-toolbar sizing, agent cursors, fitted previews, and the mini-player. Primary-environment server tabs rendered in a native webview follow that path too; streamed tabs keep server-space sizing.

The optional preload getter reads the current window zoom. A resize subscription updates mounted previews when host zoom changes; older shells and ordinary web clients fall back to host factor 1. Guest zoom behavior is unchanged. Main was merged and the original author's history retained.

The original 15-second automation-timeout claim no longer describes current main: #15328 moved viewport operations to the server. This PR now addresses renderer geometry. It overlaps #10536; this approach keeps guest page zoom separate from host layout, avoiding the fill-mode page-scale regression identified in that PR's takeover review. No changes were made to that PR.

Verification at 9a27f7cff2:

  • Original six added cases in two files were replaced with three observable behaviors, executed as four cases in two existing browser test files. The one-off hook suite and helper-only tests were removed. Reverting compensation fails two cases; removing the resize subscription fails one. The additional native-server responsive variant failed before its correction.
  • 76 focused web cases in seven suites pass; three existing mocked desktop zoom cases pass. Web, desktop, and contracts typechecks, targeted lint/format, and whitespace checks pass.
  • The additional DesktopWindow suite could not collect because Electron's binary download failed. It is not claimed as passing.
  • One independent full-diff review round by Claude Opus 5.5 and GPT-6-Sol found no code blockers. Claude independently reran 64 relevant cases successfully.

Limited live proof uses the actual base component at 0c609ab87d and current head in the built-in headless Chromium Browser panel, with matching props, a simulated preload bridge and CSS host zoom. The baseline copy only resolves imports to their original source paths; component logic is unchanged. At simulated 125% host zoom, an 800×400 request renders as a 1000×500 box before and 800×400 after. The recording changes host zoom between 100% and 125% without remounting the page. Preset fitting and fill transitions were also exercised. A locally loaded fallback font makes text visible in this headless environment.

Before — actual base component, simulated host zoom

Before: 800×400 request becomes a 1000×500 renderer box at simulated 125% host zoom

After — current component, same fixture

After: 800×400 renderer box at simulated 125% host zoom

Renderer zoom interaction recording

Native proof remains required before merge. This is DOM renderer evidence with an empty ordinary webview element, not an Electron guest or a full desktop end-to-end pass. Actual guest innerWidth/innerHeight, page scale/devicePixelRatio, attach/reload, host zoom at fractional display scales, fill/freeform/preset, responsive toolbar, cursor, and mini-player transitions still need a native desktop check, including primary-environment server tabs. macOS, Windows, iOS, Android, and native Electron desktop were not verified here. Mobile has no Electron layout path in this change. A known remaining limitation is that changing host zoom while a fitted fill-mode mini-player is already open can reflow its captured source; its stored host-pixel snapshot is outside this viewport correction.

Credit to @Ta-noshii for the original implementation and diagnosis in #14039.

Maintainer takeover through T3 Code's Codex harness: tests by GPT-6-Astra (high), implementation by GPT-6.1-Sol (high), adversarial review by Claude Opus 5.5 and GPT-6-Sol (high); integration and evidence by GPT-6.1-Sol.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 28, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 28, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9a27f7c

Macroscope's review found this PR approvable — This is a bounded preview-layout bug fix that compensates for host-window zoom across webview sizing, cursor alignment, responsive sizing, and the mini-player while preserving existing defaults and fallbacks. The additive bridge API is backward-compatible, and focused browser tests cover the affected behavior.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a8a27fb2-3958-4f4e-a4fa-1f2450f4a28f

📥 Commits

Reviewing files that changed from the base of the PR and between a382405 and f533d74.

📒 Files selected for processing (4)
  • apps/desktop/src/preload.ts
  • apps/web/src/browser/HostedBrowserWebview.test.tsx
  • apps/web/src/browser/useBrowserLayoutZoomFactor.test.tsx
  • packages/contracts/src/ipc.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The desktop bridge now exposes the window zoom factor. Browser layout calculations combine that factor with preview zoom and use the result for webview sizing, responsive viewport selection, cursor scaling, and mini-player source sizing.

Changes: Browser Layout Zoom

Layer / File(s) Summary
Window zoom bridge contract
packages/contracts/src/ipc.ts, apps/desktop/src/preload.ts
The desktop bridge adds an optional getWindowZoomFactor method. The preload implementation returns the current webFrame zoom factor.
Layout zoom resolution
apps/web/src/browser/browserViewportLayout.ts, apps/web/src/browser/useBrowserLayoutZoomFactor.ts, apps/web/src/browser/browserViewportLayout.test.ts, apps/web/src/browser/useBrowserLayoutZoomFactor.test.tsx
The resolver normalizes preview and window zoom factors before dividing them. Hooks read the window zoom factor and subscribe to window resize events. Tests cover layout sizing, preview zoom adjustment, invalid window zoom fallbacks, resize updates, and a missing desktop bridge method.
Applying layout zoom
apps/web/src/browser/HostedBrowserWebview.tsx, apps/web/src/browser/HostedBrowserWebview.test.tsx, apps/web/src/components/preview/PreviewView.tsx, apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx
Hosted webview sizing, responsive viewport selection, cursor scaling, and mini-player source sizing use the resolved layout zoom factor.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant WindowResize
  participant useWindowZoomFactor
  participant DesktopBridge
  participant useBrowserLayoutZoomFactor
  participant PreviewComponents
  WindowResize->>useWindowZoomFactor: trigger window zoom factor read
  useWindowZoomFactor->>DesktopBridge: getWindowZoomFactor
  DesktopBridge-->>useWindowZoomFactor: return window zoom factor
  useWindowZoomFactor->>useBrowserLayoutZoomFactor: provide window zoom factor
  useBrowserLayoutZoomFactor->>PreviewComponents: provide resolved layout zoom factor
Loading

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to f533d

Preview sizing accounts for desktop window zoom, and changing app zoom refreshes the layout factor. No actionable merge-blocking behavior remains.

Security architecture risk: 🔵 Low · up to f533d

The new capability only reads the current window zoom, and preview pages remain separated from the app’s capabilities. No new security issue was identified in the reviewed paths, although broader security coverage remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added capability’s direct exposure is limited to reading the calling application frame’s zoom, with downstream effects on preview geometry. The inspected flow does not add authority over another tab, environment, service, or data store.

Trust Boundaries and Controls

  • observed — Hosted preview guests receive the separate preview-pick preload through their configuration, not the application preload exposing DesktopBridge. The new zoom getter is not passed into guest content; its value is consumed by host-side layout.

Resilience and Maintainability Implications

  • inferred — The new numeric input does not replace logical viewport ownership or introduce a new commit protocol. Existing per-runtime-tab serialization, identity checks, timeout recovery, and conditional rollback remain the controls against cross-tab writes and stale restoration after concurrent changes.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main fix: preview viewports retain their requested size when the main window zoom changes.
Description check Passed The description explains the problem, implementation, scope, related issues, verification results, limitations, and required native validation. It does not use the template headings exactly, but it pr…
Full details: Description check

Explanation

The description explains the problem, implementation, scope, related issues, verification results, limitations, and required native validation. It does not use the template headings exactly, but it provides the required information in substance.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@mmmattG

mmmattG commented Sep 29, 2026 •

Copy link
Copy Markdown

Note

GPT-6.1-Sol, on behalf of Matt (@mmmattG), via the Codex harness in T3 Code.

Matt independently opened #14281 for the same bug and found this earlier PR afterward. He has closed his PR as a duplicate. I compared the two and found two useful test follow-ups:

  1. Update the existing webview test setup. Applying this PR's current diff to the same baseline as fix(web): browser preview viewport no longer scales with window zoom #14281 (d2c9281b8112dc3b2991642c4bdb985e4b08b9bb) in an isolated copy made apps/web/src/browser/HostedBrowserWebview.test.tsx fail with TypeError: window.addEventListener is not a function. That test uses globalThis as its window. Adding these two lines after its window stub made the existing test pass:

    vi.stubGlobal("addEventListener", vi.fn());
    vi.stubGlobal("removeEventListener", vi.fn());

    These test-setup changes are already included in fix(web): browser preview viewport no longer scales with window zoom #14281.

  2. Cover an app-zoom change after mount. CodeRabbit pointed out on fix(web): browser preview viewport no longer scales with window zoom #14281 that the new tests exercise the scale calculation but don't verify the resize subscription updates the preview when app zoom changes. The tests here have the same gap. A regression test could mount a hook consumer or preview component, change the mocked getWindowZoomFactor() value, dispatch a resize event, and assert that the scale or rendered layout updates.

The description of #14281 also includes independent Linux desktop measurements at 100%, 120%, and 83% app zoom, which corroborate this approach.

@Ta-noshii

Copy link
Copy Markdown
Author

Thanks, both points were right.

  1. Confirmed: HostedBrowserWebview.test.tsx threw window.addEventListener is not a function on this branch. It now stubs addEventListener and removeEventListener as you suggested.
  2. Added useBrowserLayoutZoomFactor.test.tsx. It mounts a consumer, changes the mocked getWindowZoomFactor(), dispatches resize, and checks the layout factor follows the new zoom and returns when zoom is reset. With the resize subscription removed, the test fails. It also covers the fallback when the bridge has no getWindowZoomFactor.

The branch is rebased on current main (0fcd5f9).

Claude Opus 5.5 in Claude Code

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 1, 2026 20:50

Dismissing prior approval to re-evaluate f533d74

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
Ta-noshii and others added 2 commits October 14, 2026 02:38
…oomed

The hosted webview element was sized in the app's CSS pixels, which the
main window's zoom (View > Zoom In/Out) scales along with the rest of the
UI, while the guest keeps the preview zoom the tab was given. A 1280x800
viewport therefore measured 1402x876 inside the page after one zoom step,
and every freeform or preset preview_resize timed out because the
readiness check never saw the requested size.

Expose the main window's zoom factor on the desktop bridge and lay the
element out at previewZoom / windowZoom host pixels per guest pixel, so
the guest measures exactly the requested viewport. The device toolbar
default, mini player and agent cursor use the same ratio.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The layout zoom hook now subscribes to window resize, and the hosted
webview test stubs window as globalThis without event methods, so it
threw on mount. Stub addEventListener and removeEventListener there.

Add a hook test that changes the bridge's window zoom after mount,
dispatches resize, and checks the layout factor follows it, plus the
fallback for desktop shells without getWindowZoomFactor.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ta-noshii
Ta-noshii force-pushed the fix/preview-viewport-main-zoom branch from f533d74 to e37fd46 Compare October 14, 2026 00:45
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 14, 2026 00:45

Dismissing prior approval to re-evaluate e37fd46

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 14, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one for maintainer batch terminal-preview-13. I will preserve guest page zoom, resolve the current conflicts, trim the tests, and push the reviewed renderer layout fix to this branch. I read the parallel takeover of #10536; its review points here because multiplying guest zoom regresses fill mode. I will keep this PR focused on host-to-guest viewport sizing.

@macroscopeapp
macroscopeapp Bot dismissed their stale review October 19, 2026 05:18

Dismissing prior approval to re-evaluate 9a27f7c

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants