Skip to content

Commit

Permalink
Add onClose property to Popover
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Nov 26, 2024
1 parent 82734c2 commit cdb6b2f
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 46 deletions.
49 changes: 48 additions & 1 deletion src/components/feedback/Popover.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import classnames from 'classnames';
import type { ComponentChildren, RefObject } from 'preact';
import { useCallback, useLayoutEffect, useRef } from 'preact/hooks';
import { useCallback, useEffect, useLayoutEffect, useRef } from 'preact/hooks';

import { useClickAway } from '../../hooks/use-click-away';
import { useKeyPress } from '../../hooks/use-key-press';
import { ListenerCollection } from '../../util/listener-collection';

/** Small space to apply between the anchor element and the popover */
Expand Down Expand Up @@ -159,13 +161,55 @@ function usePopoverPositioning(
}, [adjustPopoverPositioning, asNativePopover, popoverOpen, popoverRef]);
}

/**
* Add the right listeners to the popover so that `onClose` is called when
* clicking away or pressing `Escape`.
*/
function useOnClose(
popoverRef: RefObject<HTMLElement | undefined>,
onClose: () => void,
popoverOpen: boolean,
asNativePopover: boolean,
) {
// When the popover API is used, listen for the `toggle` event and call
// onClose() when transitioning from `open` to `closed`.
// This happens when clicking away or pressing `Escape` key.
useEffect(() => {
if (!asNativePopover) {
return () => {};
}

const popover = popoverRef.current!;
const toggleListener = (e: ToggleEvent) => {
if (e.oldState === 'open' && e.newState === 'closed') {
onClose();
}
};

popover.addEventListener('toggle', toggleListener as any);
return () => popover.removeEventListener('toggle', toggleListener as any);
}, [asNativePopover, onClose, popoverRef]);

// When the popover API is not used, manually add listeners for Escape key
// press and click away, to mimic the native popover behavior.
useClickAway(popoverRef, onClose, { enabled: !asNativePopover });
useKeyPress(['Escape'], onClose, { enabled: !asNativePopover });
}

