Skip to content

Commit 31f1cc2

Browse files
cpcramercpcramer
authored
feat: Add Popover UI primitive (#1849)
1 parent 3fb0f2b commit 31f1cc2

File tree

6 files changed

+460
-27
lines changed

6 files changed

+460
-27
lines changed

‎.changeset/chatty-chairs-fail.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@coinbase/onchainkit": patch
3+
---
4+
5+
- **feat**: Add Popover UI Primitive. By @cpcramer #1849

‎src/internal/primitives/Dialog.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,11 @@ export function Dialog({
5151
<FocusTrap active={isOpen}>
5252
<DismissableLayer onDismiss={onClose}>
5353
<div
54-
aria-modal={modal}
54+
aria-describedby={ariaDescribedby}
5555
aria-label={ariaLabel}
5656
aria-labelledby={ariaLabelledby}
57-
aria-describedby={ariaDescribedby}
57+
aria-modal={modal}
58+
className="zoom-in-95 animate-in duration-200"
5859
data-testid="ockDialog"
5960
onClick={(e) => e.stopPropagation()}
6061
onKeyDown={(e: React.KeyboardEvent<HTMLDivElement>) => {
@@ -64,7 +65,6 @@ export function Dialog({
6465
}}
6566
ref={dialogRef}
6667
role="dialog"
67-
className="zoom-in-95 animate-in duration-200"
6868
>
6969
{children}
7070
</div>

‎src/internal/primitives/DismissableLayer.test.tsx

+17
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,21 @@ describe('DismissableLayer', () => {
106106
fireEvent.keyDown(document, { key: 'Escape' });
107107
fireEvent.pointerDown(document.body);
108108
});
109+
110+
it('does not call onDismiss when clicking the trigger button', () => {
111+
render(
112+
<>
113+
<button type="button" aria-label="Toggle swap settings">
114+
Trigger
115+
</button>
116+
<DismissableLayer onDismiss={onDismiss}>
117+
<div>Test Content</div>
118+
</DismissableLayer>
119+
</>,
120+
);
121+
122+
const triggerButton = screen.getByLabelText('Toggle swap settings');
123+
fireEvent.pointerDown(triggerButton);
124+
expect(onDismiss).not.toHaveBeenCalled();
125+
});
109126
});

‎src/internal/primitives/DismissableLayer.tsx

+20-24
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ export function DismissableLayer({
1616
onDismiss,
1717
}: DismissableLayerProps) {
1818
const layerRef = useRef<HTMLDivElement>(null);
19-
// Tracks whether the pointer event originated inside the React component tree
20-
const isPointerInsideReactTreeRef = useRef(false);
2119

2220
useEffect(() => {
2321
if (disableOutsideClick && disableEscapeKey) {
@@ -30,24 +28,30 @@ export function DismissableLayer({
3028
}
3129
};
3230

33-
const shouldDismiss = (target: Node) => {
34-
return layerRef.current && !layerRef.current.contains(target);
35-
};
36-
37-
// Handle clicks outside the layer
3831
const handlePointerDown = (event: PointerEvent) => {
39-
// Skip if outside clicks are disabled or if the click started inside the component
40-
if (disableOutsideClick || isPointerInsideReactTreeRef.current) {
41-
isPointerInsideReactTreeRef.current = false;
32+
if (disableOutsideClick) {
4233
return;
4334
}
4435

45-
// Dismiss if click is outside the layer
46-
if (shouldDismiss(event.target as Node)) {
47-
onDismiss?.();
36+
// If the click is inside the dismissable layer content, don't dismiss
37+
// This prevents the popover from closing when clicking inside it
38+
if (layerRef.current?.contains(event.target as Node)) {
39+
return;
4840
}
49-
// Reset the flag after handling the event
50-
isPointerInsideReactTreeRef.current = false;
41+
42+
// Handling for the trigger button (e.g., settings toggle)
43+
// Without this, clicking the trigger would cause both:
44+
// 1. The button's onClick to fire (toggling isOpen)
45+
// 2. This dismissal logic to fire (forcing close)
46+
// This would create a race condition where the popover rapidly closes and reopens
47+
const isTriggerClick = (event.target as HTMLElement).closest(
48+
'[aria-label="Toggle swap settings"]',
49+
);
50+
if (isTriggerClick) {
51+
return;
52+
}
53+
54+
onDismiss?.();
5155
};
5256

5357
document.addEventListener('keydown', handleKeyDown);
@@ -60,15 +64,7 @@ export function DismissableLayer({
6064
}, [disableOutsideClick, disableEscapeKey, onDismiss]);
6165

6266
return (
63-
<div
64-
data-testid="ockDismissableLayer"
65-
// Set flag when pointer event starts inside the component
66-
// This prevents dismissal when dragging from inside to outside
67-
onPointerDownCapture={() => {
68-
isPointerInsideReactTreeRef.current = true;
69-
}}
70-
ref={layerRef}
71-
>
67+
<div data-testid="ockDismissableLayer" ref={layerRef}>
7268
{children}
7369
</div>
7470
);

‎src/internal/primitives/Popover.test.tsx

+224
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
4+
import { Popover } from './Popover';
5+
6+
describe('Popover', () => {
7+
let anchorEl: HTMLElement;
8+
9+
beforeEach(() => {
10+
Object.defineProperty(window, 'matchMedia', {
11+
writable: true,
12+
value: vi.fn().mockImplementation((query) => ({
13+
matches: false,
14+
media: query,
15+
onchange: null,
16+
addListener: vi.fn(),
17+
removeListener: vi.fn(),
18+
addEventListener: vi.fn(),
19+
removeEventListener: vi.fn(),
20+
dispatchEvent: vi.fn(),
21+
})),
22+
});
23+
24+
anchorEl = document.createElement('button');
25+
anchorEl.setAttribute('data-testid', 'anchor');
26+
document.body.appendChild(anchorEl);
27+
});
28+
29+
afterEach(() => {
30+
cleanup();
31+
document.body.innerHTML = '';
32+
vi.clearAllMocks();
33+
});
34+
35+
describe('rendering', () => {
36+
it('should not render when isOpen is false', () => {
37+
render(
38+
<Popover anchorEl={anchorEl} isOpen={false}>
39+
Content
40+
</Popover>,
41+
);
42+
43+
expect(screen.queryByTestId('ockPopover')).not.toBeInTheDocument();
44+
});
45+
46+
it('should render when isOpen is true', () => {
47+
render(
48+
<Popover anchorEl={anchorEl} isOpen={true}>
49+
Content
50+
</Popover>,
51+
);
52+
53+
expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
54+
expect(screen.getByText('Content')).toBeInTheDocument();
55+
});
56+
57+
it('should handle null anchorEl gracefully', () => {
58+
render(
59+
<Popover anchorEl={null} isOpen={true}>
60+
Content
61+
</Popover>,
62+
);
63+
64+
expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
65+
});
66+
});
67+
68+
describe('positioning', () => {
69+
const positions = ['top', 'right', 'bottom', 'left'] as const;
70+
const alignments = ['start', 'center', 'end'] as const;
71+
72+
for (const position of positions) {
73+
for (const align of alignments) {
74+
it(`should position correctly with position=${position} and align=${align}`, () => {
75+
render(
76+
<Popover
77+
anchorEl={anchorEl}
78+
isOpen={true}
79+
position={position}
80+
align={align}
81+
offset={8}
82+
>
83+
Content
84+
</Popover>,
85+
);
86+
87+
const popover = screen.getByTestId('ockPopover');
88+
expect(popover).toBeInTheDocument();
89+
90+
expect(popover.style.top).toBeDefined();
91+
expect(popover.style.left).toBeDefined();
92+
});
93+
}
94+
}
95+
96+
it('should update position on window resize', async () => {
97+
render(
98+
<Popover anchorEl={anchorEl} isOpen={true}>
99+
Content
100+
</Popover>,
101+
);
102+
103+
fireEvent(window, new Event('resize'));
104+
105+
expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
106+
});
107+
108+
it('should update position on scroll', async () => {
109+
render(
110+
<Popover anchorEl={anchorEl} isOpen={true}>
111+
Content
112+
</Popover>,
113+
);
114+
115+
fireEvent.scroll(window);
116+
117+
expect(screen.getByTestId('ockPopover')).toBeInTheDocument();
118+
});
119+
120+
it('should handle missing getBoundingClientRect gracefully', () => {
121+
const originalGetBoundingClientRect =
122+
Element.prototype.getBoundingClientRect;
123+
Element.prototype.getBoundingClientRect = vi
124+
.fn()
125+
.mockReturnValue(undefined);
126+
127+
render(
128+
<Popover anchorEl={anchorEl} isOpen={true}>
129+
Content
130+
</Popover>,
131+
);
132+
133+
const popover = screen.getByTestId('ockPopover');
134+
expect(popover).toBeInTheDocument();
135+
136+
Element.prototype.getBoundingClientRect = originalGetBoundingClientRect;
137+
});
138+
});
139+
140+
describe('interactions', () => {
141+
it('should not call onClose when clicking inside', async () => {
142+
const onClose = vi.fn();
143+
render(
144+
<Popover anchorEl={anchorEl} isOpen={true} onClose={onClose}>
145+
Content
146+
</Popover>,
147+
);
148+
149+
fireEvent.mouseDown(screen.getByText('Content'));
150+
expect(onClose).not.toHaveBeenCalled();
151+
});
152+
153+
it('should call onClose when pressing Escape', async () => {
154+
const onClose = vi.fn();
155+
render(
156+
<Popover anchorEl={anchorEl} isOpen={true} onClose={onClose}>
157+
Content
158+
</Popover>,
159+
);
160+
161+
fireEvent.keyDown(document.body, { key: 'Escape' });
162+
expect(onClose).toHaveBeenCalled();
163+
});
164+
});
165+
166+
describe('accessibility', () => {
167+
it('should have correct ARIA attributes', () => {
168+
render(
169+
<Popover
170+
anchorEl={anchorEl}
171+
isOpen={true}
172+
aria-label="Test Label"
173+
aria-labelledby="labelId"
174+
aria-describedby="describeId"
175+
>
176+
Content
177+
</Popover>,
178+
);
179+
180+
const popover = screen.getByTestId('ockPopover');
181+
expect(popover).toHaveAttribute('role', 'dialog');
182+
expect(popover).toHaveAttribute('aria-label', 'Test Label');
183+
expect(popover).toHaveAttribute('aria-labelledby', 'labelId');
184+
expect(popover).toHaveAttribute('aria-describedby', 'describeId');
185+
});
186+
187+
it('should trap focus when open', async () => {
188+
const user = userEvent.setup();
189+
render(
190+
<Popover anchorEl={anchorEl} isOpen={true}>
191+
<button type="button">First</button>
192+
<button type="button">Second</button>
193+
</Popover>,
194+
);
195+
196+
const firstButton = screen.getByText('First');
197+
const secondButton = screen.getByText('Second');
198+
199+
firstButton.focus();
200+
expect(document.activeElement).toBe(firstButton);
201+
202+
await user.tab();
203+
expect(document.activeElement).toBe(secondButton);
204+
205+
await user.tab();
206+
expect(document.activeElement).toBe(firstButton);
207+
});
208+
});
209+
210+
describe('cleanup', () => {
211+
it('should remove event listeners on unmount', () => {
212+
const { unmount } = render(
213+
<Popover anchorEl={anchorEl} isOpen={true}>
214+
Content
215+
</Popover>,
216+
);
217+
218+
const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener');
219+
unmount();
220+
221+
expect(removeEventListenerSpy).toHaveBeenCalledTimes(2);
222+
});
223+
});
224+
});

0 commit comments

Comments
 (0)