fix: popover works correctly when inside modals, including polyfilled browsers

Author: thejackshelton

apps/website/src/routes/docs/headless/modal/examples/nested-popover.tsx

@@ -0,0 +1,38 @@
+import { component$, useSignal, useStyles$ } from '@builder.io/qwik';
+import {
+  Modal,
+  Select,
+  SelectPopover,
+  SelectListbox,
+  SelectOption,
+  SelectTrigger,
+  SelectValue,
+} from '@qwik-ui/headless';
+import styles from '../snippets/modal.css?inline';
+
+export default component$(() => {
+  useStyles$(styles);
+  const isOpen = useSignal(false);
+
+  return (
+    <>
+      <button class="modal-trigger" onClick$={() => (isOpen.value = true)}>
+        Open Modal
+      </button>
+      <Modal class="modal" bind:show={isOpen}>
+        Modal Content
+        <Select class="select">
+          <SelectTrigger class="select-trigger">
+            <SelectValue placeholder="Select an option" />
+          </SelectTrigger>
+          <SelectPopover class="select-popover">
+            <SelectListbox class="select-listbox">
+              <SelectOption>Option 1</SelectOption>
+              <SelectOption>Option 2</SelectOption>
+            </SelectListbox>
+          </SelectPopover>
+        </Select>
+      </Modal>
+    </>
+  );
+});

apps/website/src/routes/docs/headless/modal/index.mdx

@@ -15,6 +15,8 @@ A window overlaid on either the primary window or another dialog window. Modal c
 
 <Showcase name="hero" />
 
+<Showcase name="nested-popover" />
+
 The modal makes use of the HTML `dialog` element, which is supported in [every major browser](https://caniuse.com/?search=dialog).
 
 > For non-modal UI elements, it is preferred to use [Qwik UI's popover component](../../../docs/headless/popover/). Qwik's popover component can be applied to the most-semantically-relevant HTML element, including the `<dialog>` element itself for non-modal dialogs.

apps/website/src/routes/docs/headless/popover/index.mdx

@@ -23,7 +23,9 @@ A non-modal primitive with overlays that open and close around a DOM element.
   ]}
 />
 
-The Qwik UI Popover component is designed to align with the [HTML Popover API Specification](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API). As of now, native support for this API is around `72%`. To ensure consistent behavior across all browsers, Qwik UI provides a `polyfill` for browsers that do not support the API natively.
+The Qwik UI Popover component is designed to align with the [HTML Popover API Specification](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API). As of now, native support for this API is around `82.5%`, and in almost every browser.
+
+To ensure consistent behavior across all browsers, Qwik UI provides a `polyfill` for browser versions that do not support the API natively.
 
 <div class="flex flex-col gap-2">
   <div>

packages/kit-headless/src/components/modal/modal.tsx

@@ -63,14 +63,17 @@ export const Modal = component$((props: ModalProps) => {
     });
   });
 
-  const closeOnBackdropClick$ = $(async (event: MouseEvent) => {
+  const closeOnBackdropClick$ = $(async (e: MouseEvent) => {
     if (props.alert === true || props.closeOnBackdropClick === false) {
       return;
     }
 
-    console.log('CLICK BACKDROP', event);
+    // We do not want to close elements that dangle outside of the modal
+    if (!(e.target instanceof HTMLDialogElement)) {
+      return;
+    }
 
-    if (await wasModalBackdropClicked(modalRef.value, event)) {
+    if (await wasModalBackdropClicked(modalRef.value, e)) {
       showSig.value = false;
     }
   });

packages/kit-headless/src/components/modal/use-modal.tsx

@@ -50,7 +50,6 @@ export function useModal() {
     if (animationDuration === '0s' && transitionDuration === '0s') {
       modal.classList.remove('modal-closing');
       enableBodyScroll(modal);
-      console.log('modal closed: ', modal);
       modal.close();
     }
   });

packages/kit-headless/src/components/popover/popover-impl.tsx

@@ -55,6 +55,7 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
   /** have we rendered on the client yet? 0: no, 1: force, 2: yes */
   const hasRenderedOnClientSig = useSignal(isServer ? 0 : 2);
   const teleportSig = useSignal(false);
+  const hasTopLayerAncestorSig = useSignal(false);
 
   // This forces a re-render on each popover instance when the signal changes
   if (hasRenderedOnClientSig.value === 1) {
@@ -69,6 +70,19 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
 
     isPolyfillSig.value = true;
 
+    const findTopLayerAncestor$ = $((element: HTMLElement | null): HTMLElement | null => {
+      while (element?.parentElement) {
+        if (
+          element.parentElement?.tagName === 'DIALOG' ||
+          element.parentElement?.hasAttribute('popover')
+        ) {
+          return element.parentElement;
+        }
+        element = element.parentElement;
+      }
+      return null;
+    });
+
     let polyfillContainer: HTMLDivElement | null = document.querySelector(
       'div[data-qwik-ui-popover-polyfill]',
     );
@@ -80,7 +94,13 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
     }
 
     if (popoverRef.value) {
-      polyfillContainer.appendChild(popoverRef.value);
+      const topLayerAncestor = await findTopLayerAncestor$(popoverRef.value!);
+
+      if (topLayerAncestor === null) {
+        polyfillContainer.appendChild(popoverRef.value);
+      } else {
+        hasTopLayerAncestorSig.value = true;
+      }
 
       document.dispatchEvent(new CustomEvent('showpopover'));
 
@@ -119,7 +139,8 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
           // move opened polyfill popovers are always above the other
           if (
             popoverRef.value.classList.contains(':popover-open') &&
-            popoverRef.value.parentElement
+            popoverRef.value.parentElement &&
+            !hasTopLayerAncestorSig.value
           ) {
             popoverRef.value.parentElement.appendChild(popoverRef.value);
           }

packages/kit-headless/src/components/popover/utils.ts

@@ -1,11 +1,13 @@
+// TODO: fix the setTimeouts
+// TODO: move this file into the usePopover hook
+
 /**
  * Adds CSS-Class to support popover-opening-animation
  */
 export function supportShowAnimation(popover: HTMLElement, isPolyfill: boolean) {
   const { transitionDuration } = getComputedStyle(popover);
 
   if (isPolyfill) {
-    console.log('polyfill run!');
     // polyfill needs a bit of extra time to execute
     if (transitionDuration !== '0s') {
       setTimeout(() => {