fix(popover): clean up popover a bit

Author: thejackshelton

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

@@ -15,9 +15,8 @@ export const ComboboxPopover = component$(
     /* REMEMBER, whenever an option is selected or onMouseDown$ the listbox is closed, and the popover should sync to that */
     useTask$(async ({ track }) => {
       track(() => context.isListboxOpenSig.value);
-      track(() => context.popoverRef.value);
 
-      console.log('POPOVER REF:', context.popoverRef.value);
+      console.log('LISTBOX OPEN?: ', context.isListboxOpenSig.value);
 
       if (context.isListboxOpenSig.value) {
         await initPopover$();

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

@@ -53,22 +53,13 @@ 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 isToggledSig = useSignal(false);
 
   // This forces a re-render on each popover instance when the signal changes
   if (hasRenderedOnClientSig.value === 1) {
     // Now run the task again after we force-rendered the contex
     setTimeout(() => (teleportSig.value = true), 0);
   }
 
-  useTask$(({ track }) => {
-    track(() => isToggledSig.value);
-
-    if (props.popoverRef) {
-      props.popoverRef.value = popoverRef.value;
-    }
-  });
-
   useTask$(async ({ track, cleanup }) => {
     track(() => teleportSig.value);
 
@@ -104,7 +95,10 @@ export const PopoverImpl = component$<PopoverImplProps>((props) => {
       popover={props.manual || props.popover === 'manual' ? 'manual' : 'auto'}
       ref={popoverRef}
       onToggle$={(e) => {
-        isToggledSig.value = true;
+        if (props.popoverRef) {
+          props.popoverRef.value = popoverRef.value;
+        }
+
         if (!popoverRef.value) return;
 
         const popover = popoverRef.value;

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

@@ -10,9 +10,13 @@ type PopoverTriggerProps = {
 
 export function usePopover(popovertarget: string) {
   const hasPolyfillLoadedSig = useSignal<boolean>(false);
-  const didInteractSig = useSignal<boolean>(false);
   const isSupportedSig = useSignal<boolean>(false);
 
+  const didInteractSig = useSignal<boolean>(false);
+  const popoverSig = useSignal<HTMLElement | null>(null);
+
+  const hookExecutedSig = useSignal<boolean>(false);
+
   const loadPolyfill$ = $(async () => {
     await import('@oddbird/popover-polyfill');
     document.dispatchEvent(new CustomEvent('poppolyload'));
@@ -26,7 +30,6 @@ export function usePopover(popovertarget: string) {
       'popover' in HTMLElement.prototype;
 
     isSupportedSig.value = isSupported;
-    console.log('INSIDE HERE: ', isSupportedSig.value);
 
     if (!hasPolyfillLoadedSig.value && !isSupported) {
       await loadPolyfill$();
@@ -38,18 +41,26 @@ export function usePopover(popovertarget: string) {
   useTask$(({ track }) => {
     track(() => didInteractSig.value);
 
-    if (isBrowser && isSupportedSig.value) {
-      const popover = document.getElementById(popovertarget);
+    if (!isBrowser) return;
 
-      if (!popover) return;
+    // get popover
+    if (!popoverSig.value) {
+      popoverSig.value = document.getElementById(popovertarget);
+    }
+
+    // so it only runs once on click for supported browsers
+    if (isSupportedSig.value) {
+      const popover = popoverSig.value;
 
-      console.log('inside interact task: ', popover);
-      console.log('popover id: ', popovertarget);
+      if (!popover) return;
 
       if (popover && popover.hasAttribute('popover')) {
         popover.showPopover();
       }
     }
+
+    console.log('HOOK EXECUTED');
+    hookExecutedSig.value = true;
   });
 
   // event is created after teleported properly
@@ -58,7 +69,7 @@ export function usePopover(popovertarget: string) {
     $(() => {
       if (!didInteractSig.value) return;
 
-      const popover = document.querySelector(`#${popovertarget}`);
+      const popover = popoverSig.value;
 
       if (!popover) return;
 
@@ -72,12 +83,12 @@ export function usePopover(popovertarget: string) {
     }),
   );
 
-  return { initPopover$, isSupportedSig };
+  return { initPopover$ };
 }
 
 export const PopoverTrigger = component$<PopoverTriggerProps>(
   ({ popovertarget, ...rest }: PopoverTriggerProps) => {
-    const { initPopover$, isSupportedSig } = usePopover(popovertarget);
+    const { initPopover$ } = usePopover(popovertarget);
 
     return (
       <button
@@ -87,7 +98,6 @@ export const PopoverTrigger = component$<PopoverTriggerProps>(
         onClick$={[
           rest.onClick$,
           $(() => {
-            console.log('isSupported: ', isSupportedSig.value);
             initPopover$();
           }),
         ]}