fix(web): hidden preview tabs larger than the window capture whole #26965 · pingdotgg/t3code

MmacroscopeappBotapproved these changes14d ago

Approvability

Verdict: Approved at a5716e7

Macroscope's review found this PR approvable — Hidden previews now fit within the window so oversized tabs can be captured without losing their edges. The change preserves configured viewport dimensions and active-preview sizing, with limited presentation-only risk.

^{You can add or adjust custom eligibility rules. Learn more.}

CcoderabbitaiBot14d ago

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Hidden browser viewport sizing now accounts for the window dimensions. The webview passes those dimensions to a new resolver, which caps configured dimensions and preserves fill-mode sizing fallbacks.

Changes: Hidden preview viewport sizing

Layer / File(s)	Summary
Resolve hidden viewport dimensions `apps/web/src/browser/browserViewportLayout.ts`, `apps/web/src/browser/browserViewportLayout.test.ts`	The resolver caps configured dimensions to the window and retains fill-mode content, last-rectangle, and default sizing rules. Tests cover oversized and unchanged freeform dimensions.
Use window dimensions in the webview `apps/web/src/browser/HostedBrowserWebview.tsx`	The webview uses `useViewportSize` and passes the current dimensions to the resolver.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge, ta-noshii

Merge Risk: External image Minimal · up to 1d3b8

Hidden preview tabs larger than the window should now capture whole, and tabs that fit the window or use fill mode keep their behavior. No actionable merge-blocking risk is evident. Windows, Linux, non-100% page zoom, and window resizing during a pending capture were not manually verified.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title names the actual change: hidden preview tabs whose fixed viewport is larger than the window now capture whole. It follows the conventional commit form and is specific to the changeset, which…
Description check	Passed	The description follows the template well. It has Problem, Change, Scope, and Verification sections. The problem is stated with a concrete example and a linked issue. The change explains the capping r…
Linked Issues check	Passed	The only directly linked issue is `#26964`, which is open and has coding requirements. Its requirement is that a hidden preview tab with a fixed viewport larger than the window captures whole. The diff …
Out of Scope Changes check	Passed	The changes support `#26964` directly. They move the hidden-size calculation into `resolveHiddenBrowserViewportSize` so it can be tested. They add the window-size subscription needed to apply the cap on…

Full details: Title check

Explanation

The title names the actual change: hidden preview tabs whose fixed viewport is larger than the window now capture whole. It follows the conventional commit form and is specific to the changeset, which caps the hidden guest size at the window size and fits it with the existing layout.

Full details: Description check

Explanation

The description follows the template well. It has Problem, Change, Scope, and Verification sections. The problem is stated with a concrete example and a linked issue. The change explains the capping rule and the moved calculation. Scope explains why the change is a small bug fix and links the issue. Verification lists a new test, the test command results, a before/after manual check with screenshots, and what was not checked. The author also names the model and harness that did the work, as the template asks. Minor gap: the Scope section says the issue is not triaged yet, and the template asks for a maintainer request if one exists, which does not apply here.

Full details: Linked Issues check

Explanation

The only directly linked issue is #26964, which is open and has coding requirements. Its requirement is that a hidden preview tab with a fixed viewport larger than the window captures whole. The diff from merge base fa8949a2 to head 1d3b81c3 meets this requirement. resolveHiddenBrowserViewportSize in browserViewportLayout.ts caps non-fill hidden dimensions at the window size. HostedBrowserWebview uses this size and reads the window size through useSyncExternalStore on resize. The existing resolveBrowserViewportLayout fit then scales the guest into the window and keeps its CSS viewport. The new test in browserViewportLayout.test.ts covers a hidden 1440 × 1700 freeform viewport in a 1400 × 1000 window, which resolves to 1400 × 1000, and a 1280 × 800 viewport, which keeps its size. The PR reports a before and after capture (1280 × 1156 ending at y 1300, and 1280 × 1511 whole). This matches the expected result in the issue. Windows, Linux, non-100% zoom, and window resize during a pending capture are not verified. These gaps are noted, but they are not stated as coding requirements in the issue.

Full details: Out of Scope Changes check

Explanation

The changes support #26964 directly. They move the hidden-size calculation into resolveHiddenBrowserViewportSize so it can be tested. They add the window-size subscription needed to apply the cap on resize. They add one focused test. The summary also states that fill-mode sizing moves into the same helper, which replaces the inline rules. The PR description states that fill-mode and fitting tabs keep their behavior. The summary does not show a behavior change for these tabs, and this assessment does not find an unrelated change. The softer guest rendering is a stated tradeoff of the approach, not an unrelated change.

Finishing Touches

Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: fa8949a
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: `d8908bac-0e7e-452b-a3ae-8e840a22df12`

External image Commits

Reviewing files that changed from the base of the PR and between a5716e764aafb4713d0104894c1903cc3f2bc803 and 1d3b81c3fefad8b900520e87dc513d2c4e48e12e.

External image Files selected for processing: 1

`apps/web/src/browser/HostedBrowserWebview.tsx`

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

EEntrpi1d ago

Measurements for this one, from macOS 26.5 (arm64) with Electron 44.10.1 and a 2560x1600 viewport at page zoom 2 in a 1900x1200 T3 window. Without this PR, a hidden tab's capture came back 2470x1560 instead of 2560x1600. Chromium rasters a child frame only inside its compositing rect (RemoteFrameView::ComputeCompositingRect): the embedder viewport mapped into the frame, plus 15% on each side, with the origin clamped to 0. That's 1.3x the window here.

With this PR the capture is whole. Its fitted guest is scaled down, so it is a bit softer, as you noted. On my fork, which also draws a capturing guest with #31568's full-detail transform, the fitted guest was sharp too: 36 of 36 snapshots at 2560x1600 were whole, sharp and current. That covers light and heavy pages, with the window in front, covered or minimized.

One detail came up there. My fork's covered-window fix (discussed in #23139) draws one pixel of the guest's top-left corner above the app while capturing. Once the fitted guest was centred in its box, that pixel was no longer on the guest. Fitting with one scale for both sides, anchored at the box's top-left corner, fixed it. With main's placement this doesn't come up.

JJulius Marminge14h ago

The automated pipeline stopped on this pull request and will not merge it:

the fix is written and reviewed, but proving it needs something no worker has:

Permanent blocker: macOS desktop outside the device pool is required for the specified reproduction and is unavailable on this VM. The scenario runner cannot exercise desktop shell behavior, and the instructions prohibit substituting another platform for this requirement. The hidden 1440 × 1700 preview capture remains unverified on both commits.

It stays open for a person to finish or close.

fix(web): hidden preview tabs larger than the window capture whole#26965

Problem

Change

Scope and approval

Verification

Checks

Approvability