From 4e705bc84bf39955852ffeeb58578859b9a2a589 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 19 Nov 2024 12:09:14 +0100 Subject: [PATCH] Split Select dual popover/listbox element in two --- src/components/input/Select.tsx | 154 ++++++++++++----------- src/components/input/test/Select-test.js | 65 +++++----- 2 files changed, 112 insertions(+), 107 deletions(-) diff --git a/src/components/input/Select.tsx b/src/components/input/Select.tsx index 085415b0..ec1eb174 100644 --- a/src/components/input/Select.tsx +++ b/src/components/input/Select.tsx @@ -257,16 +257,16 @@ function SelectOption({ SelectOption.displayName = 'Select.Option'; -/** Small space to apply between the toggle button and the listbox */ -const LISTBOX_TOGGLE_GAP = '.25rem'; +/** Small space to apply between the toggle button and the popover */ +const POPOVER_TOGGLE_GAP = '.25rem'; /** - * Space in pixels to apply between the listbox and the viewport sides to - * prevent the listbox from growing to the very edges. + * Space in pixels to apply between the popover and the viewport sides to + * prevent it from growing to the very edges. */ -export const LISTBOX_VIEWPORT_HORIZONTAL_GAP = 8; +export const POPOVER_VIEWPORT_HORIZONTAL_GAP = 8; -type ListboxCSSProps = +type PopoverCSSProps = | 'top' | 'left' | 'minWidth' @@ -275,23 +275,27 @@ type ListboxCSSProps = | 'marginTop'; /** - * Manages the listbox position manually to make sure it renders "next" to the + * Manages the popover position manually to make sure it renders "next" to the * toggle button (below or over). This is mainly needed when the listbox is used * as a popover, as that makes it render in the top layer, making it impossible * to position it relative to the toggle button via regular CSS. + * + * @param asNativePopover - Native popover API is used to toggle the popover + * @param alignToRight - Whether the popover should be aligned to the right side + * of the button or not */ -function useListboxPositioning( +function usePopoverPositioning( buttonRef: RefObject, - listboxRef: RefObject, - listboxOpen: boolean, - asPopover: boolean, - alignListboxToRight: boolean, + popoverRef: RefObject, + popoverOpen: boolean, + asNativePopover: boolean, + alignToRight: boolean, ) { - const adjustListboxPositioning = useCallback(() => { - const listboxEl = listboxRef.current; + const adjustPopoverPositioning = useCallback(() => { + const popoverEl = popoverRef.current; const buttonEl = buttonRef.current; - if (!buttonEl || !listboxEl || !listboxOpen) { + if (!buttonEl || !popoverEl || !popoverOpen) { return () => {}; } @@ -300,12 +304,12 @@ function useListboxPositioning( * prop and a piece of state), to make sure positioning happens before * `useArrowKeyNavigation` runs, focusing the first option in the listbox. */ - const setListboxCSSProps = ( - props: Partial>, + const setPopoverCSSProps = ( + props: Partial>, ) => { - Object.assign(listboxEl.style, props); - const keys = Object.keys(props) as ListboxCSSProps[]; - return () => keys.map(prop => (listboxEl.style[prop] = '')); + Object.assign(popoverEl.style, props); + const keys = Object.keys(props) as PopoverCSSProps[]; + return () => keys.map(prop => (popoverEl.style[prop] = '')); }; const viewportHeight = window.innerHeight; @@ -317,25 +321,25 @@ function useListboxPositioning( width: buttonWidth, } = buttonEl.getBoundingClientRect(); const buttonDistanceToBottom = viewportHeight - buttonBottom; - const { height: listboxHeight, width: listboxWidth } = - listboxEl.getBoundingClientRect(); + const { height: popoverHeight, width: popoverWidth } = + popoverEl.getBoundingClientRect(); - // The listbox should drop up only if there's not enough space below to + // The popover should drop up only if there's not enough space below to // fit it, and also, there's more absolute space above than below - const shouldListboxDropUp = - buttonDistanceToBottom < listboxHeight && + const shouldPopoverDropUp = + buttonDistanceToBottom < popoverHeight && buttonDistanceToTop > buttonDistanceToBottom; - if (!asPopover) { + if (!asNativePopover) { // Set styles for non-popover mode - if (shouldListboxDropUp) { - return setListboxCSSProps({ + if (shouldPopoverDropUp) { + return setPopoverCSSProps({ bottom: '100%', - marginBottom: LISTBOX_TOGGLE_GAP, + marginBottom: POPOVER_TOGGLE_GAP, }); } - return setListboxCSSProps({ top: '100%', marginTop: LISTBOX_TOGGLE_GAP }); + return setPopoverCSSProps({ top: '100%', marginTop: POPOVER_TOGGLE_GAP }); } const { top: bodyTop, width: bodyWidth } = @@ -348,44 +352,43 @@ function useListboxPositioning( // - right-aligned Selects: distance from right side of toggle button to // left side of viewport const availableSpace = - (alignListboxToRight - ? buttonLeft + buttonWidth - : bodyWidth - buttonLeft) - LISTBOX_VIEWPORT_HORIZONTAL_GAP; + (alignToRight ? buttonLeft + buttonWidth : bodyWidth - buttonLeft) - + POPOVER_VIEWPORT_HORIZONTAL_GAP; let left = buttonLeft; - if (listboxWidth > availableSpace) { - // If the listbox is not going to fit the available space, let it "grow" + if (popoverWidth > availableSpace) { + // If the popover is not going to fit the available space, let it "grow" // in the opposite direction - left = alignListboxToRight - ? LISTBOX_VIEWPORT_HORIZONTAL_GAP - : left - (listboxWidth - availableSpace); - } else if (alignListboxToRight && listboxWidth > buttonWidth) { - // If a right-aligned listbox fits the available space, but it's bigger + left = alignToRight + ? POPOVER_VIEWPORT_HORIZONTAL_GAP + : left - (popoverWidth - availableSpace); + } else if (alignToRight && popoverWidth > buttonWidth) { + // If a right-aligned popover fits the available space, but it's bigger // than the button, move it to the left so that it is aligned with the // right side of the button - left -= listboxWidth - buttonWidth; + left -= popoverWidth - buttonWidth; } - return setListboxCSSProps({ + return setPopoverCSSProps({ minWidth: `${buttonWidth}px`, - top: shouldListboxDropUp - ? `calc(${absBodyTop + buttonDistanceToTop - listboxHeight}px - ${LISTBOX_TOGGLE_GAP})` - : `calc(${absBodyTop + buttonDistanceToTop + buttonHeight}px + ${LISTBOX_TOGGLE_GAP})`, - left: `${Math.max(LISTBOX_VIEWPORT_HORIZONTAL_GAP, left)}px`, + top: shouldPopoverDropUp + ? `calc(${absBodyTop + buttonDistanceToTop - popoverHeight}px - ${POPOVER_TOGGLE_GAP})` + : `calc(${absBodyTop + buttonDistanceToTop + buttonHeight}px + ${POPOVER_TOGGLE_GAP})`, + left: `${Math.max(POPOVER_VIEWPORT_HORIZONTAL_GAP, left)}px`, }); - }, [asPopover, buttonRef, listboxOpen, listboxRef, alignListboxToRight]); + }, [asNativePopover, buttonRef, popoverOpen, popoverRef, alignToRight]); useLayoutEffect(() => { - const cleanup = adjustListboxPositioning(); + const cleanup = adjustPopoverPositioning(); - if (!asPopover) { + if (!asNativePopover) { return cleanup; } - // Readjust listbox position when any element scrolls, just in case that + // Readjust popover position when any element scrolls, just in case that // affected the toggle button position. const listeners = new ListenerCollection(); - listeners.add(document.body, 'scroll', adjustListboxPositioning, { + listeners.add(document.body, 'scroll', adjustPopoverPositioning, { capture: true, }); @@ -393,7 +396,7 @@ function useListboxPositioning( cleanup(); listeners.removeAll(); }; - }, [adjustListboxPositioning, asPopover]); + }, [adjustPopoverPositioning, asNativePopover]); } type SingleValueProps = { @@ -498,13 +501,13 @@ function SelectMain({ listboxAsPopover = HTMLElement.prototype.hasOwnProperty('popover'), }: SelectMainProps) { const wrapperRef = useRef(null); - const listboxRef = useRef(null); + const popoverRef = useRef(null); const [listboxOpen, setListboxOpen] = useState(false); const toggleListbox = useCallback( (open: boolean) => { setListboxOpen(open); if (listboxAsPopover) { - listboxRef.current?.togglePopover(open); + popoverRef.current?.togglePopover(open); } }, [listboxAsPopover], @@ -514,9 +517,9 @@ function SelectMain({ const buttonRef = useSyncedRef(elementRef); const defaultButtonId = useId(); - useListboxPositioning( + usePopoverPositioning( buttonRef, - listboxRef, + popoverRef, listboxOpen, listboxAsPopover, alignListbox === 'right', @@ -538,7 +541,7 @@ function SelectMain({ useKeyPress(['Escape'], closeListbox); // Vertical arrow key for options in the listbox - useArrowKeyNavigation(listboxRef, { + useArrowKeyNavigation(popoverRef, { horizontal: false, loop: false, autofocus: true, @@ -604,13 +607,17 @@ function SelectMain({ listboxOverflow, }} > -
    ({ }, listboxClasses, )} - role="listbox" - ref={listboxRef} - id={listboxId} - aria-multiselectable={multiple} - aria-labelledby={buttonId ?? defaultButtonId} - aria-orientation="vertical" - data-testid="select-listbox" - data-listbox-open={listboxOpen} + ref={popoverRef} // nb. Use `undefined` rather than `false` because Preact doesn't // handle boolean values correctly for this attribute (it will set // `popover="false"` instead of removing the attribute). popover={listboxAsPopover ? 'auto' : undefined} - onScroll={onListboxScroll} + data-testid="select-popover" > - {listboxOpen && children} -
+
    + {listboxOpen && children} +
+ ); diff --git a/src/components/input/test/Select-test.js b/src/components/input/test/Select-test.js index 0817825d..8e9fefd3 100644 --- a/src/components/input/test/Select-test.js +++ b/src/components/input/test/Select-test.js @@ -96,10 +96,10 @@ describe('Select', () => { const toggleListbox = wrapper => getToggleButton(wrapper).simulate('click'); - const getListbox = wrapper => wrapper.find('[data-testid="select-listbox"]'); + const getPopover = wrapper => wrapper.find('[data-testid="select-popover"]'); const isListboxClosed = wrapper => - getListbox(wrapper).prop('data-listbox-open') === false; + wrapper.find('[role="listbox"]').prop('data-listbox-open') === false; const openListbox = wrapper => { if (isListboxClosed(wrapper)) { @@ -108,14 +108,14 @@ describe('Select', () => { }; const listboxDidDropUp = wrapper => { - const { top: listboxTop } = getListbox(wrapper) + const { top: popoverTop } = getPopover(wrapper) .getDOMNode() .getBoundingClientRect(); const { top: buttonTop } = getToggleButton(wrapper) .getDOMNode() .getBoundingClientRect(); - return listboxTop < buttonTop; + return popoverTop < buttonTop; }; const getOption = (wrapper, id) => @@ -411,7 +411,7 @@ describe('Select', () => { let resolve; const promise = new Promise(res => (resolve = res)); - getListbox(wrapper).getDOMNode().addEventListener('toggle', resolve); + getPopover(wrapper).getDOMNode().addEventListener('toggle', resolve); toggleListbox(wrapper); // This test will time out if the toggle event is not dispatched @@ -425,8 +425,8 @@ describe('Select', () => { // Inferring listboxAsPopover based on browser support { listboxAsPopover: undefined, - getListboxLeft: wrapper => { - const leftStyle = getListbox(wrapper).getDOMNode().style.left; + getPopoverLeft: wrapper => { + const leftStyle = getPopover(wrapper).getDOMNode().style.left; // Remove `px` unit indicator return Number(leftStyle.replace('px', '')); }, @@ -434,8 +434,8 @@ describe('Select', () => { // Explicitly enabling listboxAsPopover { listboxAsPopover: true, - getListboxLeft: wrapper => { - const leftStyle = getListbox(wrapper).getDOMNode().style.left; + getPopoverLeft: wrapper => { + const leftStyle = getPopover(wrapper).getDOMNode().style.left; // Remove `px` unit indicator return Number(leftStyle.replace('px', '')); }, @@ -443,10 +443,10 @@ describe('Select', () => { // Explicitly disabling listboxAsPopover { listboxAsPopover: false, - getListboxLeft: wrapper => - getListbox(wrapper).getDOMNode().getBoundingClientRect().left, + getPopoverLeft: wrapper => + getPopover(wrapper).getDOMNode().getBoundingClientRect().left, }, - ].forEach(({ listboxAsPopover, getListboxLeft }) => { + ].forEach(({ listboxAsPopover, getPopoverLeft }) => { it('aligns listbox to the right if `alignListbox="right"` is provided', async () => { const wrapper = createComponent({ listboxAsPopover, @@ -461,22 +461,15 @@ describe('Select', () => { const { left: buttonLeft } = getToggleButton(wrapper) .getDOMNode() .getBoundingClientRect(); - const listboxLeft = getListboxLeft(wrapper); + const popoverLeft = getPopoverLeft(wrapper); - assert.isTrue(listboxLeft < buttonLeft); + assert.isAtMost(popoverLeft, buttonLeft); }); }); }); context('when listbox does not fit in available space', () => { - async function getOpenListbox(wrapper) { - toggleListbox(wrapper); - await waitFor(() => !isListboxClosed(wrapper)); - - return getListbox(wrapper); - } - - it('never renders a listbox bigger than the viewport', async () => { + it('never renders a listbox bigger than the viewport', () => { const name = 'name'.repeat(1000); const wrapper = createComponent( { buttonContent: 'Select a value' }, @@ -485,12 +478,13 @@ describe('Select', () => { }, ); - const listbox = await getOpenListbox(wrapper); - const { width: listboxWidth } = listbox + openListbox(wrapper); + + const { width: popoverWidth } = getPopover(wrapper) .getDOMNode() .getBoundingClientRect(); - assert.isTrue(listboxWidth < window.innerWidth); + assert.isBelow(popoverWidth, window.innerWidth); }); [ @@ -571,7 +565,7 @@ describe('Select', () => { }, }, ].forEach(({ name, alignListbox, getExpectedCoordinates }) => { - it('displays listbox in expected coordinates', async () => { + it('displays listbox in expected coordinates', () => { const wrapper = createComponent( { alignListbox, buttonContent: 'Select a value' }, { @@ -579,24 +573,25 @@ describe('Select', () => { }, ); - const listbox = await getOpenListbox(wrapper); - const listboxDOMNode = listbox.getDOMNode(); - const listboxStyleLeft = listboxDOMNode.style.left; - const listboxLeft = Number(listboxStyleLeft.replace('px', '')); - const listboxRight = - listboxDOMNode.getBoundingClientRect().width + listboxLeft; + openListbox(wrapper); + + const popoverDOMNode = getPopover(wrapper).getDOMNode(); + const popoverStyleLeft = popoverDOMNode.style.left; + const popoverLeft = Number(popoverStyleLeft.replace('px', '')); + const popoverRight = + popoverDOMNode.getBoundingClientRect().width + popoverLeft; const expectedCoordinates = getExpectedCoordinates( wrapper, - listboxDOMNode, + popoverDOMNode, ); assert.equal( - listboxLeft.toFixed(0), + popoverLeft.toFixed(0), expectedCoordinates.left.toFixed(0), ); assert.equal( - listboxRight.toFixed(0), + popoverRight.toFixed(0), expectedCoordinates.right.toFixed(0), ); });