From cdb6b2fa1204e045e0c6f30ea02e4a0ae8f77ab7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 26 Nov 2024 11:17:45 +0100 Subject: [PATCH] Add onClose property to Popover --- src/components/feedback/Popover.tsx | 49 ++++++++++++++++- src/components/feedback/test/Popover-test.js | 53 ++++++++++++++++++- src/components/input/Select.tsx | 7 +-- src/components/input/test/Select-test.js | 35 ------------ .../patterns/feedback/PopoverPage.tsx | 15 ++++++ .../examples/popover-basic.tsx | 10 ++-- .../examples/popover-custom.tsx | 1 + .../examples/popover-right.tsx | 1 + 8 files changed, 125 insertions(+), 46 deletions(-) diff --git a/src/components/feedback/Popover.tsx b/src/components/feedback/Popover.tsx index 935abb26..49353489 100644 --- a/src/components/feedback/Popover.tsx +++ b/src/components/feedback/Popover.tsx @@ -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 */ @@ -159,6 +161,41 @@ 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, + 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[]; @@ -166,6 +203,13 @@ export type PopoverProps = { /** 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; @@ -194,6 +238,7 @@ export default function Popover({ anchorElementRef, children, open, + onClose, align = 'left', classes, variant = 'panel', @@ -211,6 +256,8 @@ export default function Popover({ align === 'right', ); + useOnClose(popoverRef, onClose, open, asNativePopover); + useLayoutEffect(() => { const restoreFocusTo = open ? (document.activeElement as HTMLElement) diff --git a/src/components/feedback/test/Popover-test.js b/src/components/feedback/test/Popover-test.js index 090673cf..a3ccd751 100644 --- a/src/components/feedback/test/Popover-test.js +++ b/src/components/feedback/test/Popover-test.js @@ -17,7 +17,12 @@ function TestComponent({ children, ...rest }) { > Anchor element - + {children ?? ( <> Content of popover @@ -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', () => { diff --git a/src/components/input/Select.tsx b/src/components/input/Select.tsx index 87073887..9cdcec93 100644 --- a/src/components/input/Select.tsx +++ b/src/components/input/Select.tsx @@ -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'; @@ -381,10 +379,8 @@ function SelectMain({ [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, { @@ -456,6 +452,7 @@ function SelectMain({ { 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); diff --git a/src/pattern-library/components/patterns/feedback/PopoverPage.tsx b/src/pattern-library/components/patterns/feedback/PopoverPage.tsx index 26a5b6a7..9f5e9668 100644 --- a/src/pattern-library/components/patterns/feedback/PopoverPage.tsx +++ b/src/pattern-library/components/patterns/feedback/PopoverPage.tsx @@ -127,6 +127,21 @@ export default function PopoverPage() { + + + + Callback invoked when the popover is automatically closed by + clicking away or pressing Escape. +
+ The caller should use this callback to keep local state in sync, + so that the popover is re-rendered with open set to{' '} + false. + + + {'() => void'} + + + diff --git a/src/pattern-library/examples/popover-basic.tsx b/src/pattern-library/examples/popover-basic.tsx index 8d6fbd16..e44656cc 100644 --- a/src/pattern-library/examples/popover-basic.tsx +++ b/src/pattern-library/examples/popover-basic.tsx @@ -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(null); - useClickAway(buttonRef, () => setOpen(false)); - return (
- + setOpen(false)} + anchorElementRef={buttonRef} + classes="p-2" + > The content of the popover goes here
diff --git a/src/pattern-library/examples/popover-custom.tsx b/src/pattern-library/examples/popover-custom.tsx index d8bab2e7..6657196f 100644 --- a/src/pattern-library/examples/popover-custom.tsx +++ b/src/pattern-library/examples/popover-custom.tsx @@ -21,6 +21,7 @@ export default function App() { 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" diff --git a/src/pattern-library/examples/popover-right.tsx b/src/pattern-library/examples/popover-right.tsx index c76d917f..9ea8a024 100644 --- a/src/pattern-library/examples/popover-right.tsx +++ b/src/pattern-library/examples/popover-right.tsx @@ -21,6 +21,7 @@ export default function App() { setOpen(false)} align="right" anchorElementRef={buttonRef} classes="p-2"