diff --git a/.changeset/fix-select-search.md b/.changeset/fix-select-search.md new file mode 100644 index 000000000..6e2ca8869 --- /dev/null +++ b/.changeset/fix-select-search.md @@ -0,0 +1,12 @@ +--- +"@clickhouse/click-ui": patch +--- + +Fix a few bugs by changing the way Select works with its options: instead of imperative rebuilding of several service entities, +derive them in-flow, “you might not need an effect”. + +Bugs fixed: +- search now always works in Selects and matches any text that is rendered in items, including options that arrive while the menu is open +- disabled items can no longer be selected via keyboard navigation + +Note: filtered-out items now stay mounted (hidden) instead of unmounting, so their rendered text stays available to search. diff --git a/src/components/CheckboxMultiSelect/CheckboxMultiSelect.test.tsx b/src/components/CheckboxMultiSelect/CheckboxMultiSelect.test.tsx index 2e81fec69..466942979 100644 --- a/src/components/CheckboxMultiSelect/CheckboxMultiSelect.test.tsx +++ b/src/components/CheckboxMultiSelect/CheckboxMultiSelect.test.tsx @@ -258,11 +258,11 @@ describe('CheckboxCheckboxMultiSelect', () => { const selectTrigger = getByTestId('select-trigger'); selectTrigger && fireEvent.click(selectTrigger); - expect(queryByText('Content0')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).toBeInTheDocument(); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content3')).toBeInTheDocument(); - expect(queryByText('Content4')).toBeInTheDocument(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); }); it('filters items by search text', () => { @@ -275,16 +275,16 @@ describe('CheckboxCheckboxMultiSelect', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).toBeInTheDocument(); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content3')).toBeInTheDocument(); - expect(queryByText('Content4')).toBeInTheDocument(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content2' }, }); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).not.toBeInTheDocument(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -299,16 +299,16 @@ describe('CheckboxCheckboxMultiSelect', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).toBeInTheDocument(); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content3')).toBeInTheDocument(); - expect(queryByText('Content4')).toBeInTheDocument(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content2' }, }); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).not.toBeInTheDocument(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -325,16 +325,16 @@ describe('CheckboxCheckboxMultiSelect', () => { fireEvent.change(selectInput, { target: { value: 'content2' }, }); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).not.toBeInTheDocument(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); fireEvent.click(getByTestId('select-search-close')); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).toBeInTheDocument(); - expect(queryByText('Content1 long text content')).toBeInTheDocument(); - expect(queryByText('Content2')).toBeInTheDocument(); - expect(queryByText('Content3')).toBeInTheDocument(); - expect(queryByText('Content4')).toBeInTheDocument(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); expect(document.activeElement).toBe(selectInput); }); @@ -350,8 +350,8 @@ describe('CheckboxCheckboxMultiSelect', () => { fireEvent.change(getByTestId('select-search-input'), { target: { value: 'nodata' }, }); - expect(queryByText('Content2')).not.toBeInTheDocument(); - expect(queryByText('Content1 long text content')).not.toBeInTheDocument(); + expect(queryByText('Content2')).not.toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); const btn = queryByText(/No Options found/i); expect(btn).toBeInTheDocument(); diff --git a/src/components/MultiSelect/MultiSelect.test.tsx b/src/components/MultiSelect/MultiSelect.test.tsx index 55e46874f..c8d5add7e 100644 --- a/src/components/MultiSelect/MultiSelect.test.tsx +++ b/src/components/MultiSelect/MultiSelect.test.tsx @@ -223,11 +223,11 @@ describe('MultiSelect', () => { expect(selectTrigger).not.toBeNull(); selectTrigger && fireEvent.click(selectTrigger); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); }); it('filter by text', () => { @@ -239,16 +239,16 @@ describe('MultiSelect', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content2' }, }); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -262,16 +262,16 @@ describe('MultiSelect', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content2' }, }); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -287,16 +287,16 @@ describe('MultiSelect', () => { fireEvent.change(selectInput, { target: { value: 'content2' }, }); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); fireEvent.click(getByTestId('select-search-close')); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); expect(document.activeElement).toBe(selectInput); }); it('on no options available show no data', () => { @@ -310,8 +310,8 @@ describe('MultiSelect', () => { fireEvent.change(getByTestId('select-search-input'), { target: { value: 'nodata' }, }); - expect(queryByText('Content2')).toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).not.toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); const btn = queryByText(/No Options found/i); expect(btn).not.toBeNull(); @@ -333,8 +333,8 @@ describe('MultiSelect', () => { fireEvent.change(getByTestId('select-search-input'), { target: { value: 'nodata' }, }); - expect(queryByText('Content2')).toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).not.toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); const btn = queryByText(/No Field found/i); expect(btn).not.toBeNull(); diff --git a/src/components/Select/Select.search.test.tsx b/src/components/Select/Select.search.test.tsx new file mode 100644 index 000000000..b0819516e --- /dev/null +++ b/src/components/Select/Select.search.test.tsx @@ -0,0 +1,270 @@ +import { fireEvent } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { Select } from '@/components/Select'; +import { renderCUI } from '@/utils/test-utils'; + +/** + * Search must match what the user actually sees. The text is read from the + * rendered DOM, so it works regardless of how an item produces its text — a + * child component, a number, or the `label` prop — none of which a static walk + * of the React tree could recover. + */ + +// A row whose visible text comes from a prop, not from string children. +const ServiceRow = ({ name }: { name: string }) => {name}; + +const open = (getByTestId: (id: string) => HTMLElement) => + fireEvent.click(getByTestId('select-trigger')); + +const type = (getByTestId: (id: string) => HTMLElement, query: string) => + fireEvent.change(getByTestId('select-search-input'), { target: { value: query } }); + +const renderChildren = (children: ReactNode) => + renderCUI( + + ); + +describe('InternalSelect search', () => { + beforeAll(() => { + window.HTMLElement.prototype.scrollIntoView = vi.fn(); + global.ResizeObserver = vi.fn().mockImplementation(() => ({ + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + })); + }); + + describe('children API', () => { + it('matches plain string children', () => { + const { getByTestId, queryByText } = renderChildren( + <> + Apple + Banana + + ); + open(getByTestId); + type(getByTestId, 'ban'); + expect(queryByText('Banana')).toBeVisible(); + expect(queryByText('Apple')).not.toBeVisible(); + }); + + it('matches text supplied via the `label` prop', () => { + const { getByTestId, queryByText } = renderChildren( + <> + + + + ); + open(getByTestId); + type(getByTestId, 'ban'); + expect(queryByText('Banana')).toBeVisible(); + expect(queryByText('Apple')).not.toBeVisible(); + }); + + it('matches numeric children', () => { + const { getByTestId, queryByText } = renderChildren( + <> + {2024} + {2025} + + ); + open(getByTestId); + type(getByTestId, '2025'); + expect(queryByText('2025')).toBeVisible(); + expect(queryByText('2024')).not.toBeVisible(); + }); + + it('matches text rendered by a custom child component (the reported case)', () => { + const { getByTestId, queryByText } = renderChildren( + <> + + + + + + + + ); + open(getByTestId); + type(getByTestId, 'bill'); + expect(queryByText('Billing')).toBeVisible(); + expect(queryByText('Analytics')).not.toBeVisible(); + }); + + it('restores every item when the search is cleared', () => { + const { getByTestId, queryByText } = renderChildren( + <> + + + + + + + + ); + open(getByTestId); + type(getByTestId, 'bill'); + expect(queryByText('Analytics')).not.toBeVisible(); + type(getByTestId, ''); + expect(queryByText('Analytics')).toBeVisible(); + expect(queryByText('Billing')).toBeVisible(); + }); + }); + + describe('options API', () => { + it('matches a label rendered by a custom component', () => { + const { getByTestId, queryByText } = renderCUI( + + ); + open(getByTestId); + rerender( + + ); + open(getByTestId); + type(getByTestId, 'yellow'); + expect(queryByText('Banana')).toBeVisible(); + expect(queryByText('Apple')).not.toBeVisible(); + }); + }); + + describe('group heading', () => { + it('reveals a group’s items when the search matches its heading', () => { + const { getByTestId, queryByText } = renderChildren( + <> + + Apple + Banana + + + Carrot + + + ); + open(getByTestId); + type(getByTestId, 'fruit'); + expect(queryByText('Apple')).toBeVisible(); + expect(queryByText('Banana')).toBeVisible(); + expect(queryByText('Carrot')).not.toBeVisible(); + }); + }); + + describe('keyboard navigation seed', () => { + it('highlights the first item on open and selects it with Enter', () => { + const onSelect = vi.fn(); + const { getByTestId } = renderCUI( + + ); + open(getByTestId); + const input = getByTestId('select-search-input'); + // seed highlights "a"; one ArrowDown moves to "b" + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'Enter' }); + expect(onSelect).toHaveBeenCalledWith('b', undefined, expect.anything()); + }); + + it('skips disabled options while navigating (options API)', () => { + const onSelect = vi.fn(); + const { getByTestId } = renderCUI( + + Apple + Banana + Cherry + + ); + open(getByTestId); + const input = getByTestId('select-search-input'); + fireEvent.keyDown(input, { key: 'ArrowDown' }); // highlight "b" + fireEvent.change(input, { target: { value: 'cher' } }); // "b" no longer visible + fireEvent.keyDown(input, { key: 'Enter' }); + expect(onSelect).toHaveBeenCalledWith('c', undefined, expect.anything()); + }); + }); +}); diff --git a/src/components/Select/Select.test.tsx b/src/components/Select/Select.test.tsx index 07367628a..2dcd6826b 100644 --- a/src/components/Select/Select.test.tsx +++ b/src/components/Select/Select.test.tsx @@ -261,11 +261,11 @@ describe('Select', () => { expect(selectTrigger).not.toBeNull(); selectTrigger && fireEvent.click(selectTrigger); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); }); it('filter by text', () => { @@ -277,16 +277,16 @@ describe('Select', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content3' }, }); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -300,16 +300,16 @@ describe('Select', () => { selectTrigger && fireEvent.click(selectTrigger); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); fireEvent.change(getByTestId('select-search-input'), { target: { value: 'content3' }, }); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); }); @@ -325,16 +325,16 @@ describe('Select', () => { fireEvent.change(selectInput, { target: { value: 'content3' }, }); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); fireEvent.click(getByTestId('select-search-close')); expect(queryByText('Group label')).toBeVisible(); - expect(queryByText('Content0')).not.toBeNull(); - expect(queryByText('Content1 long text content')).not.toBeNull(); - expect(queryByText('Content2')).not.toBeNull(); - expect(queryByText('Content3')).not.toBeNull(); - expect(queryByText('Content4')).not.toBeNull(); + expect(queryByText('Content0')).toBeVisible(); + expect(queryByText('Content1 long text content')).toBeVisible(); + expect(queryByText('Content2')).toBeVisible(); + expect(queryByText('Content3')).toBeVisible(); + expect(queryByText('Content4')).toBeVisible(); expect(document.activeElement).toBe(selectInput); }); it('on no options available show no data', () => { @@ -348,8 +348,8 @@ describe('Select', () => { fireEvent.change(getByTestId('select-search-input'), { target: { value: 'nodata' }, }); - expect(queryByText('Content2')).toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).not.toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); const btn = queryByText(/No Options found/i); expect(btn).not.toBeNull(); @@ -371,8 +371,8 @@ describe('Select', () => { fireEvent.change(getByTestId('select-search-input'), { target: { value: 'nodata' }, }); - expect(queryByText('Content2')).toBeNull(); - expect(queryByText('Content1 long text content')).toBeNull(); + expect(queryByText('Content2')).not.toBeVisible(); + expect(queryByText('Content1 long text content')).not.toBeVisible(); expect(queryByText('Group label')).not.toBeVisible(); const btn = queryByText(/No Field found/i); expect(btn).not.toBeNull(); diff --git a/src/components/Select/common/InternalSelect.tsx b/src/components/Select/common/InternalSelect.tsx index 4a9e823d8..fa4f26658 100644 --- a/src/components/Select/common/InternalSelect.tsx +++ b/src/components/Select/common/InternalSelect.tsx @@ -8,7 +8,6 @@ import { forwardRef, isValidElement, useCallback, - useEffect, useId, useMemo, useRef, @@ -18,9 +17,7 @@ import { NoAvailableOptionsFactoryProps, SelectContainerProps, SelectGroupProps, - SelectItemObject, SelectItemProps, - SelectOptionListItem, } from './types'; import { Error, FormElementContainer, FormRoot } from '@/components/FormContainer'; import { Portal } from '@radix-ui/react-popover'; @@ -60,13 +57,15 @@ import { GenericMenuItem } from '@/components/GenericMenu'; import { IconWrapper } from '@/components/IconWrapper'; import { useInputModality } from '@/hooks/internal'; import { cn } from '@/lib/cva'; -import { getTextFromNodes } from '@/lib/getTextFromNodes'; import { useResolvedPortalContainer } from '@/providers/PortalContext'; -type CallbackProps = SelectItemObject & { - nodeProps: SelectItemProps; +type NormalizedOption = { + value: string; + disabled?: boolean; }; +type RegisterItem = (value: string, nodeProps: SelectItemProps) => void; + interface NoOptionsDisplayProps { allowCreateOption: boolean; search: string; @@ -152,40 +151,21 @@ const NoOptionsDisplay: React.FC = ({ ); }; -const childrenToComboboxItemArray = ( +const childrenToNormalizedOptions = ( children: ReactNode, - callback: (props: CallbackProps) => void, - heading?: string -): SelectItemObject[] => { + register: RegisterItem +): NormalizedOption[] => { return Children.toArray(children).flatMap(child => { if (isValidElement(child) && child && typeof child === 'object') { const type = child.type as FunctionComponent; if (type.displayName === 'Select.Group') { - const groupChildren = child.props.children; - return childrenToComboboxItemArray( - groupChildren, - callback, - getTextFromNodes(child.props.heading).toLowerCase() - ); + return childrenToNormalizedOptions(child.props.children, register); } else if (type.displayName === 'Select.Item') { - const title = getTextFromNodes(child).toLowerCase(); - const value = child.props.value; - const disabled = child.props.disabled; - callback({ - disabled, - value, - title, - heading, - nodeProps: child.props, - }); - return { - disabled, - value, - title, - heading, - }; + const { value, disabled } = child.props; + register(value, child.props); + return { value, disabled }; } else if ('props' in child && child.props.children) { - return childrenToComboboxItemArray(child.props.children, callback, heading); + return childrenToNormalizedOptions(child.props.children, register); } } return []; @@ -227,108 +207,81 @@ export const InternalSelect = ({ const defaultId = useId(); const [search, setSearch] = useState(''); const [highlighted, setHighlighted] = useState(); - const visibleList = useRef([]); - const navigatable = useRef([]); + // value -> lowercased rendered text (item + its group heading). + const [searchSource, setSearchSource] = useState>(() => new Map()); const valueNode = useRef>(new Map()); - const [isInitialized, setInitialized] = useState(false); - const [list, setList] = useState([]); - const updateElements = useCallback( - ({ disabled, value, title, heading, nodeProps }: CallbackProps) => { - if (title.includes(search) || heading?.includes(search)) { - visibleList.current.push(value); - if (!disabled) { - navigatable.current.push(value); + + const registerValueNode = useCallback((value, nodeProps) => { + valueNode.current.set(value, nodeProps); + }, []); + + const normalizedOptions = useMemo(() => { + if (options) { + return options.flatMap(option => { + if ('options' in option) { + return (option.options ?? []).map(item => { + registerValueNode(item.value, item); + return { value: item.value, disabled: item.disabled }; + }); } + registerValueNode(option.value, option); + return { value: option.value, disabled: option.disabled }; + }); + } else if (children) { + return childrenToNormalizedOptions(children, registerValueNode); + } + + return []; + }, [children, options, registerValueNode]); + + // Radix mounts the popover content a commit after `open` flips, so the item + // text is read once the list node itself commits. + const listRef = useCallback( + (node: HTMLDivElement | null) => { + if (!node) { + return; } - valueNode.current.set(value, nodeProps); - }, - [search] - ); - const onUpdateSearch = useCallback( - (search: string) => { - setSearch(search); - let hasHighlightedValue = false; - const visibleItemsList: string[] = []; - const navigatableList: string[] = []; - const searchLowerCase = search.toLowerCase(); - list.forEach(item => { - if ( - item.title.includes(searchLowerCase) || - item.heading?.includes(searchLowerCase) - ) { - if (item.value === highlighted) { - hasHighlightedValue = true; - } - visibleItemsList.push(item.value); - if (!item.disabled) { - navigatableList.push(item.value); - } + const next = new Map(); + node.querySelectorAll('[cui-select-item][data-value]').forEach(el => { + const value = el.getAttribute('data-value'); + if (value === null) { + return; } + const group = el.closest('[cui-select-group]'); + const heading = + group?.querySelector('[cui-select-group-name]')?.textContent ?? ''; + next.set(value, `${el.textContent ?? ''} ${heading}`.toLowerCase()); }); - navigatable.current = navigatableList; - visibleList.current = visibleItemsList; - if (!hasHighlightedValue) { - setHighlighted(navigatableList[0] ?? null); - } + setSearchSource(next); }, - [highlighted, list] + // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on normalizedOptions to re-read DOM if options change + [normalizedOptions] ); - const updateList = useCallback( - (children?: ReactNode, options?: SelectOptionListItem[]) => { - const lowerCasedSearch = search.toLowerCase(); - if (options) { - setList( - options.flatMap(option => { - if ('options' in option) { - const heading = getTextFromNodes(option.heading).toLowerCase(); - return (option.options ?? []).map(item => { - valueNode.current.set(item.value, item); - const title = getTextFromNodes(item.label).toLowerCase(); - if ( - title.includes(lowerCasedSearch) || - heading?.includes(lowerCasedSearch) - ) { - visibleList.current.push(item.value); - if (!disabled) { - navigatable.current.push(item.value); - } - } - return { - heading, - disabled: item.disabled, - value: item.value, - title, - }; - }); - } else { - valueNode.current.set(option.value, option); - const title = getTextFromNodes(option.label).toLowerCase(); - if (title.includes(lowerCasedSearch)) { - visibleList.current.push(option.value); - if (!disabled) { - navigatable.current.push(option.value); - } - } - return { - disabled: option.disabled, - value: option.value, - title: getTextFromNodes(option.label), - }; - } - }) - ); - } else if (children) { - setList(childrenToComboboxItemArray(children, updateElements)); - } - }, - [disabled, search, updateElements] + const matchedOptions = useMemo(() => { + if (search === '') { + return normalizedOptions; + } + const searchLowerCase = search.toLowerCase(); + return normalizedOptions.filter(item => + (searchSource.get(item.value) ?? '').includes(searchLowerCase) + ); + }, [search, normalizedOptions, searchSource]); + + const visibleValues = useMemo( + () => new Set(matchedOptions.map(o => o.value)), + [matchedOptions] + ); + const navigableValues = useMemo( + () => matchedOptions.filter(o => !o.disabled).map(o => o.value), + [matchedOptions] ); - useEffect(() => { - updateList(children, options); - setInitialized(true); - }, [children, options, updateList]); + // `highlighted` is the user's last pointer; honor it only while it stays navigable. + const effectiveHighlight = + highlighted && navigableValues.includes(highlighted) + ? highlighted + : navigableValues[0]; const inputRef = useRef(null); const inputModalityProps = useInputModality(); @@ -339,42 +292,42 @@ export const InternalSelect = ({ }; const clearSearch = () => { - onUpdateSearch(''); + setSearch(''); }; const onKeyDown = (e: KeyboardEvent) => { if (!e.defaultPrevented) { if (e.key === 'Enter') { e.preventDefault(); - if (highlighted) { - onSelect(highlighted, undefined, e); - } else if (visibleList.current.length === 0 && allowCreateOption) { + if (effectiveHighlight) { + onSelect(effectiveHighlight, undefined, e); + } else if (matchedOptions.length === 0 && allowCreateOption) { onSelect(search, 'custom', e); } } else if (['ArrowUp', 'ArrowDown', 'Home', 'End'].includes(e.key)) { e.preventDefault(); - let nextHighlightedValue = highlighted; - const highlightedIndex = navigatable.current.findIndex( - value => value === highlighted + let nextHighlightedValue = effectiveHighlight; + const highlightedIndex = navigableValues.findIndex( + value => value === effectiveHighlight ); if (e.key === 'ArrowUp') { if (highlightedIndex === 0) { - nextHighlightedValue = navigatable.current[navigatable.current.length - 1]; + nextHighlightedValue = navigableValues[navigableValues.length - 1]; } else { - nextHighlightedValue = navigatable.current[highlightedIndex - 1]; + nextHighlightedValue = navigableValues[highlightedIndex - 1]; } } else if (e.key === 'ArrowDown') { e.preventDefault(); - if (highlightedIndex === navigatable.current.length - 1) { - nextHighlightedValue = navigatable.current[0]; + if (highlightedIndex === navigableValues.length - 1) { + nextHighlightedValue = navigableValues[0]; } else { - nextHighlightedValue = navigatable.current[highlightedIndex + 1]; + nextHighlightedValue = navigableValues[highlightedIndex + 1]; } } else if (e.key === 'End') { e.preventDefault(); - nextHighlightedValue = navigatable.current[navigatable.current.length - 1]; + nextHighlightedValue = navigableValues[navigableValues.length - 1]; } else if (e.key === 'Home') { - nextHighlightedValue = navigatable.current[0]; + nextHighlightedValue = navigableValues[0]; e.preventDefault(); } setHighlighted(nextHighlightedValue); @@ -382,22 +335,20 @@ export const InternalSelect = ({ } }; const isHidden = useCallback( - (value?: string) => { - return !visibleList.current.includes(value ?? ''); - }, - [visibleList] + (value?: string) => !visibleValues.has(value ?? ''), + [visibleValues] ); const optionContextValue = useMemo(() => { return { search, updateHighlighted: setHighlighted, - highlighted, + highlighted: effectiveHighlight, isHidden, onSelect, selectedValues, }; - }, [search, highlighted, isHidden, onSelect, selectedValues]); + }, [search, effectiveHighlight, isHidden, onSelect, selectedValues]); const onCreateOption = (e: MouseEvent) => { e.preventDefault(); @@ -427,31 +378,29 @@ export const InternalSelect = ({ data-testid="select-trigger" {...triggerProps} > - {isInitialized && ( - - {selectedValues.length === 0 ? ( - placeholder - ) : multiple ? ( - - ) : ( - - )} - - )} + + {selectedValues.length === 0 ? ( + placeholder + ) : multiple ? ( + + ) : ( + + )} + null} > - {list.map(item => ( + {normalizedOptions.map(item => (