fix(preview): apply viewport changes when the panel is hidden - #7303
fix(preview): apply viewport changes when the panel is hidden#7303gbarros-dev wants to merge 4 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the web-side changes for UI/behavior consistency. Two concerns about the new guest viewport override and how it interacts with the existing viewport-readiness contract; both are in apps/web/src/components/preview.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR adds new viewport override functionality via CDP rather than a simple fix. Multiple unresolved review comments identify race conditions where viewport application may fail silently or apply incorrectly after crash recovery or during drag operations. You can customize Macroscope's approvability policy. Learn more. |
preview_resize only updated the CSS/React chrome. If the browser panel was hidden, the guest never changed size and wait timed out. Resize now persists the setting, then applies a CDP device-metrics override so the guest viewport changes even when the tab is not visible.
A size-only ready check could resolve resize before React committed the webview geometry. A failed CDP apply also left the persisted snapshot on the new size. Resize now waits for the existing ready gates, rolls the snapshot back when the override fails, and re-applies the committed setting when the guest attaches.
6a56ea4 to
441e9f9
Compare
There was a problem hiding this comment.
Reviewed the web-side viewport override wiring (PreviewView.tsx, PreviewAutomationHosts.tsx, previewGuestViewport.ts). Two issues in the new PreviewView effect affect visible preview chrome; details inline.
Posted via Macroscope — UI Consistency
The restore effect depended on the viewport object, so every preview event re-sent CDP and flashed the agent-controlling badge. Toolbar resizes now use a setViewport path that does not take agent control, and the effect keys only on the viewport size string.
There was a problem hiding this comment.
One finding: the new guest viewport override is synced only from the committed viewport, so it desyncs from the <webview> box during a resize-handle drag.
Posted via Macroscope — UI Consistency
| useEffect(() => { | ||
| if (!runtimeTabId || !desktopOverlay?.hasWebContents) return; | ||
| void applyPreviewGuestViewport( | ||
| previewBridge?.setViewport, | ||
| runtimeTabId, | ||
| viewportRef.current, | ||
| ).catch(() => undefined); | ||
| }, [desktopOverlay?.hasWebContents, runtimeTabId, viewportOverrideKey]); |
There was a problem hiding this comment.
The override is keyed on the committed viewport, but the <webview> box follows the uncommitted drag: useBrowserViewportResize sets dragViewport on every pointermove, effectiveViewport/layout feed the element width/height (and data-preview-css-*) in HostedBrowserWebview, and the commit only happens on pointerup (or after the 150 ms keyboard-resize debounce).
Because the guest is now pinned by Emulation.setDeviceMetricsOverride to the previously committed size, the page stops reflowing while the frame is being dragged and snaps only at commit — previously the guest CSS viewport was the element box, so responsive breakpoints updated live under the drag badge. For the duration of the drag the badge and data-preview-css-width/height also disagree with what the guest reports.
Suggest owning the sync where the drag state is known — e.g. apply applyPreviewGuestViewport from effectiveViewport in HostedBrowserWebview/useBrowserViewportResize, keyed on browserViewportSettingKey(effectiveViewport) and throttled to a frame — so intermediate drag sizes reach the guest and the committed value remains the final write.
Posted via Macroscope — UI Consistency
Width-only mobile detection treated 844x390 phones as desktop. The guest also stayed pinned to the last committed size while the frame was dragged. mobile now uses the shortest side. HostedBrowserWebview applies a frame-throttled CDP override from the effective (including drag) viewport.
There was a problem hiding this comment.
One finding on the new guest-viewport application path. See the inline comment on apps/web/src/browser/HostedBrowserWebview.tsx.
Posted via Macroscope — UI Consistency
| useEffect(() => { | ||
| const setViewport = previewBridge?.setViewport; | ||
| if (!setViewport) return; | ||
| const frame = window.requestAnimationFrame(() => { | ||
| void applyPreviewGuestViewport(setViewport, runtimeTabId, guestViewportRef.current).catch( | ||
| () => undefined, | ||
| ); | ||
| }); | ||
| return () => window.cancelAnimationFrame(frame); | ||
| }, [guestViewportKey, runtimeTabId, webviewGeneration]); |
There was a problem hiding this comment.
This applier runs a single requestAnimationFrame after mount (and after a webviewGeneration recovery), but the guest is not registered yet at that point: register() first awaits lease.ready and then bridge.registerWebview(...) — two IPC roundtrips, usually settled after did-attach. The setViewport call therefore rejects (no webContents for the tab), .catch(() => undefined) swallows it, and nothing re-runs the effect afterwards, so the guest keeps reporting host-derived dimensions while the toolbar shows the fixed size.
It only looks correct today because PreviewView has a second, desktopOverlay?.hasWebContents-gated effect that reapplies once registration lands. Surfaces that present a tab without PreviewView do not get that recovery — e.g. a session opened straight into the floating mini-player (opening PiP closes the right panel, unmounting PreviewView) with a persisted non-fill viewport, and crash recovery while the panel is closed.
Suggest gating this effect on registration rather than a frame: read hasWebContents for tabId from the preview state that usePreviewBridge already publishes (desktopByTabId[tabId]?.hasWebContents) and add it to the dependency list, so the override is applied — and reapplied after recovery — as soon as the webview is registered.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d9ec290. Configure here.
| ); | ||
| }); | ||
| return () => window.cancelAnimationFrame(frame); | ||
| }, [guestViewportKey, runtimeTabId, webviewGeneration]); |
There was a problem hiding this comment.
Viewport apply races registration
Medium Severity
The new guest-viewport effect fires on webviewGeneration without waiting for registerWebview, and failures are swallowed. After a crash remount, registration can finish later while hasWebContents never goes false, so PreviewView does not re-apply either and the CDP override can stick wrong until the next viewport change.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d9ec290. Configure here.
1 participant
preview_resizeonly updated the CSS/React chrome. If the browser panel was hidden, the guest never changed size and wait timed out.Resize now persists the setting, then applies a CDP device-metrics override so the guest viewport changes even when the tab is not visible.
Fixes #3712.
Split out of closed #7127. Land after #7236 if both are touching preview hosts in the same week; this branch is based on current
mainand does not include #7236.Tests:
vp test run packages/contracts/src/preview.test.ts apps/web/src/components/preview/previewGuestViewport.test.tsImplemented with Grok 4.6 through Grok CLI.
Note
Medium Risk
Touches preview automation control sessions and CDP emulation on live webviews; incorrect rollback or metrics could affect resize reliability but scope is limited to desktop preview, not auth or data.
Overview
Fixes
preview_resizetiming out when the browser panel is hidden by resizing the guest page through CDP, not only React/CSS chrome.The desktop
PreviewManagergainssetViewport(Emulation.setDeviceMetricsOverride/clearDeviceMetricsOverride, mobile flag from shortest side) andautomationSetViewport, which uses the same metrics but runs inside an agent control session so aresizeaction is recorded. The human path deliberately avoids agent control so toolbar resizes do not show the agent badge.New IPC/preload/contract surface exposes
setViewportandautomation.setViewport. The web app addsapplyPreviewGuestViewportto mapPreviewViewportSetting(fill→ clear, fixed sizes → width/height) and calls it fromHostedBrowserWebview,PreviewView, and the automation resize handler after persisting server state, with rollback of both state and guest override on failure.Reviewed by Cursor Bugbot for commit d9ec290. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Apply CDP viewport overrides to the preview panel when it is hidden
setViewportandautomationSetViewportoperations toPreviewManagerthat sendEmulation.setDeviceMetricsOverride(orclearDeviceMetricsOverride) via the Electron debugger, so viewport changes take effect even when the panel is not visible.setViewportapplies the override without taking agent control;automationSetViewportparticipates in the control session and is recorded in theresizeaction stream.previewGuestViewport.tsmapsPreviewViewportSettingto the override payload and no-ops on older desktops that lack the bridge method.HostedBrowserWebviewandPreviewViewcallapplyPreviewGuestViewporton viewport or webview changes; automation resize inPreviewAutomationHostsapplies the guest override and rolls back both server and guest state on failure.Macroscope summarized d9ec290.