diff --git a/package.json b/package.json index 6cb48378a..0c6bf2343 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "@babel/preset-react": "^7.0.0", "@babel/preset-typescript": "^7.18.6", "@hypothesis/frontend-build": "^3.0.0", + "@hypothesis/frontend-testing": "^1.2.2", "@rollup/plugin-babel": "^6.0.0", "@rollup/plugin-commonjs": "^25.0.0", "@rollup/plugin-node-resolve": "^15.0.0", diff --git a/src/components/input/SelectNext.tsx b/src/components/input/SelectNext.tsx index 2cfb2127d..45dbf009e 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,108 @@ function SelectOption({ SelectOption.displayName = 'SelectNext.Option'; -function useShouldDropUp( +const LISTBOX_TOGGLE_GAP = '.25rem'; + +type ListboxCSSProps = + | 'top' + | 'left' + | 'minWidth' + | 'marginBottom' + | 'bottom' + | 'marginTop'; + +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 () => {}; } + /** + * Sets some CSS props in the listbox, and returns a cleanup function that + * resets them to their default values. + */ + const setListboxCSSProps = ( + props: Partial>, + ) => { + const entries = Object.entries(props) as Array<[ListboxCSSProps, string]>; + entries.forEach(([prop, value]) => (listboxEl.style[prop] = value)); + + return () => entries.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 +238,13 @@ export type SelectProps = CompositeProps & { 'aria-label'?: string; 'aria-labelledby'?: string; + + /** + * Test seam. + * Used to determine if current browser supports native popovers. + * Defaults to `'popover' in document.body` + */ + nativePopoverSupported?: boolean; }; function SelectMain({ @@ -182,18 +261,37 @@ function SelectMain({ right = false, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, + + /* istanbul ignore next - test seam */ + nativePopoverSupported = 'popover' in document.body, }: 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 (nativePopoverSupported) { + listboxRef.current?.togglePopover(open); + } + }, + [nativePopoverSupported], + ); + const closeListbox = useCallback(() => toggleListbox(false), [toggleListbox]); const listboxId = useId(); const buttonRef = useSyncedRef(elementRef); const defaultButtonId = useId(); - const shouldListboxDropUp = useShouldDropUp( + const extraProps = useMemo( + () => (nativePopoverSupported ? { popover: true } : {}), + [nativePopoverSupported], + ); + + useListboxPositioning( buttonRef, listboxRef, listboxOpen, + nativePopoverSupported, + right, ); const selectValue = useCallback( @@ -257,11 +355,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 +371,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 8a49bab51..d4f6a7fff 100644 --- a/src/components/input/test/SelectNext-test.js +++ b/src/components/input/test/SelectNext-test.js @@ -1,3 +1,4 @@ +import { waitFor } from '@hypothesis/frontend-testing'; import { mount } from 'enzyme'; import { checkAccessibility } from '../../../../test/util/accessibility.js'; @@ -28,6 +29,8 @@ describe('SelectNext', () => { container.style.paddingTop = `${paddingTop}px`; document.body.append(container); + props.nativePopoverSupported = props.nativePopoverSupported ?? false; + const wrapper = mount( {items.map(item => ( @@ -74,10 +77,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 => { + const { top: listboxTop } = getListbox(wrapper) + .getDOMNode() + .getBoundingClientRect(); + const { top: buttonTop } = getToggleButton(wrapper) + .getDOMNode() + .getBoundingClientRect(); - const listboxDidDropUp = wrapper => - getListbox(wrapper).prop('className').includes('bottom-full'); + return listboxTop < buttonTop; + }; it('changes selected value when an option is clicked', () => { const onChange = sinon.stub(); @@ -261,11 +272,32 @@ describe('SelectNext', () => { }); [ - { containerPaddingTop: 0, shouldDropUp: false }, - { containerPaddingTop: 1000, shouldDropUp: true }, - ].forEach(({ containerPaddingTop, shouldDropUp }) => { + { + containerPaddingTop: 0, + shouldDropUp: false, + nativePopoverSupported: true, + }, + { + containerPaddingTop: 0, + shouldDropUp: false, + nativePopoverSupported: false, + }, + { + containerPaddingTop: 1000, + shouldDropUp: true, + nativePopoverSupported: true, + }, + { + containerPaddingTop: 1000, + shouldDropUp: true, + nativePopoverSupported: false, + }, + ].forEach(({ containerPaddingTop, shouldDropUp, nativePopoverSupported }) => { it('makes listbox drop up or down based on available space below', () => { - const wrapper = createComponent({}, { paddingTop: containerPaddingTop }); + const wrapper = createComponent( + { nativePopoverSupported }, + { paddingTop: containerPaddingTop }, + ); toggleListbox(wrapper); assert.equal(listboxDidDropUp(wrapper), shouldDropUp); @@ -282,6 +314,57 @@ describe('SelectNext', () => { }); }); + context('when popover is supported', () => { + it('opens listbox via popover API', async () => { + const wrapper = createComponent({ nativePopoverSupported: 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', () => { + [ + { + nativePopoverSupported: true, + getListboxLeft: wrapper => { + const leftStyle = getListbox(wrapper).getDOMNode().style.left; + // Remove last 2 chars, which are the `px` unit indicator + return Number(leftStyle.slice(0, -2)); + }, + }, + { + nativePopoverSupported: false, + getListboxLeft: wrapper => + getListbox(wrapper).getDOMNode().getBoundingClientRect().left, + }, + ].forEach(({ nativePopoverSupported, getListboxLeft }) => { + it('aligns listbox to the right if `right` prop is true', async () => { + const wrapper = createComponent({ + nativePopoverSupported, + right: true, + buttonClasses: '!w-8', // Set a small width in the button + }); + 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..225cbd471 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'; @@ -206,6 +207,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. +

diff --git a/yarn.lock b/yarn.lock index 88ee40b94..90b41bf90 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2060,6 +2060,7 @@ __metadata: "@babel/preset-react": ^7.0.0 "@babel/preset-typescript": ^7.18.6 "@hypothesis/frontend-build": ^3.0.0 + "@hypothesis/frontend-testing": ^1.2.2 "@rollup/plugin-babel": ^6.0.0 "@rollup/plugin-commonjs": ^25.0.0 "@rollup/plugin-node-resolve": ^15.0.0 @@ -2112,6 +2113,17 @@ __metadata: languageName: unknown linkType: soft +"@hypothesis/frontend-testing@npm:^1.2.2": + version: 1.2.2 + resolution: "@hypothesis/frontend-testing@npm:1.2.2" + dependencies: + axe-core: ^4.8.2 + enzyme: ^3.11.0 + preact: ^10.18.1 + checksum: b3892f6f1db87387aa60a420f0d2a7c89c5031a1375273cce6777aba335f5d01b223a5fee499f1fbf418044eea6c85684d8488647e8b353d9637c05ba29d15de + languageName: node + linkType: hard + "@isaacs/cliui@npm:^8.0.2": version: 8.0.2 resolution: "@isaacs/cliui@npm:8.0.2" @@ -3335,6 +3347,13 @@ __metadata: languageName: node linkType: hard +"axe-core@npm:^4.8.2": + version: 4.9.1 + resolution: "axe-core@npm:4.9.1" + checksum: 41d9227871781f96c2952e2a777fca73624959dd0e98864f6d82806a77602f82b4fc490852082a7e524d8cd864e50d8b4d9931819b4a150112981d8c932110c5 + languageName: node + linkType: hard + "axobject-query@npm:^3.2.1": version: 3.2.1 resolution: "axobject-query@npm:3.2.1" @@ -4815,7 +4834,7 @@ __metadata: languageName: node linkType: hard -"enzyme@npm:^3.8.0": +"enzyme@npm:^3.11.0, enzyme@npm:^3.8.0": version: 3.11.0 resolution: "enzyme@npm:3.11.0" dependencies: @@ -9336,6 +9355,13 @@ __metadata: languageName: node linkType: hard +"preact@npm:^10.18.1": + version: 10.21.0 + resolution: "preact@npm:10.21.0" + checksum: 04acdd300e779eec230256d043120e564ac76bb535b05a9b23949f8641bf8e135098d0bd12c008cc4316174469356d09a2326dde6acbd8f539d1b4bf2231cbaa + languageName: node + linkType: hard + "preact@npm:^10.5.6": version: 10.20.2 resolution: "preact@npm:10.20.2"