From dacd40dc485497d9466fb66a0e64866bea2effb9 Mon Sep 17 00:00:00 2001 From: adamviktora Date: Tue, 23 Apr 2024 17:51:34 +0200 Subject: [PATCH] fix(SelectTypeahead): address PR review - new changes were done also based on SelectTypeahead example updates (https://github.com/patternfly/patternfly-react/pull/10207) --- .../src/components/Select/SelectTypeahead.tsx | 148 ++++++++++-------- 1 file changed, 86 insertions(+), 62 deletions(-) diff --git a/packages/react-templates/src/components/Select/SelectTypeahead.tsx b/packages/react-templates/src/components/Select/SelectTypeahead.tsx index 4be92446fd0..f880bad1ac2 100644 --- a/packages/react-templates/src/components/Select/SelectTypeahead.tsx +++ b/packages/react-templates/src/components/Select/SelectTypeahead.tsx @@ -24,9 +24,12 @@ export interface SelectTypeaheadProps { /** @hide Forwarded ref */ innerRef?: React.Ref; /** Initial options of the select. */ - initialOptions?: SelectTypeaheadOption[]; + initialOptions: SelectTypeaheadOption[]; /** Callback triggered on selection. */ - onSelect?: (_event: React.MouseEvent, selection: string | number) => void; + onSelect?: ( + _event: React.MouseEvent | React.KeyboardEvent, + selection: string | number + ) => void; /** Callback triggered when the select opens or closes. */ onToggle?: (nextIsOpen: boolean) => void; /** Callback triggered when the text in the input field changes. */ @@ -56,7 +59,7 @@ export const SelectTypeaheadBase: React.FunctionComponent const [filterValue, setFilterValue] = React.useState(''); const [selectOptions, setSelectOptions] = React.useState(initialOptions); const [focusedItemIndex, setFocusedItemIndex] = React.useState(null); - const [activeItem, setActiveItem] = React.useState(null); + const [activeItemId, setActiveItemId] = React.useState(null); const textInputRef = React.useRef(); const NO_RESULTS = 'no results'; @@ -80,7 +83,6 @@ export const SelectTypeaheadBase: React.FunctionComponent value: NO_RESULTS } ]; - resetActiveAndFocusedItem(); } // Open the menu when the input value changes and the new value is not empty @@ -92,21 +94,17 @@ export const SelectTypeaheadBase: React.FunctionComponent setSelectOptions(newSelectOptions); }, [filterValue]); - React.useEffect(() => { - if (isOpen && selectOptions.length && selectOptions[0].value !== NO_RESULTS) { - setActiveAndFocusedItem(0); - } - }, [isOpen, filterValue]); + const createItemId = (value: string | number) => `select-typeahead-${String(value).replace(' ', '-')}`; const setActiveAndFocusedItem = (itemIndex: number) => { setFocusedItemIndex(itemIndex); - const focusedItem = selectOptions.filter((option) => !option.isDisabled)[itemIndex]; - setActiveItem(`select-typeahead-${String(focusedItem.value).replace(' ', '-')}`); + const focusedItem = selectOptions[itemIndex]; + setActiveItemId(createItemId(focusedItem.value)); }; const resetActiveAndFocusedItem = () => { setFocusedItemIndex(null); - setActiveItem(null); + setActiveItemId(null); }; const closeMenu = () => { @@ -122,16 +120,24 @@ export const SelectTypeaheadBase: React.FunctionComponent } }; - const _onSelect = (_event: React.MouseEvent | undefined, value: string | number | undefined) => { - onSelect && onSelect(_event, value); + const selectOption = ( + _event: React.MouseEvent | React.KeyboardEvent | undefined, + option: SelectTypeaheadOption + ) => { + onSelect && onSelect(_event, option.value); + + setInputValue(String(option.content)); + setFilterValue(''); + setSelected(String(option.value)); + + closeMenu(); + }; + const _onSelect = (_event: React.MouseEvent | undefined, value: string | number | undefined) => { if (value && value !== NO_RESULTS) { - const inputText = selectOptions.find((option) => option.value === value).content; - setInputValue(String(inputText)); - setFilterValue(''); - setSelected(String(value)); + const optionToSelect = selectOptions.find((option) => option.value === value); + selectOption(_event, optionToSelect); } - closeMenu(); }; const onTextInputChange = (_event: React.FormEvent, value: string) => { @@ -139,53 +145,73 @@ export const SelectTypeaheadBase: React.FunctionComponent onInputChange && onInputChange(value); setFilterValue(value); + resetActiveAndFocusedItem(); + if (value !== selected) { setSelected(''); } }; const handleMenuArrowKeys = (key: string) => { - let indexToFocus; + let indexToFocus = 0; + + if (!isOpen) { + setIsOpen(true); + } + + if (selectOptions.every((option) => option.isDisabled)) { + return; + } - if (isOpen) { - if (key === 'ArrowUp') { - // When no index is set or at the first index, focus to the last, otherwise decrement focus index - if (focusedItemIndex === null || focusedItemIndex === 0) { + if (key === 'ArrowUp') { + // When no index is set or at the first index, focus to the last, otherwise decrement focus index + if (focusedItemIndex === null || focusedItemIndex === 0) { + indexToFocus = selectOptions.length - 1; + } else { + indexToFocus = focusedItemIndex - 1; + } + + // Skip disabled options + while (selectOptions[indexToFocus].isDisabled) { + indexToFocus--; + if (indexToFocus === -1) { indexToFocus = selectOptions.length - 1; - } else { - indexToFocus = focusedItemIndex - 1; } } + } - if (key === 'ArrowDown') { - // When no index is set or at the last index, focus to the first, otherwise increment focus index - if (focusedItemIndex === null || focusedItemIndex === selectOptions.length - 1) { + if (key === 'ArrowDown') { + // When no index is set or at the last index, focus to the first, otherwise increment focus index + if (focusedItemIndex === null || focusedItemIndex === selectOptions.length - 1) { + indexToFocus = 0; + } else { + indexToFocus = focusedItemIndex + 1; + } + + // Skip disabled options + while (selectOptions[indexToFocus].isDisabled) { + indexToFocus++; + if (indexToFocus === selectOptions.length) { indexToFocus = 0; - } else { - indexToFocus = focusedItemIndex + 1; } } - - setActiveAndFocusedItem(indexToFocus); } + + setActiveAndFocusedItem(indexToFocus); }; const onInputKeyDown = (event: React.KeyboardEvent) => { - const enabledMenuItems = selectOptions.filter((option) => !option.isDisabled); - const [firstMenuItem] = enabledMenuItems; - const focusedItem = focusedItemIndex ? enabledMenuItems[focusedItemIndex] : firstMenuItem; + const focusedItem = focusedItemIndex !== null ? selectOptions[focusedItemIndex] : null; switch (event.key) { - // Select the first available option case 'Enter': - if (isOpen && focusedItem.value !== NO_RESULTS) { - setInputValue(String(focusedItem.content)); - setFilterValue(''); - setSelected(String(focusedItem.value)); + if (isOpen && focusedItem && focusedItem.value !== NO_RESULTS && !focusedItem.isAriaDisabled) { + selectOption(event, focusedItem); } - setIsOpen((prevIsOpen) => !prevIsOpen); - resetActiveAndFocusedItem(); + if (!isOpen) { + setIsOpen(true); + } break; case 'ArrowUp': @@ -222,28 +248,27 @@ export const SelectTypeaheadBase: React.FunctionComponent autoComplete="off" innerRef={textInputRef} placeholder={placeholder} - {...(activeItem && { 'aria-activedescendant': activeItem })} + {...(activeItemId && { 'aria-activedescendant': activeItemId })} role="combobox" isExpanded={isOpen} aria-controls="select-typeahead-listbox" /> - - {!!inputValue && ( - - )} + + @@ -273,8 +298,7 @@ export const SelectTypeaheadBase: React.FunctionComponent key={value} value={value} isFocused={focusedItemIndex === index} - onMouseEnter={() => setActiveAndFocusedItem(index)} - id={`select-typeahead-${String(value).replace(' ', '-')}`} + id={createItemId(value)} > {content}