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 6 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 introduces new CDP viewport control capability via new IPC methods. An unresolved Medium-severity finding identifies a race condition where async viewport override calls may complete out of order, potentially leaving the guest viewport in a stale state. You can customize Macroscope's approvability policy. Learn more. |
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
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
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
There was a problem hiding this comment.
One ownership finding on the new guest-viewport override plumbing. The rest of the web-side changes (shared previewGuestViewport helper, automation rollback path, webview-owner effect) look consistent with the existing preview surface contracts.
Posted via Macroscope — UI Consistency
| const viewportOverrideKey = | ||
| viewport._tag === "fill" ? "fill" : `${viewport._tag}:${viewport.width}x${viewport.height}`; | ||
| const viewportRef = useRef(viewport); | ||
| viewportRef.current = viewport; | ||
| 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.
HostedBrowserWebview already owns this override for every tab: it applies previewBridge.setViewport right after registerWebview, and re-applies from an effect keyed on browserViewportSettingKey(effectiveViewport) + hasWebContents for the same runtimeTabId (and ElectronBrowserHost mounts it app-wide for every session). This effect duplicates that work for the active tab, and it pushes the committed viewport while the webview owner pushes the drag-live effectiveViewport — so any re-run here (e.g. hasWebContents flipping after a crash-recovery re-register) races the owner and can momentarily pin the guest to a stale size during a resize drag or a pending keyboard resize.
Suggest dropping this effect (plus the now-unused viewportOverrideKey/viewportRef) and leaving the guest override with the webview owner, so the behavior has a single owner.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Done. Removed the PreviewView effect. HostedBrowserWebview is the only guest-override owner.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
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.
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.
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.
The HostedBrowserWebview override ran one frame after mount, before registerWebview finished, then swallowed the failure. Crash recovery and mini-player never retried. The override now runs after a successful register, and drag updates wait for hasWebContents.
PreviewView reapplied the committed size after the webview owner already set the live override. That raced drags and keyboard resizes. The active-tab effect is gone; HostedBrowserWebview is the single owner.
6b94fa7 to
a202af0
Compare
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 a202af0. Configure here.
| ); | ||
| }); | ||
| return () => window.cancelAnimationFrame(frame); | ||
| }, [guestViewportKey, hasWebContents, runtimeTabId]); |
There was a problem hiding this comment.
Drag resize CDP race
Medium Severity
The guest viewport effect keys off effectiveViewport, including in-progress drag/keyboard sizes, and fires async setViewport IPC without cancelling in-flight calls. Cleanup only cancels the pending animation frame, so an older override can finish after a newer one and leave the guest stuck on a stale size.
Reviewed by Cursor Bugbot for commit a202af0. 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
Changes preview guest sizing via CDP and splits human vs automation control paths, but scope is localized to preview IPC/manager with rollback on automation failure and tests for the new behavior.
Overview
Fixes preview resize when the browser panel is hidden: persisted viewport settings now drive a CDP device-metrics override on the guest
webContents, not only CSS/React layout.Desktop:
PreviewManageraddssetViewport(toolbar/restore—CDP only, no agent badge) andautomationSetViewport(same CDP calls under agent control with a recordedresizeaction). Mobile emulation uses shortest-side < 768px. New IPC channels and bridge methods wire both paths.Web:
previewGuestViewportmapsPreviewViewportSettingto{ width, height }or{ clear: true }for fill mode.HostedBrowserWebviewreapplies after webview registration and when the effective viewport orhasWebContentschanges.PreviewAutomationHostspersists resize, then applies the override; failures roll back server state and guest metrics.Contracts add
DesktopPreviewAutomationSetViewportInputSchema; unit tests cover mapping, schema, and humansetViewportwithout agent control.Reviewed by Cursor Bugbot for commit a202af0. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Apply guest viewport CDP override when the preview panel is hidden
setViewportandautomationSetViewporttoPreviewManagerin Manager.ts, sendingEmulation.setDeviceMetricsOverrideorEmulation.clearDeviceMetricsOverridevia the Chrome DevTools Protocol without requiring the panel to be visible.desktop:preview-set-viewport,desktop:preview-automation-set-viewport) and thedesktopBridge.previewAPI in preload.ts.applyPreviewGuestViewporton registration and whenever the effective viewport setting changes.mobileflag in the CDP override is computed astruewhen the shortest dimension is less than 768px.Macroscope summarized a202af0.