feat: Add Popover UI primitive

cpcramer · cpcramer · commit fa0cf6e7ce94 · 2025-01-22T11:02:03.000-08:00
diff --git a/.changeset/chatty-chairs-fail.md b/.changeset/chatty-chairs-fail.md
@@ -0,0 +1,5 @@
+---
+"@coinbase/onchainkit": patch
+---
+
+- **feat**: Add Popover UI Primitive. By @cpcramer #1849
diff --git a/src/internal/primitives/Dialog.tsx b/src/internal/primitives/Dialog.tsx
@@ -51,10 +51,11 @@ export function Dialog({
       <FocusTrap active={isOpen}>
         <DismissableLayer onDismiss={onClose}>
           <div
-            aria-modal={modal}
+            aria-describedby={ariaDescribedby}
             aria-label={ariaLabel}
             aria-labelledby={ariaLabelledby}
-            aria-describedby={ariaDescribedby}
+            aria-modal={modal}
+            className="zoom-in-95 animate-in duration-200"
             data-testid="ockDialog"
             onClick={(e) => e.stopPropagation()}
             onKeyDown={(e: React.KeyboardEvent<HTMLDivElement>) => {
@@ -64,7 +65,6 @@ export function Dialog({
             }}
             ref={dialogRef}
             role="dialog"
-            className="zoom-in-95 animate-in duration-200"
           >
             {children}
           </div>
diff --git a/src/internal/primitives/DismissableLayer.test.tsx b/src/internal/primitives/DismissableLayer.test.tsx
@@ -106,4 +106,21 @@ describe('DismissableLayer', () => {
     fireEvent.keyDown(document, { key: 'Escape' });
     fireEvent.pointerDown(document.body);
   });
+
+  it('does not call onDismiss when clicking the trigger button', () => {
+    render(
+      <>
+        <button type="button" aria-label="Toggle swap settings">
+          Trigger
+        </button>
+        <DismissableLayer onDismiss={onDismiss}>
+          <div>Test Content</div>
+        </DismissableLayer>
+      </>,
+    );
+
+    const triggerButton = screen.getByLabelText('Toggle swap settings');
+    fireEvent.pointerDown(triggerButton);
+    expect(onDismiss).not.toHaveBeenCalled();
+  });
 });
diff --git a/src/internal/primitives/DismissableLayer.tsx b/src/internal/primitives/DismissableLayer.tsx
@@ -16,8 +16,6 @@ export function DismissableLayer({
   onDismiss,
 }: DismissableLayerProps) {
   const layerRef = useRef<HTMLDivElement>(null);
-  // Tracks whether the pointer event originated inside the React component tree
-  const isPointerInsideReactTreeRef = useRef(false);
 
   useEffect(() => {
     if (disableOutsideClick && disableEscapeKey) {
@@ -30,24 +28,30 @@ export function DismissableLayer({
       }
     };
 
-    const shouldDismiss = (target: Node) => {
-      return layerRef.current && !layerRef.current.contains(target);
-    };
-
-    // Handle clicks outside the layer
     const handlePointerDown = (event: PointerEvent) => {
-      // Skip if outside clicks are disabled or if the click started inside the component
-      if (disableOutsideClick || isPointerInsideReactTreeRef.current) {
-        isPointerInsideReactTreeRef.current = false;
+      if (disableOutsideClick) {
         return;
       }
 
-      // Dismiss if click is outside the layer
-      if (shouldDismiss(event.target as Node)) {
-        onDismiss?.();
+      // If the click is inside the dismissable layer content, don't dismiss
+      // This prevents the popover from closing when clicking inside it
+      if (layerRef.current?.contains(event.target as Node)) {
+        return;
       }
-      // Reset the flag after handling the event
-      isPointerInsideReactTreeRef.current = false;
+
+      // Handling for the trigger button (e.g., settings toggle)
+      // Without this, clicking the trigger would cause both:
+      // 1. The button's onClick to fire (toggling isOpen)
+      // 2. This dismissal logic to fire (forcing close)
+      // This would create a race condition where the popover rapidly closes and reopens
+      const isTriggerClick = (event.target as HTMLElement).closest(
+        '[aria-label="Toggle swap settings"]',
+      );
+      if (isTriggerClick) {
+        return;
+      }
+
+      onDismiss?.();
     };
 
     document.addEventListener('keydown', handleKeyDown);
@@ -60,15 +64,7 @@ export function DismissableLayer({
   }, [disableOutsideClick, disableEscapeKey, onDismiss]);
 
   return (
-    <div
-      data-testid="ockDismissableLayer"
-      // Set flag when pointer event starts inside the component
-      // This prevents dismissal when dragging from inside to outside
-      onPointerDownCapture={() => {
-        isPointerInsideReactTreeRef.current = true;
-      }}
-      ref={layerRef}
-    >
+    <div data-testid="ockDismissableLayer" ref={layerRef}>
       {children}
     </div>
   );
diff --git a/src/internal/primitives/Popover.test.tsx b/src/internal/primitives/Popover.test.tsx
@@ -0,0 +1,256 @@
+import { fireEvent, render, screen } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { Popover } from './Popover';
+
+vi.mock('react-dom', () => ({
+  createPortal: (node: React.ReactNode) => node,
+}));
+
+describe('Popover', () => {
+  const onClose = vi.fn();
+  let anchorEl: HTMLElement;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    anchorEl = document.createElement('button');
+    anchorEl.setAttribute('data-testid', 'anchor');
+    document.body.appendChild(anchorEl);
+    vi.spyOn(anchorEl, 'getBoundingClientRect').mockReturnValue({
+      x: 100,
+      y: 100,
+      top: 100,
+      right: 200,
+      bottom: 200,
+      left: 100,
+      width: 100,
+      height: 100,
+      toJSON: () => {},
+    });
+  });
+
+  describe('rendering', () => {
+    it('renders nothing when isOpen is false', () => {
+      render(
+        <Popover isOpen={false} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+      expect(screen.queryByTestId('ockPopover')).not.toBeInTheDocument();
+    });
+
+    it('renders content when isOpen is true', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div data-testid="content">Content</div>
+        </Popover>,
+      );
+      expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
+      expect(screen.getByTestId('content')).toBeInTheDocument();
+    });
+  });
+
+  describe('accessibility', () => {
+    it('sets all ARIA attributes correctly', () => {
+      render(
+        <Popover
+          isOpen={true}
+          anchorEl={anchorEl}
+          aria-label="Test Popover"
+          aria-describedby="desc"
+          aria-labelledby="title"
+          onClose={onClose}
+        >
+          <div>Content</div>
+        </Popover>,
+      );
+
+      const popover = screen.getByTestId('ockPopover');
+      expect(popover).toHaveAttribute('role', 'dialog');
+      expect(popover).toHaveAttribute('aria-label', 'Test Popover');
+      expect(popover).toHaveAttribute('aria-describedby', 'desc');
+      expect(popover).toHaveAttribute('aria-labelledby', 'title');
+    });
+  });
+
+  describe('positioning', () => {
+    const testCases = [
+      {
+        position: 'top',
+        align: 'start',
+        expectedTop: -8,
+        expectedLeft: 100,
+      },
+      {
+        position: 'bottom',
+        align: 'center',
+        expectedTop: 208,
+        expectedLeft: 100,
+      },
+      {
+        position: 'left',
+        align: 'end',
+        expectedTop: 200,
+        expectedLeft: -8,
+      },
+      {
+        position: 'right',
+        align: 'center',
+        expectedTop: 150,
+        expectedLeft: 208,
+      },
+    ] as const;
+
+    for (const { position, align, expectedTop, expectedLeft } of testCases) {
+      it(`positions correctly with position=${position} and align=${align}`, () => {
+        render(
+          <Popover
+            isOpen={true}
+            anchorEl={anchorEl}
+            position={position}
+            align={align}
+            offset={8}
+            onClose={onClose}
+          >
+            <div style={{ width: '100px', height: '100px' }}>Content</div>
+          </Popover>,
+        );
+
+        const popover = screen.getByTestId('ockPopover');
+        vi.spyOn(popover, 'getBoundingClientRect').mockReturnValue({
+          width: 100,
+          height: 100,
+        } as DOMRect);
+
+        fireEvent(window, new Event('resize'));
+
+        expect(popover.style.top).toBe(`${expectedTop}px`);
+        expect(popover.style.left).toBe(`${expectedLeft}px`);
+      });
+    }
+
+    it('updates position on scroll', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      fireEvent.scroll(window);
+      expect(anchorEl.getBoundingClientRect).toHaveBeenCalled();
+    });
+  });
+
+  describe('dismissal behavior', () => {
+    it('calls onClose when clicking outside', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      fireEvent.pointerDown(document.body);
+      expect(onClose).toHaveBeenCalledTimes(1);
+    });
+
+    it('calls onClose when pressing Escape', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      fireEvent.keyDown(document, { key: 'Escape' });
+      expect(onClose).toHaveBeenCalledTimes(1);
+    });
+
+    it('handles undefined onClose prop gracefully', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      fireEvent.pointerDown(document.body);
+      fireEvent.keyDown(document, { key: 'Escape' });
+    });
+  });
+
+  describe('cleanup', () => {
+    it('removes event listeners on unmount', () => {
+      const { unmount } = render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener');
+      unmount();
+
+      expect(removeEventListenerSpy).toHaveBeenCalledWith(
+        'resize',
+        expect.any(Function),
+      );
+      expect(removeEventListenerSpy).toHaveBeenCalledWith(
+        'scroll',
+        expect.any(Function),
+      );
+    });
+  });
+
+  describe('portal rendering', () => {
+    it('renders in portal', () => {
+      const { baseElement } = render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+
+      expect(baseElement.contains(screen.getByTestId('ockPopover'))).toBe(true);
+    });
+  });
+
+  describe('edge cases', () => {
+    it('handles null anchorEl gracefully', () => {
+      render(
+        <Popover isOpen={true} anchorEl={null} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+      expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
+    });
+
+    it('handles missing getBoundingClientRect gracefully', () => {
+      const brokenAnchorEl = document.createElement('button');
+      vi.spyOn(brokenAnchorEl, 'getBoundingClientRect').mockReturnValue(
+        null as unknown as DOMRect,
+      );
+
+      render(
+        <Popover isOpen={true} anchorEl={brokenAnchorEl} onClose={onClose}>
+          <div>Content</div>
+        </Popover>,
+      );
+      expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
+    });
+  });
+
+  describe('focus management', () => {
+    it('traps focus when open', () => {
+      render(
+        <Popover isOpen={true} anchorEl={anchorEl} onClose={onClose}>
+          <button type="button">First</button>
+          <button type="button">Second</button>
+        </Popover>,
+      );
+
+      const firstButton = screen.getByText('First');
+      const secondButton = screen.getByText('Second');
+
+      firstButton.focus();
+      expect(document.activeElement).toBe(firstButton);
+
+      fireEvent.keyDown(firstButton, { key: 'Tab' });
+      expect(document.activeElement).toBe(secondButton);
+    });
+  });
+});
diff --git a/src/internal/primitives/Popover.tsx b/src/internal/primitives/Popover.tsx
