Merge pull request #717 from thejackshelton/fix-popover

Improve popover test suite

Author: thejackshelton

.changeset/six-cheetahs-bow.md

@@ -0,0 +1,5 @@
+---
+'@qwik-ui/headless': patch
+---
+
+fix: popover opening immediately in CSR

.changeset/thirty-carrots-sip.md

@@ -0,0 +1,5 @@
+---
+'@qwik-ui/headless': patch
+---
+
+fix: regression with popover polyfill executing unnecessary code

apps/website/src/routes/docs/headless/popover/examples/basic.tsx

@@ -3,10 +3,10 @@ import { Popover, PopoverTrigger } from '@qwik-ui/headless';
 export default component$(() => {
   return (
     <>
-      <PopoverTrigger popovertarget="hero-id" class="popover-trigger">
+      <PopoverTrigger popovertarget="basic-id" class="popover-trigger">
         Popover Trigger
       </PopoverTrigger>
-      <Popover id="hero-id" class="popover">
+      <Popover id="basic-id" class="popover">
         My Hero!
       </Popover>
     </>

apps/website/src/routes/docs/headless/popover/examples/hero.tsx

@@ -6,14 +6,14 @@ export default component$(() => {
 
   return (
     <>
-      <PopoverTrigger ref={triggerRef} popovertarget="popover-id" class="popover-trigger">
+      <PopoverTrigger ref={triggerRef} popovertarget="hero-id" class="popover-trigger">
         Click me
       </PopoverTrigger>
       <Popover
         anchorRef={triggerRef}
         floating={true}
         gutter={4}
-        id="popover-id"
+        id="hero-id"
         class="popover"
       >
         I am anchored to the popover trigger!

apps/website/src/routes/docs/headless/popover/examples/hide.tsx

@@ -0,0 +1,17 @@
+import { component$ } from '@builder.io/qwik';
+import { Popover, PopoverTrigger, usePopover } from '@qwik-ui/headless';
+export default component$(() => {
+  const { hidePopover } = usePopover(`hide-id`);
+
+  return (
+    <>
+      <button onClick$={() => hidePopover()}>hide popover</button>
+      <PopoverTrigger popovertarget="hide-id" class="popover-trigger">
+        Click me
+      </PopoverTrigger>
+      <Popover id="hide-id" class="popover">
+        My Hero!
+      </Popover>
+    </>
+  );
+});

apps/website/src/routes/docs/headless/popover/examples/show.tsx

@@ -0,0 +1,14 @@
+import { component$ } from '@builder.io/qwik';
+import { Popover, usePopover } from '@qwik-ui/headless';
+export default component$(() => {
+  const { showPopover } = usePopover(`show-id`);
+
+  return (
+    <>
+      <button onClick$={() => showPopover()}>show popover</button>
+      <Popover id="show-id" class="popover">
+        My Hero!
+      </Popover>
+    </>
+  );
+});

apps/website/src/routes/docs/headless/popover/examples/toggle.tsx

@@ -0,0 +1,14 @@
+import { component$ } from '@builder.io/qwik';
+import { Popover, usePopover } from '@qwik-ui/headless';
+export default component$(() => {
+  const { togglePopover } = usePopover(`toggle-id`);
+
+  return (
+    <>
+      <button onClick$={() => togglePopover()}>toggle popover</button>
+      <Popover id="toggle-id" class="popover">
+        My Hero!
+      </Popover>
+    </>
+  );
+});

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

@@ -21,6 +21,7 @@ export function usePopover(popovertarget: string) {
   const didInteractSig = useSignal<boolean>(false);
   const popoverSig = useSignal<HTMLElement | null>(null);
   const initialClickSig = useSignal<boolean>(false);
+  const isCSRSig = useSignal<boolean>(false);
 
   const loadPolyfill$ = $(async () => {
     await import('@oddbird/popover-polyfill');
@@ -43,17 +44,29 @@ export function usePopover(popovertarget: string) {
     didInteractSig.value = true;
   });
 
-  useTask$(async ({ track }) => {
+  useTask$(() => {
+    if (isBrowser) {
+      isCSRSig.value = true;
+    }
+  });
+
+  useTask$(({ track }) => {
     track(() => didInteractSig.value);
 
     if (!isBrowser) return;
 
     // get popover
     if (!popoverSig.value) {
       popoverSig.value = document.getElementById(popovertarget);
+    }
 
-      if (!initialClickSig.value) {
-        popoverSig.value?.showPopover();
+    // so it only runs once on click for supported browsers
+    if (isSupportedSig.value) {
+      if (!popoverSig.value) return;
+
+      if (!initialClickSig.value && !isCSRSig.value) {
+        /* opens manual on any event */
+        popoverSig.value.showPopover();
       }
     }
   });
@@ -69,10 +82,7 @@ export function usePopover(popovertarget: string) {
       // calls code in here twice for some reason, we think it's because of the client re-render, but it still works
 
       // so it only runs once on first click
-      if (
-        !popoverSig.value.classList.contains(':popover-open') &&
-        !isSupportedSig.value
-      ) {
+      if (!popoverSig.value.classList.contains(':popover-open')) {
         popoverSig.value.showPopover();
       }
     }),