Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement Popover, DismissableLayer, and FocusTrap primitives in SwapSettings #1856

Merged
merged 5 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/many-glasses-visit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@coinbase/onchainkit": patch
---

- **feat**: Implement Popover, DismissableLayer, and FocusTrap primitives in SwapSettings. @cpcramer #1856
126 changes: 106 additions & 20 deletions src/internal/primitives/DismissableLayer.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fireEvent, render, screen } from '@testing-library/react';
import { useRef } from 'react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { DismissableLayer } from './DismissableLayer';

Expand Down Expand Up @@ -63,23 +64,73 @@ describe('DismissableLayer', () => {
</DismissableLayer>,
);

const innerElement = screen.getByTestId('inner');
fireEvent.pointerDown(innerElement);
fireEvent.pointerDown(screen.getByTestId('inner'));
expect(onDismiss).not.toHaveBeenCalled();
});

it('does not call onDismiss when clicking outside and disableOutsideClick is true', () => {
render(
<DismissableLayer onDismiss={onDismiss} disableOutsideClick={true}>
<div>Test Content</div>
</DismissableLayer>,
);
it('handles trigger clicks with preventTriggerBubbling', () => {
const TestComponent = () => {
const triggerRef = useRef<HTMLButtonElement>(null);
return (
<>
<button type="button" ref={triggerRef} data-testid="trigger">
Trigger
</button>
<DismissableLayer
onDismiss={onDismiss}
triggerRef={triggerRef}
preventTriggerEvents={true}
>
<div>Content</div>
</DismissableLayer>
</>
);
};

render(<TestComponent />);

const event = new Event('pointerdown', { bubbles: true });
Object.defineProperty(event, 'preventDefault', { value: vi.fn() });
Object.defineProperty(event, 'stopPropagation', { value: vi.fn() });

const trigger = screen.getByTestId('trigger');
trigger.dispatchEvent(event);

expect(event.preventDefault).toHaveBeenCalled();
expect(event.stopPropagation).toHaveBeenCalled();
expect(onDismiss).not.toHaveBeenCalled();
});

fireEvent.pointerDown(document.body);
it('handles trigger clicks without preventTriggerBubbling', () => {
const TestComponent = () => {
const triggerRef = useRef<HTMLButtonElement>(null);
return (
<>
<button type="button" ref={triggerRef} data-testid="trigger">
Trigger
</button>
<DismissableLayer onDismiss={onDismiss} triggerRef={triggerRef}>
<div>Content</div>
</DismissableLayer>
</>
);
};

render(<TestComponent />);

const event = new Event('pointerdown', { bubbles: true });
Object.defineProperty(event, 'preventDefault', { value: vi.fn() });
Object.defineProperty(event, 'stopPropagation', { value: vi.fn() });

const trigger = screen.getByTestId('trigger');
trigger.dispatchEvent(event);

expect(event.preventDefault).not.toHaveBeenCalled();
expect(event.stopPropagation).not.toHaveBeenCalled();
expect(onDismiss).not.toHaveBeenCalled();
});

it('handles case when both disableEscapeKey and disableOutsideClick are true', () => {
it('handles both disableEscapeKey and disableOutsideClick being true', () => {
render(
<DismissableLayer
onDismiss={onDismiss}
Expand Down Expand Up @@ -108,19 +159,54 @@ describe('DismissableLayer', () => {
});

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 TestComponent = () => {
const triggerRef = useRef<HTMLButtonElement>(null);
return (
<>
<button
type="button"
ref={triggerRef}
aria-label="Toggle swap settings"
>
Trigger
</button>
<DismissableLayer onDismiss={onDismiss} triggerRef={triggerRef}>
<div>Test Content</div>
</DismissableLayer>
</>
);
};

render(<TestComponent />);

const triggerButton = screen.getByLabelText('Toggle swap settings');
fireEvent.pointerDown(triggerButton);
expect(onDismiss).not.toHaveBeenCalled();
});

it('does not call onDismiss when clicking outside and disableOutsideClick is true', () => {
render(
<DismissableLayer onDismiss={onDismiss} disableOutsideClick={true}>
<div>Test Content</div>
</DismissableLayer>,
);

fireEvent.pointerDown(document.body);
expect(onDismiss).not.toHaveBeenCalled();
});

it('handles non-Node event target gracefully', () => {
render(
<DismissableLayer onDismiss={onDismiss}>
<div>Test Content</div>
</DismissableLayer>,
);

const event = new Event('pointerdown', { bubbles: true });
// Create a non-Node object as target
Object.defineProperty(event, 'target', { value: {} });
document.dispatchEvent(event);

expect(onDismiss).not.toHaveBeenCalled();
});
});
71 changes: 50 additions & 21 deletions src/internal/primitives/DismissableLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,26 @@ type DismissableLayerProps = {
disableEscapeKey?: boolean;
disableOutsideClick?: boolean;
onDismiss?: () => void;
/**
* Reference to the trigger element (e.g., button) that opens this layer.
* Prevents the layer from being dismissed when the trigger is clicked, enabling proper toggle behavior.
*/
triggerRef?: React.RefObject<HTMLElement>;
/**
* When `true`, prevents trigger click events from bubbling up.
* Useful for bottom sheets to prevent unwanted side effects.
*/
preventTriggerEvents?: boolean;
};

// DismissableLayer handles dismissal using outside clicks and escape key events
/** DismissableLayer handles dismissal using outside clicks and escape key events */
export function DismissableLayer({
children,
disableEscapeKey = false,
disableOutsideClick = false,
onDismiss,
triggerRef,
preventTriggerEvents = false,
}: DismissableLayerProps) {
const layerRef = useRef<HTMLDivElement>(null);

Expand All @@ -22,46 +34,63 @@ export function DismissableLayer({
return;
}

const handleKeyDown = (event: KeyboardEvent) => {
if (!disableEscapeKey && event.key === 'Escape') {
onDismiss?.();
const handleTriggerClick = (event: PointerEvent) => {
if (preventTriggerEvents) {
event.preventDefault();
event.stopPropagation();
}
};

const handlePointerDown = (event: PointerEvent) => {
const isClickInsideLayer = (target: Node) => {
return layerRef.current?.contains(target);
};

// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: TODO Refactor this component
const handlePointerDownCapture = (event: PointerEvent) => {
if (disableOutsideClick) {
return;
}

// 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)) {
if (!(event.target instanceof Node)) {
return;
}

// 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) {
const target = event.target;

if (triggerRef?.current?.contains(target)) {
handleTriggerClick(event);
return;
}

onDismiss?.();
if (!isClickInsideLayer(target)) {
onDismiss?.();
}
};

const handleKeyDown = (event: KeyboardEvent) => {
if (!disableEscapeKey && event.key === 'Escape') {
onDismiss?.();
}
};

document.addEventListener('pointerdown', handlePointerDownCapture, true);
document.addEventListener('keydown', handleKeyDown);
document.addEventListener('pointerdown', handlePointerDown);

return () => {
document.removeEventListener(
'pointerdown',
handlePointerDownCapture,
true,
);
document.removeEventListener('keydown', handleKeyDown);
document.removeEventListener('pointerdown', handlePointerDown);
};
}, [disableOutsideClick, disableEscapeKey, onDismiss]);
}, [
disableOutsideClick,
disableEscapeKey,
onDismiss,
triggerRef,
preventTriggerEvents,
]);

return (
<div data-testid="ockDismissableLayer" ref={layerRef}>
Expand Down
2 changes: 1 addition & 1 deletion src/internal/primitives/FocusTrap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface FocusTrapProps {
children?: React.ReactNode;
}

// FocusTrap ensures keyboard focus remains within a contained area for accessibility
/** FocusTrap ensures keyboard focus remains within a contained area for accessibility */
export function FocusTrap({ active = true, children }: FocusTrapProps) {
const containerRef = useRef<HTMLDivElement>(null);
const previousFocusRef = useRef<HTMLElement | null>(null);
Expand Down
Loading
Loading