export type PopoverProps = {
children?: ComponentChildren;
classes?: string | string[];
variant?: 'panel' | 'custom';

/** Whether the popover is currently open or not */
open: boolean;

/**
* Callback invoked when the popover is automatically closed.
* Use this callback to sync local state.
*/
onClose: () => void;

/** The element relative to which the popover should be positioned */
anchorElementRef: RefObject<HTMLElement | undefined>;

Expand Down Expand Up @@ -194,6 +238,7 @@ export default function Popover({
anchorElementRef,
children,
open,
onClose,
align = 'left',
classes,
variant = 'panel',
Expand All @@ -211,6 +256,8 @@ export default function Popover({
align === 'right',
);

useOnClose(popoverRef, onClose, open, asNativePopover);

useLayoutEffect(() => {
const restoreFocusTo = open
? (document.activeElement as HTMLElement)
Expand Down
53 changes: 52 additions & 1 deletion src/components/feedback/test/Popover-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ function TestComponent({ children, ...rest }) {
>
Anchor element
</button>
<Popover open={open} anchorElementRef={buttonRef} {...rest}>
<Popover
open={open}
onClose={sinon.stub()}
anchorElementRef={buttonRef}
{...rest}
>
{children ?? (
<>
Content of popover
Expand Down Expand Up @@ -195,6 +200,52 @@ describe('Popover', () => {
assert.isBelow(popoverLeft, buttonLeft);
});
});

[
{ oldState: 'open', newState: 'closed', shouldCallOnClose: true },
{ oldState: 'open', newState: 'open', shouldCallOnClose: false },
{ oldState: 'closed', newState: 'open', shouldCallOnClose: false },
{ oldState: 'closed', newState: 'closed', shouldCallOnClose: false },
].forEach(({ shouldCallOnClose, ...eventInit }) => {
it('closes popover when toggle event is dispatched transitioning from open to closed', () => {
const onClose = sinon.stub();
const wrapper = createComponent({ onClose });

getPopover(wrapper)
.getDOMNode()
.dispatchEvent(new ToggleEvent('toggle', eventInit));

assert.equal(onClose.called, shouldCallOnClose);
});
});
});

context('when popover is not supported', () => {
it('closes popover when Escape is pressed', () => {
const onClose = sinon.stub();
createComponent({ onClose, asNativePopover: false });

document.body.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape' }),
);

assert.called(onClose);
});

it('closes popover when clicking away', () => {
const onClose = sinon.stub();
createComponent({ onClose, asNativePopover: false });

const externalButton = document.createElement('button');
document.body.append(externalButton);
externalButton.click();

try {
assert.called(onClose);
} finally {
externalButton.remove();
}
});
});

context('when popover does not fit in available space', () => {
Expand Down
7 changes: 2 additions & 5 deletions src/components/input/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import {
} from 'preact/hooks';

import { useArrowKeyNavigation } from '../../hooks/use-arrow-key-navigation';
import { useClickAway } from '../../hooks/use-click-away';
import { useFocusAway } from '../../hooks/use-focus-away';
import { useKeyPress } from '../../hooks/use-key-press';
import { useSyncedRef } from '../../hooks/use-synced-ref';
import type { CompositeProps } from '../../types';
import { downcastRef } from '../../util/typing';
Expand Down Expand Up @@ -381,10 +379,8 @@ function SelectMain<T>({
[onChange, closeListbox],
);

// When clicking away, focusing away or pressing `Esc`, close the listbox
useClickAway(wrapperRef, closeListbox);
// Close the listbox when focusing away
useFocusAway(wrapperRef, closeListbox);
useKeyPress(['Escape'], closeListbox);

// Vertical arrow key for options in the listbox
useArrowKeyNavigation(listboxRef, {
Expand Down Expand Up @@ -456,6 +452,7 @@ function SelectMain<T>({
<Popover
anchorElementRef={wrapperRef}
open={listboxOpen}
onClose={closeListbox}
asNativePopover={listboxAsPopover}
align={alignListbox}
classes={popoverClasses}
Expand Down
35 changes: 0 additions & 35 deletions src/components/input/test/Select-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,41 +221,6 @@ describe('Select', () => {
assert.isFalse(isListboxClosed(wrapper));
});

it('closes listbox when Escape is pressed', () => {
const wrapper = createComponent();

toggleListbox(wrapper);
assert.isFalse(isListboxClosed(wrapper));

document.body.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape' }),
);
wrapper.update();

// Listbox is closed after `Escape` is pressed
assert.isTrue(isListboxClosed(wrapper));
});

it('closes listbox when clicking away', () => {
const wrapper = createComponent();

toggleListbox(wrapper);
assert.isFalse(isListboxClosed(wrapper));

const externalButton = document.createElement('button');
document.body.append(externalButton);

externalButton.click();
wrapper.update();

try {
// Listbox is closed after other element is clicked
assert.isTrue(isListboxClosed(wrapper));
} finally {
externalButton.remove();
}
});

it('closes listbox when focusing away', () => {
const wrapper = createComponent();
toggleListbox(wrapper);
Expand Down
15 changes: 15 additions & 0 deletions src/pattern-library/components/patterns/feedback/PopoverPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@ export default function PopoverPage() {
</Library.InfoItem>
</Library.Info>
</Library.Example>
<Library.Example title="onClose">
<Library.Info>
<Library.InfoItem label="description">
Callback invoked when the popover is automatically closed by
clicking away or pressing <code>Escape</code>.
<br />
The caller should use this callback to keep local state in sync,
so that the popover is re-rendered with <code>open</code> set to{' '}
<code>false</code>.
</Library.InfoItem>
<Library.InfoItem label="type">
<code>{'() => void'}</code>
</Library.InfoItem>
</Library.Info>
</Library.Example>
<Library.Example title="open">
<Library.Info>
<Library.InfoItem label="description">
Expand Down
10 changes: 6 additions & 4 deletions src/pattern-library/examples/popover-basic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@ import { useRef, useState } from 'preact/hooks';

import { Popover } from '../../components/feedback';
import { Button } from '../../components/input';
import { useClickAway } from '../../hooks/use-click-away';

export default function App() {
const [open, setOpen] = useState(false);
const buttonRef = useRef<HTMLButtonElement | null>(null);

useClickAway(buttonRef, () => setOpen(false));

return (
<div className="relative flex justify-center">
<Button
Expand All @@ -19,7 +16,12 @@ export default function App() {
>
{open ? 'Close' : 'Open'} Popover
</Button>
<Popover open={open} anchorElementRef={buttonRef} classes="p-2">
<Popover
open={open}
onClose={() => setOpen(false)}
anchorElementRef={buttonRef}
classes="p-2"
>
The content of the popover goes here
</Popover>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/pattern-library/examples/popover-custom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default function App() {
</Button>
<Popover
open={open}
onClose={() => setOpen(false)}
anchorElementRef={buttonRef}
variant="custom"
classes="p-3 border-4 border-slate-7 bg-green-success text-white font-bold rounded-tr-lg rounded-bl-lg"
Expand Down
1 change: 1 addition & 0 deletions src/pattern-library/examples/popover-right.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default function App() {
</Button>
<Popover
open={open}
onClose={() => setOpen(false)}
align="right"
anchorElementRef={buttonRef}
classes="p-2"
Expand Down

0 comments on commit cdb6b2f

Please sign in to comment.