Skip to content

Hide preview overlay while popups are open #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BunsDev wants to merge 1 commit into from
Closed

Hide preview overlay while popups are open #110

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion apps/web/src/components/PreviewPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ const HIDDEN_PREVIEW_BOUNDS = {
viewportHeight: 0,
} as const;

/**
* Selector that matches any popup positioner element portaled to the body.
* When any of these are present, the native BrowserView overlay should be
* hidden so it doesn't render on top of dropdown menus / popovers.
*/
const POPUP_POSITIONER_SELECTOR = [
'[data-slot="menu-positioner"]',
'[data-slot="popover-positioner"]',
'[data-slot="select-positioner"]',
'[data-slot="combobox-positioner"]',
'[data-slot="autocomplete-positioner"]',
].join(",");

function getActiveTab(state: PreviewTabsState): PreviewTabState | null {
if (!state.activeTabId) return null;
return state.tabs.find((t) => t.tabId === state.activeTabId) ?? null;
Expand Down Expand Up @@ -117,11 +130,15 @@ export function PreviewPanel({ threadId, onClose }: PreviewPanelProps) {
if (!element) return HIDDEN_PREVIEW_BOUNDS;

const rect = element.getBoundingClientRect();
// Hide the native BrowserView when any popup/dropdown is open so it
// doesn't render on top of menus (native overlays ignore CSS z-index).
const hasOpenPopup = document.querySelector(POPUP_POSITIONER_SELECTOR) !== null;
const visible =
tabsState.tabs.length > 0 &&
document.visibilityState === "visible" &&
rect.width > 0 &&
rect.height > 0;
rect.height > 0 &&
!hasOpenPopup;
Comment on lines +133 to +141
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computeBounds runs on every animation frame (the RAF loop in syncBounds). Adding document.querySelector(POPUP_POSITIONER_SELECTOR) here means we traverse the DOM ~60 times/sec while the preview bridge is active, even when the preview isn't eligible to be shown. Consider avoiding the per-frame query by (a) short-circuiting on the cheap checks first (tabs length, document visibility, rect size) and only querying when those pass, and/or (b) caching hasOpenPopup in a ref that’s updated by the MutationObserver / popup lifecycle so computeBounds is O(1).

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +141
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new behavior (hiding the native BrowserView overlay when popup positioners exist in the DOM), but there are currently no tests asserting the bounds/visibility behavior (existing test only checks the module exports). If feasible, add a unit test that stubs previewBridge.setBounds and verifies visible flips to false when an element matching POPUP_POSITIONER_SELECTOR is present and back to true when it’s removed.

Copilot uses AI. Check for mistakes.
return {
x: rect.left,
y: rect.top,
Expand Down Expand Up @@ -164,6 +181,11 @@ export function PreviewPanel({ threadId, onClose }: PreviewPanelProps) {
lastBoundsKey = "";
};

// Watch for popup positioners being added/removed from the DOM so we
// can immediately hide/show the native BrowserView overlay.
const popupObserver = new MutationObserver(invalidateBounds);
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MutationObserver invalidates bounds on any direct child add/remove under document.body, even when the change is unrelated to popup positioners (e.g., toast portals, other overlays). Because invalidateBounds forces lastBoundsKey to "", this can trigger extra previewBridge.setBounds(...) calls (IPC) with unchanged geometry. Consider filtering mutation records to only invalidate when nodes matching (or containing) POPUP_POSITIONER_SELECTOR are added/removed, or remove the observer entirely if the always-on RAF loop is sufficient for responsiveness.

Suggested change
const popupObserver = new MutationObserver(invalidateBounds);
const popupObserver = new MutationObserver((mutations) => {
for (const mutation of mutations) {
if (mutation.type !== "childList") continue;
for (const node of Array.from(mutation.addedNodes)) {
if (node instanceof Element) {
if (
(typeof (node as Element).matches === "function" &&
(node as Element).matches(POPUP_POSITIONER_SELECTOR)) ||
(typeof (node as Element).querySelector === "function" &&
(node as Element).querySelector(POPUP_POSITIONER_SELECTOR))
) {
invalidateBounds();
return;
}
}
}
for (const node of Array.from(mutation.removedNodes)) {
if (node instanceof Element) {
if (
(typeof (node as Element).matches === "function" &&
(node as Element).matches(POPUP_POSITIONER_SELECTOR)) ||
(typeof (node as Element).querySelector === "function" &&
(node as Element).querySelector(POPUP_POSITIONER_SELECTOR))
) {
invalidateBounds();
return;
}
}
}
}
});

Copilot uses AI. Check for mistakes.
popupObserver.observe(document.body, { childList: true, subtree: false });

window.addEventListener("resize", invalidateBounds);
window.addEventListener("scroll", invalidateBounds, true);
document.addEventListener("visibilitychange", invalidateBounds);
Expand All @@ -176,6 +198,7 @@ export function PreviewPanel({ threadId, onClose }: PreviewPanelProps) {
destroyed = true;
if (frameId !== 0) window.cancelAnimationFrame(frameId);
resizeObserver?.disconnect();
popupObserver.disconnect();
window.removeEventListener("resize", invalidateBounds);
window.removeEventListener("scroll", invalidateBounds, true);
document.removeEventListener("visibilitychange", invalidateBounds);
Expand Down
Loading