Conversation
- Detect portal dropdown and popover positioners in the web UI - Suspend the native BrowserView overlay so menus render above it
github-actions
bot
added
size:S
vouch:trusted
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR prevents the native desktop preview (BrowserView) overlay from drawing above web UI popups by hiding the overlay whenever popup/dropdown/combobox “positioner” elements are present in the DOM, and attempts to react immediately to popup mount/unmount events.
Changes:
- Add a shared selector for Base UI popup “positioner” elements portaled to
document.body. - Hide the native preview overlay (
visible: false) when any matching positioner is present. - Add a
MutationObserverto invalidate bounds when DOM children are added/removed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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; |
There was a problem hiding this comment.
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).
|
|
||
| // 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); |
There was a problem hiding this comment.
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.
| 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; | |
| } | |
| } | |
| } | |
| } | |
| }); |
| // 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; |
There was a problem hiding this comment.
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.
Summary
MutationObserverso the preview bounds are invalidated immediately when popups mount or unmount.Testing