Merge pull request #716 from thejackshelton/fix-popover

refactor: improved popover middle tests

Author: thejackshelton

.changeset/blue-spiders-flash.md

@@ -0,0 +1,5 @@
+---
+'@qwik-ui/headless': patch
+---
+
+fix: race condition in the popover when programmatically opening on supported browsers

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

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

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

@@ -1,13 +1,22 @@
-import { component$ } from '@builder.io/qwik';
+import { component$, useSignal } from '@builder.io/qwik';
 import { Popover, PopoverTrigger } from '@qwik-ui/headless';
+
 export default component$(() => {
+  const triggerRef = useSignal<HTMLButtonElement>();
+
   return (
     <>
-      <PopoverTrigger popovertarget="hero-id" class="popover-trigger">
-        Popover Trigger
+      <PopoverTrigger ref={triggerRef} popovertarget="popover-id" class="popover-trigger">
+        Click me
       </PopoverTrigger>
-      <Popover id="hero-id" class="popover">
-        My Hero!
+      <Popover
+        anchorRef={triggerRef}
+        floating={true}
+        gutter={4}
+        id="popover-id"
+        class="popover"
+      >
+        I am anchored to the popover trigger!
       </Popover>
     </>
   );

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

@@ -23,9 +23,17 @@ A non-modal primitive with overlays that open and close around a DOM element.
   ]}
 />
 
+<Note status="warning">
+  You are about to see a fixed scrolling popover. Yes, this behavior is intended. It is
+  part of the native popover API! If you want to anchor items, there is a floating section
+  that covers this behavior.
+</Note>
+
+<Showcase name="basic" />
+
 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.
+To ensure consistent behavior across all browsers, Qwik UI provides a `polyfill` with feature parity for browser versions that do not support the API natively.
 
 <div class="flex flex-col gap-2">
   <div>
@@ -296,7 +304,9 @@ useTask$(async ({ track }) => {
 
 To use the popover API with floating elements, you can add the `floating={true}` prop to the Popover component. This API enables the use of JavaScript to dynamically position the listbox using the `top` & `left` absolute properties.
 
-> Enabling the `floating={true}` property will introduce a slight increase in code size, as we currently utilize JavaScript to implement floating items. We've strived to keep it as minimal as possible, but powerful in building complex components in anticipation of the forthcoming anchor API.
+Enabling the `floating={true}` property will introduce a slight increase in code size, as we currently utilize JavaScript to implement floating items. We've strived to keep it as minimal as possible, but powerful in building complex components.
+
+> Once the native anchor API is available in all browsers, you can remove the floating prop and use CSS instead! Smaller bundle size for the win!
 
 To float an element, it must have an anchored element to latch onto. This can be done with the `anchorRef` prop.
 

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

@@ -1,7 +1,9 @@
-import { type Locator, type Page } from '@playwright/test';
+import { expect, type Locator, type Page } from '@playwright/test';
 
 export type DriverLocator = Locator | Page;
 
+export type PopoverOpenKeys = 'Enter' | 'Space';
+
 export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
   const getPopover = () => {
     return rootLocator.locator('[popover]');
@@ -11,6 +13,21 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
     return rootLocator.locator('[popovertarget]');
   };
 
+  const openPopover = async (key: PopoverOpenKeys | 'click', index?: number) => {
+    const action = key === 'click' ? 'click' : 'press';
+    const trigger = index !== undefined ? getTrigger().nth(index) : getTrigger();
+    const popover = index !== undefined ? getPopover().nth(index) : getPopover();
+
+    if (action === 'click') {
+      await trigger.click({ position: { x: 1, y: 1 } }); // Modified line
+    } else {
+      await trigger.press(key);
+    }
+
+    // Needed because Playwright doesn't wait for the listbox to be visible
+    await expect(popover).toBeVisible();
+  };
+
   const getAllPopovers = () => {
     return getPopover().all();
   };
@@ -30,6 +47,7 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
     getAllPopovers,
     getTrigger,
     getAllTriggers,
+    openPopover,
     getProgrammaticButtonTrigger,
   };
 }