From 90cfe28d35d81b2a0c293aae38ca7881e1dac6be Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Oct 2024 15:22:18 +0200 Subject: [PATCH] Prevent Select listbox to grow out of viewport --- src/components/input/Select.tsx | 109 +++++++--- src/components/input/SelectContext.ts | 9 +- src/components/input/test/Select-test.js | 186 ++++++++++++++++-- .../patterns/prototype/SelectPage.tsx | 9 +- src/pattern-library/examples/select-right.tsx | 10 +- 5 files changed, 267 insertions(+), 56 deletions(-) diff --git a/src/components/input/Select.tsx b/src/components/input/Select.tsx index 21b6f806..7db136cb 100644 --- a/src/components/input/Select.tsx +++ b/src/components/input/Select.tsx @@ -26,7 +26,7 @@ import { } from '../icons'; import Checkbox from './Checkbox'; import { inputGroupStyles } from './InputGroup'; -import type { SelectValueOptions } from './SelectContext'; +import type { ListboxOverflow, SelectValueOptions } from './SelectContext'; import SelectContext from './SelectContext'; export type SelectOptionStatus = { @@ -190,10 +190,7 @@ function SelectOption({ >
({ }, )} > -
+
{optionChildren(children, { selected, disabled })}
{!multiple && ( -
+
({ {multiple && ( 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`, - }); + if (!asPopover) { + // Set styles for non-popover mode + if (shouldListboxDropUp) { + return setListboxCSSProps({ + bottom: '100%', + marginBottom: LISTBOX_TOGGLE_GAP, + }); + } + + return setListboxCSSProps({ top: '100%', marginTop: LISTBOX_TOGGLE_GAP }); } - // Set styles for non-popover mode - if (shouldListboxDropUp) { - return setListboxCSSProps({ - bottom: '100%', - marginBottom: LISTBOX_TOGGLE_GAP, - }); + const { top: bodyTop, width: bodyWidth } = + document.body.getBoundingClientRect(); + const absBodyTop = Math.abs(bodyTop); + + // The available space is: + // - left-aligned Selects: distance from left side of toggle button to right + // side of viewport + // - right-aligned Selects: distance from right side of toggle button to + // left side of viewport + const availableSpace = + (right ? buttonLeft + buttonWidth : bodyWidth - buttonLeft) - + LISTBOX_VIEWPORT_HORIZONTAL_GAP; + + let left = buttonLeft; + if (listboxWidth > availableSpace) { + // If the listbox is not going to fit the available space, let it "grow" + // in the opposite direction + left = right + ? LISTBOX_VIEWPORT_HORIZONTAL_GAP + : left - (listboxWidth - availableSpace); + } else if (right && listboxWidth > buttonWidth) { + // If a right-aligned listbox 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; } - return setListboxCSSProps({ top: '100%', marginTop: LISTBOX_TOGGLE_GAP }); + return setListboxCSSProps({ + 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`, + }); }, [asPopover, buttonRef, listboxOpen, listboxRef, right]); useLayoutEffect(() => { @@ -405,6 +436,19 @@ type BaseSelectProps = CompositeProps & { /** A callback passed to the listbox onScroll */ onListboxScroll?: JSX.HTMLAttributes['onScroll']; + + /** + * Indicates how overflowing content should be handled in the listbox. + * + * - `truncate`: Truncate the options via `text-overflow: ellipsis`, so that + * they all fit in one line. This is the default value. + * - `wrap`: Let options content wrap onto multiple lines via + * `white-space: normal` + * + * Complex content may still need to provide its own styling to handle content + * overflow. + */ + listboxOverflow?: ListboxOverflow; }; export type SelectProps = BaseSelectProps & SingleValueProps; @@ -435,6 +479,7 @@ function SelectMain({ onListboxScroll, right = false, multiple, + listboxOverflow = 'truncate', 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, /* eslint-disable-next-line no-prototype-builtins */ @@ -550,11 +595,15 @@ function SelectMain({ value: value as typeof multiple extends false ? T : T[], selectValue, multiple, + listboxOverflow, }} >
    = { multiple: true; }; -export type SelectContextType = +export type SelectContextType = ( | SingleSelectContext - | MultiSelectContext; + | MultiSelectContext +) & { + listboxOverflow: ListboxOverflow; +}; const SelectContext = createContext(null); diff --git a/src/components/input/test/Select-test.js b/src/components/input/test/Select-test.js index 6c990d92..8fcbbf0b 100644 --- a/src/components/input/test/Select-test.js +++ b/src/components/input/test/Select-test.js @@ -1,11 +1,15 @@ import { checkAccessibility, waitFor } from '@hypothesis/frontend-testing'; import { mount } from 'enzyme'; -import { MultiSelect, Select } from '../Select'; +import { + LISTBOX_VIEWPORT_HORIZONTAL_GAP, + MultiSelect, + Select, +} from '../Select'; describe('Select', () => { let wrappers; - const items = [ + const defaultItems = [ { id: '1', name: 'All students' }, { id: '2', name: 'Albert Banana' }, { id: '3', name: 'Bernard California' }, @@ -18,20 +22,27 @@ describe('Select', () => { * @param {number} [options.paddingTop] - Extra padding top for the container. * Defaults to 0. * @param {boolean} [options.optionsChildrenAsCallback] - - * Whether to render Select.Option children with callback notation. + * Whether to render `Select.Option` children with callback notation. * Used primarily to test and cover both branches. * Defaults to true. * @param {MultiSelect | Select} [options.Component] - * The actual "select" component to use. Defaults to `Select`. + * @param {Array<{ id: string; name: string }>} [options.items] - + * Alternative list of items to render. */ const createComponent = (props = {}, options = {}) => { const { paddingTop = 0, optionsChildrenAsCallback = true, Component = Select, + items = defaultItems, } = options; const container = document.createElement('div'); container.style.paddingTop = `${paddingTop}px`; + // Add horizontal paddings to the container, so that there's room for the + // listbox to grow if needed + container.style.paddingLeft = '200px'; + container.style.paddingRight = '200px'; document.body.append(container); const wrapper = mount( @@ -151,13 +162,13 @@ describe('Select', () => { const wrapper = createComponent({ onChange }); clickOption(wrapper, 3); - assert.calledWith(onChange.lastCall, items[2]); + assert.calledWith(onChange.lastCall, defaultItems[2]); clickOption(wrapper, 5); - assert.calledWith(onChange.lastCall, items[4]); + assert.calledWith(onChange.lastCall, defaultItems[4]); clickOption(wrapper, 1); - assert.calledWith(onChange.lastCall, items[0]); + assert.calledWith(onChange.lastCall, defaultItems[0]); }); it('does not change selected value when a disabled option is clicked', () => { @@ -176,13 +187,13 @@ describe('Select', () => { const wrapper = createComponent({ onChange }); pressKeyInOption(wrapper, 3, key); - assert.calledWith(onChange.lastCall, items[2]); + assert.calledWith(onChange.lastCall, defaultItems[2]); pressKeyInOption(wrapper, 5, key); - assert.calledWith(onChange.lastCall, items[4]); + assert.calledWith(onChange.lastCall, defaultItems[4]); pressKeyInOption(wrapper, 1, key); - assert.calledWith(onChange.lastCall, items[0]); + assert.calledWith(onChange.lastCall, defaultItems[0]); }); it(`does not change selected value when ${key} is pressed in a disabled option`, () => { @@ -195,7 +206,7 @@ describe('Select', () => { }); it('marks the right item as selected', () => { - const wrapper = createComponent({ value: items[2] }); + const wrapper = createComponent({ value: defaultItems[2] }); assert.isFalse(isOptionSelected(wrapper, 1)); assert.isFalse(isOptionSelected(wrapper, 2)); @@ -371,7 +382,7 @@ describe('Select', () => { getListbox(wrapper).getDOMNode().addEventListener('toggle', resolve); toggleListbox(wrapper); - // This test will timeout if the toggle event is not dispatched + // This test will time out if the toggle event is not dispatched await promise; }); }); @@ -425,12 +436,147 @@ describe('Select', () => { }); }); + 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 () => { + const name = 'name'.repeat(1000); + const wrapper = createComponent( + { buttonContent: 'Select a value' }, + { + items: ['1', '2', '3'].map(id => ({ id, name })), + }, + ); + + const listbox = await getOpenListbox(wrapper); + const { width: listboxWidth } = listbox + .getDOMNode() + .getBoundingClientRect(); + + assert.isTrue(listboxWidth < window.innerWidth); + }); + + [ + // Content is small. The listbox matches the toggle button size regardless + // the orientation. + ...[true, false].map(right => ({ + name: 'short name', + right, + getExpectedCoordinates: (wrapper, listboxDOMNode) => { + const buttonDOMNode = getToggleButton(wrapper).getDOMNode(); + const buttonLeft = buttonDOMNode.getBoundingClientRect().left; + + return { + left: buttonLeft, + right: listboxDOMNode.getBoundingClientRect().width + buttonLeft, + }; + }, + })), + + // Content is slightly longer. The listbox matches one of the toggle's + // sides but spans further to the opposite one + { + name: 'slightly longer name'.repeat(3), + right: false, + getExpectedCoordinates: (wrapper, listboxDOMNode) => { + const buttonDOMNode = getToggleButton(wrapper).getDOMNode(); + const buttonRect = buttonDOMNode.getBoundingClientRect(); + const listboxRect = listboxDOMNode.getBoundingClientRect(); + const listboxRightIncrement = listboxRect.width - buttonRect.width; + + return { + left: buttonRect.left, + right: buttonRect.right + listboxRightIncrement, + }; + }, + }, + { + name: 'slightly longer name'.repeat(3), + right: true, + getExpectedCoordinates: (wrapper, listboxDOMNode) => { + const buttonDOMNode = getToggleButton(wrapper).getDOMNode(); + const buttonRect = buttonDOMNode.getBoundingClientRect(); + const listboxRect = listboxDOMNode.getBoundingClientRect(); + const listboxLeftIncrement = listboxRect.width - buttonRect.width; + + return { + left: buttonRect.left - listboxLeftIncrement, + right: buttonRect.right, + }; + }, + }, + + // Content is very big, so listbox spans to the edge of the viewport grows + // further than the opposite side of the toggle button + { + name: 'very long name'.repeat(6), + right: false, + getExpectedCoordinates: (wrapper, listboxDOMNode) => { + const listboxRect = listboxDOMNode.getBoundingClientRect(); + const bodyWidth = document.body.getBoundingClientRect().width; + + return { + left: + bodyWidth - listboxRect.width - LISTBOX_VIEWPORT_HORIZONTAL_GAP, + right: bodyWidth - LISTBOX_VIEWPORT_HORIZONTAL_GAP, + }; + }, + }, + { + name: 'very long name'.repeat(6), + right: true, + getExpectedCoordinates: (wrapper, listboxDOMNode) => { + const listboxRect = listboxDOMNode.getBoundingClientRect(); + return { + left: LISTBOX_VIEWPORT_HORIZONTAL_GAP, + right: listboxRect.width + LISTBOX_VIEWPORT_HORIZONTAL_GAP, + }; + }, + }, + ].forEach(({ name, right, getExpectedCoordinates }) => { + it('displays listbox in expected coordinates', async () => { + const wrapper = createComponent( + { right, buttonContent: 'Select a value' }, + { + items: ['1', '2', '3'].map(id => ({ id, name })), + }, + ); + + 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; + + const expectedCoordinates = getExpectedCoordinates( + wrapper, + listboxDOMNode, + ); + + assert.equal( + listboxLeft.toFixed(0), + expectedCoordinates.left.toFixed(0), + ); + assert.equal( + listboxRight.toFixed(0), + expectedCoordinates.right.toFixed(0), + ); + }); + }); + }); + context('MultiSelect', () => { it('allows multiple items to be selected via checkboxes', () => { const onChange = sinon.stub(); const wrapper = createComponent( { - value: [items[0], items[2]], + value: [defaultItems[0], defaultItems[2]], onChange, }, { Component: MultiSelect }, @@ -440,7 +586,11 @@ describe('Select', () => { clickOptionCheckbox(wrapper, 2); // When a not-yet-selected item is clicked, it will be selected - assert.calledWith(onChange, [items[0], items[2], items[1]]); + assert.calledWith(onChange, [ + defaultItems[0], + defaultItems[2], + defaultItems[1], + ]); // After clicking a checkbox, the listbox is still open assert.isFalse(isListboxClosed(wrapper)); @@ -450,7 +600,7 @@ describe('Select', () => { const onChange = sinon.stub(); const wrapper = createComponent( { - value: [items[0], items[2]], + value: [defaultItems[0], defaultItems[2]], onChange, }, { Component: MultiSelect }, @@ -460,7 +610,7 @@ describe('Select', () => { clickOptionCheckbox(wrapper, 3); // When an already selected item is clicked, it will be de-selected - assert.calledWith(onChange, [items[0]]); + assert.calledWith(onChange, [defaultItems[0]]); }); [ @@ -471,7 +621,7 @@ describe('Select', () => { const onChange = sinon.stub(); const wrapper = createComponent( { - value: [items[0], items[2]], + value: [defaultItems[0], defaultItems[2]], onChange, }, { Component: MultiSelect }, @@ -486,7 +636,7 @@ describe('Select', () => { [ { value: [], isResetSelected: true }, - { value: [items[0], items[2]], isResetSelected: false }, + { value: [defaultItems[0], defaultItems[2]], isResetSelected: false }, ].forEach(({ value, isResetSelected }) => { it('marks reset option as selected when value is empty', () => { const wrapper = createComponent({ value }, { Component: MultiSelect }); @@ -580,7 +730,7 @@ describe('Select', () => { { buttonContent: 'Select', 'aria-label': 'Select', - value: [items[1], items[3]], + value: [defaultItems[1], defaultItems[3]], }, { optionsChildrenAsCallback: false, Component: MultiSelect }, ); diff --git a/src/pattern-library/components/patterns/prototype/SelectPage.tsx b/src/pattern-library/components/patterns/prototype/SelectPage.tsx index d7833496..cdd13b6e 100644 --- a/src/pattern-library/components/patterns/prototype/SelectPage.tsx +++ b/src/pattern-library/components/patterns/prototype/SelectPage.tsx @@ -241,7 +241,14 @@ export default function SelectPage() {
    - + ({ + ...rest, + name: `${name} (this item has very long text)`, + }))} + />
    diff --git a/src/pattern-library/examples/select-right.tsx b/src/pattern-library/examples/select-right.tsx index 9e77f5bf..5d061a33 100644 --- a/src/pattern-library/examples/select-right.tsx +++ b/src/pattern-library/examples/select-right.tsx @@ -3,11 +3,11 @@ import { useId, useState } from 'preact/hooks'; import { Select } from '../..'; const items = [ - { id: '1', name: 'All students' }, - { id: '2', name: 'Albert Banana' }, - { id: '3', name: 'Bernard California' }, - { id: '4', name: 'Cecelia Davenport' }, - { id: '5', name: 'Doris Evanescence' }, + { id: '1', name: 'All students (this item has very long text)' }, + { id: '2', name: 'Albert Banana (this item has very long text)' }, + { id: '3', name: 'Bernard California (this item has very long text)' }, + { id: '4', name: 'Cecelia Davenport (this item has very long text)' }, + { id: '5', name: 'Doris Evanescence (this item has very long text)' }, ]; type Item = (typeof items)[number];