Repository navigation
feat(browser): flash the page viewport size while the pane resizes - #31145
Conversation
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title is concise, specific, and accurately describes the primary user-facing change: showing viewport-size feedback while the browser pane resizes. |
| Linked Issues check | |
Issue #31014 is an active direct link. The PR adds a non-interactive top-right feedback badge for the built-in browser pane. A ResizeObserver detects real size changes. The implementation converts hos… |
| Out of Scope Changes check | |
The changes stay within issue #31014. The badge rename, shared zoom and viewport feedback state, conversion helpers, resize hook, and tests directly implement or verify the requested behavior. The cli… |
| Description check | |
The description is thorough and follows the template closely. It has an ELI5, What Changed, Why, a linked issue (Fixes #31014), Visual Proof with before/after videos, detailed Testing with a live grid… |
Full details: Linked Issues check
-
Explanation
Issue
#31014is an active direct link. The PR adds a non-interactive top-right feedback badge for the built-in browser pane. A ResizeObserver detects real size changes. The implementation converts host dimensions to guest CSS pixels with UI and live page zoom. It suppresses activation baselines, tab-switch feedback, preset resize feedback, and unreadable zoom values. The shared timer fades feedback after the existing interval. Unit and end-to-end tests cover conversion, zoom, activation, presets, inactive panes, and first-resize behavior.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#31014. The badge rename, shared zoom and viewport feedback state, conversion helpers, resize hook, and tests directly implement or verify the requested behavior. The client-hosted pane only adopts the renamed badge and does not add viewport flashing, which matches the stated scope. No unrelated product behavior is demonstrated.
Full details: Description check
-
Explanation
The description is thorough and follows the template closely. It has an ELI5, What Changed, Why, a linked issue (Fixes
#31014), Visual Proof with before/after videos, detailed Testing with a live grid and unit tests, an AI Disclosure, the Review section, the upstream boundary checkbox marked not applicable, Notes covering security, cross-platform, SSH/remote, mobile, and performance, and a checklist. The only unchecked item is the full pnpm lint/test/build run, which the author explicitly defers to CI and discloses. Minor gap: the Linked Issue section uses a 'Fixes#31014' line that the template expects, which is present. Overall the description is mostly complete and passes.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
54cede5 to
6762c6a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
32e2b81e-cd93-480a-b8a1-8e3076502dd8
📥 Commits
Reviewing files that changed from the base of the PR and between 54cede5bfcad1cd22c14926e6c975bfaef6b187c and 6762c6ac3a751bd6517f6aedc7a02ba5f5c4116d.
📒 Files selected for processing: 10
src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-pane.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-viewport-overlays.tsxsrc/renderer/src/components/browser-pane/host-guest/browser-page-viewport-size.test.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-viewport-size.tssrc/renderer/src/components/browser-pane/host-guest/use-browser-page-viewport-size-feedback.test.tsxsrc/renderer/src/components/browser-pane/host-guest/use-browser-page-viewport-size-feedback.tssrc/renderer/src/components/browser-pane/host-guest/use-browser-page-zoom-feedback.test.tssrc/renderer/src/components/browser-pane/host-guest/use-browser-page-zoom-feedback.tssrc/shared/browser-page-zoom.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.
Review follow-up (CodeRabbit review 5609286361)✅ Fixed (2)
❌ Rebutted (1)
⏭ Deferred (0)
Commit: 6cb5087327a (pushed to the PR branch) Tests: Generated by counter-review |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
0ce2d67b-6556-472b-a292-f1377eb4d321
📥 Commits
Reviewing files that changed from the base of the PR and between 6762c6ac3a751bd6517f6aedc7a02ba5f5c4116d and 6cb5087327a666c7d19ab4cb08724c25bf560a7c.
📒 Files selected for processing: 4
src/renderer/src/components/browser-pane/assemble-chrome/browser-page-pane.tsxsrc/renderer/src/components/browser-pane/host-guest/use-browser-page-viewport-size-feedback.test.tsxsrc/renderer/src/components/browser-pane/host-guest/use-browser-page-viewport-size-feedback.tssrc/renderer/src/components/browser-pane/host-guest/use-browser-page-zoom-feedback.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.
Review follow-up (CodeRabbit review 5623033503)✅ Fixed (1)
❌ Rebutted (0)
⏭ Deferred (0)
Commit: b521044eb97 (pushed to the PR branch) Tests: Reviews 5623053226 and 5623055638 were CodeRabbit confirmations on the two threads from the previous round; nothing to address there. Generated by counter-review |
Resizing the built-in browser pane gave no sign of the page's size, so landing on a CSS breakpoint meant guessing or reading innerWidth in DevTools. The existing zoom badge now also flashes the guest's innerWidth × innerHeight after each resize, so zoom and size share one corner instead of stacking. The size comes from the host box, UI zoom and the guest's live zoom level, because a UI zoom change overwrites the guest's zoom behind the pane's remembered level. Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The issue promises the size badge stays hidden when a tab opens or is shown again, but the e2e only checked it after the 1.4s fade had already passed, so a stray flash would go unnoticed. A renderer-side counter now records every size flash, and the hook test covers a pane that is hidden and shown again at a new size. Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Zoom and size feedback now share one label and one fade timer, so the latest change wins, but nothing exercised that hook directly. The new test checks that a size flash replaces the zoom label and restarts the fade instead of letting the earlier timer hide it early. Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After rebasing onto per-site zoom (stablyai#31621), zoom levels snap to Chrome presets such as 90%, so the badge drifted a pixel from innerWidth: the zoom factor snapped levels that UI zoom sets freely, and rounding did not match Blink. The factor now uses the guest's level as is, and the size mirrors Blink's AdjustForAbsoluteZoom::AdjustInt on the integer DIP size, checked against the e2e's measured guest sizes. Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Before dom-ready the guest's zoom cannot be read, and a saved site zoom lands at that moment, so sizing it at the default zoom could flash a wrong number. A flash left running when the page went inactive or was parked at 0×0 could also come back stale, since nothing measures it then. Both cases now skip or hide the flash, because no size is better than a misleading one. Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Skipping measurements until the guest zoom is readable also dropped the baseline, and a new tab's opening measurement usually arrives before dom-ready, so the first resize afterwards only set a baseline and never showed the badge. The baseline is now kept as the host box size and converted at the zoom readable when the next measurement comes, so no zoom is guessed and the first real resize is still announced. Refs stablyai#31014 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The case asserted that the label reads exactly `1024 × 768`, which restates the format string rather than any behavior, and main now treats such copied-string tests as low value (stablyai#33736, stablyai#33778). Refs stablyai#31014 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b521044 to
19432a1
Compare
… that don't prove it Moves the guest-size conversion into the hook that uses it, inlines the one-use label formatter and predicate, and drops the e2e spec, the badge timer test and the predicate tests. The conversion keeps its test against sizes measured from a real guest.
…of the pane's render A preset pins the page size, and mobile emulation's layout viewport can differ from the host box, so the readout only runs on a free-sized pane. The badge now owns its label and fade state behind a ref handle, so a resize drag re-renders only the badge instead of the whole page pane.
… the page metrics
|
Thanks @chucoding, I'm taking this over to land it and pushed commits on top. Changes: merged main in, folded the size helpers into the hook, moved the label/fade state into the badge so a drag re-renders only the badge, and skipped the readout while a preset, a phone or a remote viewer sets the page metrics (mobile emulation's layout viewport can differ from the box). I also dropped the e2e spec and the predicate/timer tests to keep CI lean. The conversion test with your measured sizes stays. |
Blink rounds the guest viewport in device pixels with the screen scale times page zoom as its factor, so ignoring the screen scale left the badge 1px off at most non-100% zoom levels on 2x screens.
 [Medium impact] The latest change appears safe to merge; the earlier missing preset-size flash remains a non-blocking issue. Findings
SummaryThis PR adds a shared badge for browser zoom and viewport-size feedback.
Reviews (4) · Last reviewed commit: "fix(browser): round the readout's guest ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Chromium 150's AdjustForAbsoluteZoom::AdjustInt adds 0.5 in float math before dividing when zoomed in and truncates with a 0.01 slack, so the +1 rule showed the badge 1px wide at many zoomed-in sizes.
… screen scale Chromium rounds a guest frame to whole DIPs and only then scales it to device pixels, so a fractional split width read one device pixel short.
A live sweep at 1.2 UI zoom on a 2x screen showed Chromium sizes the guest frame as the ceiling of the box in DIPs, not the nearest DIP.
ELI5
When you drag the built-in browser narrower or wider, a small
1024 × 768label now appears in the top-right corner for about a second, like Chrome DevTools does. The number is the page's owninnerWidth × innerHeight, so you can land exactly on a CSS breakpoint by dragging.What Changed
innerWidthin DevTools.ResizeObserveron the page's host box, attached only while the page is active and free-sized, so it costs nothing when idle.0 × 0, so tab opens and switches never count as resizes.devicePixelRatio / UI zoom), then Blink'sAdjustForAbsoluteZoom::AdjustIntwith zoom = screen scale × the guest's live zoom level.dom-ready), no size is flashed, but the baseline is kept so the first resize after a tab opens is still announced.BrowserPageZoomIndicator) becameBrowserPageFeedbackBadge. It owns its label and fade timer behind a ref handle, so zoom and size share one badge (the latest change wins), and a resize drag re-renders only the badge, not the whole page pane.ClientHostedBrowserPagePane) only adopts the renamed badge. An agent'sorca browser viewportoverride is applied in main with no renderer signal, so the readout can show the box size there.Why
1, 120%) is not one of the page zoom presets.getZoomLevel()is a synchronous IPC, made only while the pane is actually resizing.Math.round: plain rounding is off by 1 px at many sizes. Each step above was checked against a live 2× Mac across 100/90/80/50/110/125% page zoom and UI zoom +1. Theceiland screen-scale steps came from mismatches in that grid.executeJavaScript('[innerWidth, innerHeight]')is async. It can answer before the guest has re-laid out, which leaves a stale number at the end of a drag.Linked Issue
Fixes #31014
Visual Proof
Hidden dev build over CDP, macOS 2× screen, a local page that prints its own
innerWidth × innerHeight. Same script in each video: widen the left sidebar, open the right sidebar, hide the left sidebar, close the right sidebar.Before (main): the page size changes and nothing appears.
before.mp4
After (100% page zoom): the badge flashes
859 × 609,509 × 609,929 × 609,1279 × 609, matching the page's text, and fades.after.mp4
After (90% page zoom): the badge flashes
954 × 676,565 × 676,1032 × 676,1421 × 676, matching the page's text.after-zoom.mp4
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Live grid (hidden dev build, 2× Mac, badge vs the guest's real
innerWidth × innerHeightread over its own CDP target, page zoom set with real Cmd+−/=/0 keys):90%/110%/100%.Unit tests (
use-browser-page-viewport-size-feedback.test.tsx): the conversion against sizes measured from real guests (1× and 2× screens, UI zoom, fractional boxes), and the hook's baseline rules (first measurement, beforedom-ready, parked at0 × 0, reactivation).pnpm test src/renderer/src/components/browser-paneand changed-fileoxlint/oxfmtpass. The e2e spec from the first version was dropped to keep CI time down; the live grid above covers what it checked.Not verified: Linux, Windows, and 1.25×/1.5× screen scales.
AI Disclosure
Implemented with Claude Code (Claude Opus 5.5) under my direction, including the tests and this description. I reviewed the diff and the test results.
Review
To try (worktree
/Users/kelvinamoaba/orca/workspaces/orca/browser-viewport-size-readout):W × Hbadge shows top right, matchesinnerWidth × innerHeight, and fades about 1.4 s after you stop.Agent skill upstream boundary
Notes
pointer-events-noneand uses existing tokens.devicePixelRatio, so it should hold on fractional Windows/Linux scales, but only a 2× macOS screen was measured.getZoomLevel()per frame, and only the badge re-renders.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred). I ran typecheck, the browser-pane tests, changed-file lint, and the e2e build; the fullpnpm lintandpnpm testare left to CI.🤖 Generated with Claude Code