Repository navigation
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused desktop preview bug fix that composes stored preview zoom with host UI zoom across attach, reload/re-registration, and zoom-control paths. Its runtime scope is limited to viewport sizing and is covered by targeted regression tests, with no schema, infrastructure, security, billing, default, or static-analysis changes. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughPreview guest zoom now applies each tab’s stored zoom relative to the host WebContents zoom. Tests cover registration, zoom changes, replacement guests, and host zoom changes. ChangesRelative Preview Zoom
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Fixed issue severity: Merge Risk: ⚪ Minimal · up to The preview zoom change is covered across its main lifecycle paths. The reported one-pixel fractional differences do not establish a reproducible layout failure, so no actionable merge-blocking risk remains beyond normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Dismissing prior approval to re-evaluate 0d99f57
0d99f57 to
edb1bca
Compare
|
Note 🤖 Claude Opus 5.5 responding on behalf of Victor I tested this PR in the macOS desktop app with the main window zoomed. It fixes #12319. Setup: macOS 26.7.1 (arm64),
With the PR, Electron reports the guest zoom as 1.3145 at 100% and 1.0516 at 80% (0.8 × 1.3145). Agent, preview zoom 100%Before: before-1-window-zoom.mp4After: after-1-window-zoom.mp4Agent, preview zoom 80%Before: before-2-page-zoom.mp4After: after-2-page-zoom.mp4Toolbar preset, preview zoom 80%Before: before-3-toolbar.mp4After: after-3-toolbar.mp4Not checked: Windows and Linux. The 2104×1315 size from |
|
Note 🤖 Claude Opus 5.5 responding on behalf of Victor Update after #15328: #12319 is closed ( Setup: macOS 26.7.1 (arm64),
A live zoom change makes the guest follow the window (guest zoom 1.3145), so the size is right for a short time. When the preview attaches, reloads, or navigates, @Gigioxx the PR now conflicts with |
Preserve host zoom composition, reconcile shortcut test fixtures, and cover guest re-registration plus explicit preview zoom controls. Focused tests, desktop typecheck, lint, and formatting verified; skip the merge formatter hook to avoid rewriting unrelated upstream files.
Dismissing prior approval to re-evaluate 11b0f70
|
Note Astra responding on behalf of Guillermo @ViaxCo conflicts are resolved and pushed in Reran native UI verification on macOS, Windows, and Linux with Electron 44.4.2. At app 144%, a 390 × 844 toolbar measures 562 × 1216 before → 390 × 844 after in the fresh macOS comparison. Refresh, navigation, preview zoom, and same-draft close/reopen pass on all three platforms. Across 81 size/zoom combinations, 73 match exactly. The remaining eight differ by one pixel and reproduce in bare Electron without T3 preview code. Every 144% case matches exactly. The earlier reopen uncertainty is resolved using Reopen Closed View, with a new guest confirmed in the same draft. Updated screenshots, measurements, and verification report. No product source changes were needed for these follow-ups. Linux used X11/Xvfb; agent MCP commands were not independently rerun. |
|
Note Written by Hi! We are cleaning up open PRs, and this one appears to have been created with an older model ( |
|
Note Written by Reopening, this was closed by mistake. Sorry for the noise! |
|
Taking over this one. I'll resolve the conflict with main, fold the host zoom into the one place main now applies guest zoom, trim the tests, and push to this branch. |
|
Update after the takeover review: I am not pushing a rebuilt version of this branch, and I'm leaving the merge decision to a human. What the review found (two independent reviewers, confirmed against the code and the triage on #10525):
So the diagnosis, the measurement matrix, and the cross-platform evidence in this PR are solid and were useful, but I think the fix belongs in the renderer's box sizing rather than in A maintainer should decide: close this in favour of the box-sizing approach (#14039, which needs a rebase and its own review), or confirm that fill mode is allowed to zoom with the UI, in which case this branch can be revisited. Local takeover state for reference: |
|
Note Grok responding on behalf of Julius. Closing as superseded by #14039, which just merged and fixes the same zoomed-app preview viewport bug (#10525) by compensating renderer geometry for host zoom while keeping guest page zoom separate. Thanks for the detailed cross-platform evidence here, it helped confirm the bug. If you see a case #14039 misses, please open a fresh issue or PR against current main. |
Problem
App UI zoom enlarged the desktop preview's CSS viewport without changing the device toolbar dimensions. At 144% app zoom, a 375 x 667 preview laid out at 540 x 960 and selected desktop breakpoints.
Change
Include the host zoom factor in the three places that apply guest zoom. The tab's saved zoom stays independent; attachment, reload, app zoom changes, and preview zoom controls use the same composition. No renderer or protocol changes.
Scope and approval
Fixes #10525. Maintainer triage confirmed it as a real desktop Preview bug and traced it to guest zoom not accounting for the host page zoom: #10525 (comment)
Verification
Fresh native UI checks on
11b0f70c68, Electron 44.4.2: screenshots, measurements, and full report.9a3070bcf0and preserved host zoom composition. 131/131 focused PreviewManager/DesktopWindow tests pass, with extended coverage for guest re-registration, zoom-out, direct zoom-factor changes, and reset. Three zoom regression cases fail against unfixed main.git diff --checkpass. Changed-file lint reports only the existingClipboardItemmock warning. No source changes during the UI follow-ups. Repo-wide checks were left to CI; PR checks were green when inspected.Windows and Linux screenshots
Original Linux evidence, retained as historical
Original Electron 43.4.1 pass checked 36 combinations, with all 144% cases exact and four fractional cases differing by one pixel. Reload and device-toolbar close/reopen passed.
Original change by GPT-6 via Codex. Rebase and description update by Claude Opus 5.5 via Claude Code. Latest conflict resolution, regression tests, and cross-platform UI verification by GPT-6 Astra via Codex in T3 Code.
Note
Fix preview viewport zoom to account for host window zoom factor
PreviewManager.assertTabZoom,PreviewManager.registerWebview, andPreviewManager.applyZoomto scale guest zoom writes by the host factor while storing the unscaled value as the tab's logical zoom.PreviewManager.reapplyZoomre-asserts each tab's zoom relative to the current host zoom after app UI zoom changes.Macroscope summarized 0d99f57.