diff --git a/src/components/input/Select.tsx b/src/components/input/Select.tsx index 085415b02..400aaffbb 100644 --- a/src/components/input/Select.tsx +++ b/src/components/input/Select.tsx @@ -280,18 +280,18 @@ type ListboxCSSProps = * 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. */ -function useListboxPositioning( +function usePopoverPositioning( buttonRef: RefObject, - listboxRef: RefObject, + popoverRef: RefObject, listboxOpen: boolean, asPopover: boolean, alignListboxToRight: boolean, ) { const adjustListboxPositioning = useCallback(() => { - const listboxEl = listboxRef.current; + const popoverEl = popoverRef.current; const buttonEl = buttonRef.current; - if (!buttonEl || !listboxEl || !listboxOpen) { + if (!buttonEl || !popoverEl || !listboxOpen) { return () => {}; } @@ -303,9 +303,9 @@ function useListboxPositioning( const setListboxCSSProps = ( props: Partial>, ) => { - Object.assign(listboxEl.style, props); + Object.assign(popoverEl.style, props); const keys = Object.keys(props) as ListboxCSSProps[]; - return () => keys.map(prop => (listboxEl.style[prop] = '')); + return () => keys.map(prop => (popoverEl.style[prop] = '')); }; const viewportHeight = window.innerHeight; @@ -318,7 +318,7 @@ function useListboxPositioning( } = buttonEl.getBoundingClientRect(); const buttonDistanceToBottom = viewportHeight - buttonBottom; const { height: listboxHeight, width: listboxWidth } = - listboxEl.getBoundingClientRect(); + popoverEl.getBoundingClientRect(); // The listbox should drop up only if there's not enough space below to // fit it, and also, there's more absolute space above than below @@ -373,7 +373,7 @@ function useListboxPositioning( : `calc(${absBodyTop + buttonDistanceToTop + buttonHeight}px + ${LISTBOX_TOGGLE_GAP})`, left: `${Math.max(LISTBOX_VIEWPORT_HORIZONTAL_GAP, left)}px`, }); - }, [asPopover, buttonRef, listboxOpen, listboxRef, alignListboxToRight]); + }, [asPopover, buttonRef, listboxOpen, popoverRef, alignListboxToRight]); useLayoutEffect(() => { const cleanup = adjustListboxPositioning(); @@ -498,13 +498,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 +514,9 @@ function SelectMain({ const buttonRef = useSyncedRef(elementRef); const defaultButtonId = useId(); - useListboxPositioning( + usePopoverPositioning( buttonRef, - listboxRef, + popoverRef, listboxOpen, listboxAsPopover, alignListbox === 'right', @@ -538,7 +538,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 +604,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 0817825df..ab51ff0cf 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,19 +461,17 @@ 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) { + async function openListbox(wrapper) { toggleListbox(wrapper); await waitFor(() => !isListboxClosed(wrapper)); - - return getListbox(wrapper); } it('never renders a listbox bigger than the viewport', async () => { @@ -485,12 +483,13 @@ describe('Select', () => { }, ); - const listbox = await getOpenListbox(wrapper); - const { width: listboxWidth } = listbox + await openListbox(wrapper); + + const { width: popoverWidth } = getPopover(wrapper) .getDOMNode() .getBoundingClientRect(); - assert.isTrue(listboxWidth < window.innerWidth); + assert.isBelow(popoverWidth, window.innerWidth); }); [ @@ -579,24 +578,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; + await 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), ); });