diff --git a/src/components/input/SelectNext.tsx b/src/components/input/SelectNext.tsx index 2cfb2127d..021940dfc 100644 --- a/src/components/input/SelectNext.tsx +++ b/src/components/input/SelectNext.tsx @@ -5,6 +5,7 @@ import { useContext, useId, useLayoutEffect, + useMemo, useRef, useState, } from 'preact/hooks'; @@ -15,6 +16,7 @@ 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 { ListenerCollection } from '../../util/listener-collection'; import { downcastRef } from '../../util/typing'; import { MenuCollapseIcon, MenuExpandIcon } from '../icons'; import { inputGroupStyles } from './InputGroup'; @@ -103,38 +105,115 @@ function SelectOption({ SelectOption.displayName = 'SelectNext.Option'; -function useShouldDropUp( +/** Small space to apply between the toggle button and the listbox */ +const LISTBOX_TOGGLE_GAP = '.25rem'; + +type ListboxCSSProps = + | 'top' + | 'left' + | 'minWidth' + | 'marginBottom' + | 'bottom' + | 'marginTop'; + +/** + * Manages the listbox 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. + */ +function useListboxPositioning( buttonRef: RefObject, listboxRef: RefObject, listboxOpen: boolean, -): boolean { - const [shouldListboxDropUp, setShouldListboxDropUp] = useState(false); + asPopover: boolean, + right: boolean, +) { + const adjustListboxPositioning = useCallback(() => { + const listboxEl = listboxRef.current; + const buttonEl = buttonRef.current; - useLayoutEffect(() => { - // Reset shouldListboxDropUp so that it does not affect calculations next - // time listbox opens - if (!buttonRef.current || !listboxRef.current || !listboxOpen) { - setShouldListboxDropUp(false); - return; + if (!buttonEl || !listboxEl || !listboxOpen) { + return () => {}; } + /** + * We need to set the positioning styles synchronously (not via `style` + * 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>, + ) => { + Object.assign(listboxEl.style, props); + const keys = Object.keys(props) as ListboxCSSProps[]; + return () => keys.map(prop => (listboxEl.style[prop] = '')); + }; + const viewportHeight = window.innerHeight; - const { top: buttonDistanceToTop, bottom: buttonBottom } = - buttonRef.current.getBoundingClientRect(); + const { + top: buttonDistanceToTop, + bottom: buttonBottom, + left: buttonLeft, + height: buttonHeight, + width: buttonWidth, + } = buttonEl.getBoundingClientRect(); const buttonDistanceToBottom = viewportHeight - buttonBottom; - const { bottom: listboxBottom } = - listboxRef.current.getBoundingClientRect(); - const listboxDistanceToBottom = viewportHeight - listboxBottom; + const { height: listboxHeight, width: listboxWidth } = + listboxEl.getBoundingClientRect(); // The listbox should drop up only if there's not enough space below to - // fit it, and there's also more absolute space above than below - setShouldListboxDropUp( - listboxDistanceToBottom < 0 && - buttonDistanceToTop > buttonDistanceToBottom, - ); - }, [buttonRef, listboxRef, listboxOpen]); - - return shouldListboxDropUp; + // fit it, and also, there's more absolute space above than below + const shouldListboxDropUp = + buttonDistanceToBottom < listboxHeight && + buttonDistanceToTop > buttonDistanceToBottom; + + if (asPopover) { + const { top: bodyTop } = document.body.getBoundingClientRect(); + const absBodyTop = Math.abs(bodyTop); + + return setListboxCSSProps({ + minWidth: `${buttonWidth}px`, + top: shouldListboxDropUp + ? `calc(${absBodyTop + buttonDistanceToTop - listboxHeight}px - ${LISTBOX_TOGGLE_GAP})` + : `calc(${absBodyTop + buttonDistanceToTop + buttonHeight}px + ${LISTBOX_TOGGLE_GAP})`, + left: + right && listboxWidth > buttonWidth + ? `${buttonLeft - (listboxWidth - buttonWidth)}px` + : `${buttonLeft}px`, + }); + } + + // Set styles for non-popover mode + if (shouldListboxDropUp) { + return setListboxCSSProps({ + bottom: '100%', + marginBottom: LISTBOX_TOGGLE_GAP, + }); + } + + return setListboxCSSProps({ top: '100%', marginTop: LISTBOX_TOGGLE_GAP }); + }, [asPopover, buttonRef, listboxOpen, listboxRef, right]); + + useLayoutEffect(() => { + const cleanup = adjustListboxPositioning(); + + if (!asPopover) { + return cleanup; + } + + // Readjust listbox position when any element scrolls, just in case that + // affected the toggle button position. + const listeners = new ListenerCollection(); + listeners.add(document.body, 'scroll', adjustListboxPositioning, { + capture: true, + }); + + return () => { + cleanup(); + listeners.removeAll(); + }; + }, [adjustListboxPositioning, asPopover]); } export type SelectProps = CompositeProps & { @@ -166,6 +245,12 @@ export type SelectProps = CompositeProps & { 'aria-label'?: string; 'aria-labelledby'?: string; + + /** + * Used to determine if the listbox should use the popover API. + * Defaults to true, as long as the browser supports it. + */ + listboxAsPopover?: boolean; }; function SelectMain({ @@ -182,18 +267,36 @@ function SelectMain({ right = false, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, + /* eslint-disable-next-line no-prototype-builtins */ + listboxAsPopover = HTMLElement.prototype.hasOwnProperty('popover'), }: SelectProps) { - const [listboxOpen, setListboxOpen] = useState(false); - const closeListbox = useCallback(() => setListboxOpen(false), []); const wrapperRef = useRef(null); const listboxRef = useRef(null); + const [listboxOpen, setListboxOpen] = useState(false); + const toggleListbox = useCallback( + (open: boolean) => { + setListboxOpen(open); + if (listboxAsPopover) { + listboxRef.current?.togglePopover(open); + } + }, + [listboxAsPopover], + ); + const closeListbox = useCallback(() => toggleListbox(false), [toggleListbox]); const listboxId = useId(); const buttonRef = useSyncedRef(elementRef); const defaultButtonId = useId(); - const shouldListboxDropUp = useShouldDropUp( + const extraProps = useMemo( + () => (listboxAsPopover ? { popover: true } : {}), + [listboxAsPopover], + ); + + useListboxPositioning( buttonRef, listboxRef, listboxOpen, + listboxAsPopover, + right, ); const selectValue = useCallback( @@ -257,11 +360,11 @@ function SelectMain({ aria-label={ariaLabel} aria-labelledby={ariaLabelledBy} ref={downcastRef(buttonRef)} - onClick={() => setListboxOpen(prev => !prev)} + onClick={() => toggleListbox(!listboxOpen)} onKeyDown={e => { if (e.key === 'ArrowDown' && !listboxOpen) { e.preventDefault(); - setListboxOpen(true); + toggleListbox(true); } }} data-testid="select-toggle-button" @@ -273,18 +376,17 @@ function SelectMain({
    ({ aria-labelledby={buttonId ?? defaultButtonId} aria-orientation="vertical" data-testid="select-listbox" + data-listbox-open={listboxOpen} > {children}
diff --git a/src/components/input/test/SelectNext-test.js b/src/components/input/test/SelectNext-test.js index 61cdd8cde..d3cd841f9 100644 --- a/src/components/input/test/SelectNext-test.js +++ b/src/components/input/test/SelectNext-test.js @@ -1,4 +1,4 @@ -import { checkAccessibility } from '@hypothesis/frontend-testing'; +import { checkAccessibility, waitFor } from '@hypothesis/frontend-testing'; import { mount } from 'enzyme'; import SelectNext from '../SelectNext'; @@ -21,13 +21,26 @@ describe('SelectNext', () => { * Whether to renders SelectNext.Option children with callback notation. * Used primarily to test and cover both branches. * Defaults to true. + * @param {boolean} [options.defaultListboxAsPopover] - + * Whether we should let the `listboxAsPopover` prop use default value + * if not explicitly provided, or we should initialize it instead. */ const createComponent = (props = {}, options = {}) => { - const { paddingTop = 0, optionsChildrenAsCallback = true } = options; + const { + paddingTop = 0, + optionsChildrenAsCallback = true, + defaultListboxAsPopover = false, + } = options; const container = document.createElement('div'); container.style.paddingTop = `${paddingTop}px`; document.body.append(container); + // Explicitly disable listboxAsPopover, unless requested differently or a + // value has been provided + if (!defaultListboxAsPopover && !('listboxAsPopover' in props)) { + props.listboxAsPopover = false; + } + const wrapper = mount( {items.map(item => ( @@ -74,10 +87,18 @@ describe('SelectNext', () => { const getListbox = wrapper => wrapper.find('[data-testid="select-listbox"]'); const isListboxClosed = wrapper => - getListbox(wrapper).prop('className').includes('hidden'); + getListbox(wrapper).prop('data-listbox-open') === false; - const listboxDidDropUp = wrapper => - getListbox(wrapper).prop('className').includes('bottom-full'); + const listboxDidDropUp = wrapper => { + const { top: listboxTop } = getListbox(wrapper) + .getDOMNode() + .getBoundingClientRect(); + const { top: buttonTop } = getToggleButton(wrapper) + .getDOMNode() + .getBoundingClientRect(); + + return listboxTop < buttonTop; + }; it('changes selected value when an option is clicked', () => { const onChange = sinon.stub(); @@ -261,11 +282,32 @@ describe('SelectNext', () => { }); [ - { containerPaddingTop: 0, shouldDropUp: false }, - { containerPaddingTop: 1000, shouldDropUp: true }, - ].forEach(({ containerPaddingTop, shouldDropUp }) => { + { + containerPaddingTop: 0, + shouldDropUp: false, + listboxAsPopover: true, + }, + { + containerPaddingTop: 0, + shouldDropUp: false, + listboxAsPopover: false, + }, + { + containerPaddingTop: 1000, + shouldDropUp: true, + listboxAsPopover: true, + }, + { + containerPaddingTop: 1000, + shouldDropUp: true, + listboxAsPopover: false, + }, + ].forEach(({ containerPaddingTop, shouldDropUp, listboxAsPopover }) => { it('makes listbox drop up or down based on available space below', () => { - const wrapper = createComponent({}, { paddingTop: containerPaddingTop }); + const wrapper = createComponent( + { listboxAsPopover }, + { paddingTop: containerPaddingTop }, + ); toggleListbox(wrapper); assert.equal(listboxDidDropUp(wrapper), shouldDropUp); @@ -282,6 +324,74 @@ describe('SelectNext', () => { }); }); + context('when popover is supported', () => { + it('opens listbox via popover API', async () => { + const wrapper = createComponent({ listboxAsPopover: true }); + let resolve; + const promise = new Promise(res => (resolve = res)); + + getListbox(wrapper).getDOMNode().addEventListener('toggle', resolve); + toggleListbox(wrapper); + + // This test will timeout if the toggle event is not dispatched + await promise; + }); + }); + + context('when listbox is bigger than toggle button', () => { + [ + // Inferring listboxAsPopover based on browser support + { + listboxAsPopover: 'default', + getListboxLeft: wrapper => { + const leftStyle = getListbox(wrapper).getDOMNode().style.left; + // Remove `px` unit indicator + return Number(leftStyle.replace('px', '')); + }, + }, + // Explicitly enabling listboxAsPopover + { + listboxAsPopover: true, + getListboxLeft: wrapper => { + const leftStyle = getListbox(wrapper).getDOMNode().style.left; + // Remove `px` unit indicator + return Number(leftStyle.replace('px', '')); + }, + }, + // Explicitly disabling listboxAsPopover + { + listboxAsPopover: false, + getListboxLeft: wrapper => + getListbox(wrapper).getDOMNode().getBoundingClientRect().left, + }, + ].forEach(({ listboxAsPopover, getListboxLeft }) => { + it('aligns listbox to the right if `right` prop is true', async () => { + const wrapper = createComponent( + { + listboxAsPopover: + typeof listboxAsPopover === 'boolean' + ? listboxAsPopover + : undefined, + right: true, + buttonClasses: '!w-8', // Set a small width in the button + }, + { defaultListboxAsPopover: listboxAsPopover === 'default' }, + ); + toggleListbox(wrapper); + + // Wait for listbox to be open + await waitFor(() => !isListboxClosed(wrapper)); + + const { left: buttonLeft } = getToggleButton(wrapper) + .getDOMNode() + .getBoundingClientRect(); + const listboxLeft = getListboxLeft(wrapper); + + assert.isTrue(listboxLeft < buttonLeft); + }); + }); + }); + it( 'should pass a11y checks', checkAccessibility([ diff --git a/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx b/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx index 4ba0b0829..ebd4bd089 100644 --- a/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx +++ b/src/pattern-library/components/patterns/prototype/SelectNextPage.tsx @@ -1,6 +1,7 @@ import classnames from 'classnames'; import { useCallback, useId, useMemo, useState } from 'preact/hooks'; +import { Link } from '../../../..'; import { ArrowLeftIcon, ArrowRightIcon } from '../../../../components/icons'; import type { SelectNextProps } from '../../../../components/input'; import { IconButton, InputGroup } from '../../../../components/input'; @@ -35,6 +36,7 @@ function SelectExample({ | 'listboxClasses' | 'disabled' | 'right' + | 'listboxAsPopover' > & { textOnly?: boolean; items?: ItemType[]; @@ -206,6 +208,23 @@ export default function SelectNextPage() { SelectNext toggles a listbox where Options {"'"} UI can be customized and values can be objects.

+

+ In browsers that support it, the listbox uses the{' '} + + popover + {' '} + attribute and gets toggled via{' '} + + popover API + + . Otherwise, it is rendered as an absolute-positioned element. +

@@ -469,6 +488,39 @@ export default function SelectNextPage() { + + + + Determines if the listbox should be rendered using the{' '} + + popover API + + . It{"'"}s mainly used as a test seam, but can help explicitly + disabling this behavior if needed. + + + boolean + + + true if the browser supports [popover] + . Otherwise it is false + + + +
+

+ When not using the popover API, the listbox will + be constrained by its container dimensions. +

+
+ +
+
+ +