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$();
  }),
  ]}