-
Notifications
You must be signed in to change notification settings - Fork 129
fix: issue with autocomplete input blur #3690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| "@sumup-oss/circuit-ui": patch | ||
| --- | ||
|
|
||
| **AutocompleteInput:** emit `onSearch` when the search text is reset internally (on | ||
| blur-restore, clear, and immersive modal dismiss), so consumers that derive `options` | ||
| from `onSearch` no longer show a stale, previously-filtered list after the field is | ||
| dismissed without a selection. | ||
|
|
||
| **AutocompleteInput:** close the results list when focus leaves the field (e.g. via | ||
| `Tab`), not only on pointer click-outside, so keyboard users don't leave an orphaned | ||
| open listbox. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,7 +25,7 @@ While the AutocompleteInput component is accessible, it can be complex to intera | |||||
|
|
||||||
| ## How to use | ||||||
|
|
||||||
| - Use the `onSearch` prop to update the list of options as the user types. You can optionally set the `minQueryLength` prop to delay this behavior until a minimum number of characters has been typed. | ||||||
| - Use the `onSearch` prop to update the list of options as the user types. You can optionally set the `minQueryLength` prop to delay this behavior until a minimum number of characters has been typed. When the component resets its search text internally (for example after blur without a selection, clear, or closing the immersive modal), it emits `onSearch` with an empty string so consumers can restore the full option list. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| - Use the `onChange` prop to handle the selection of an option and set the input's `value` prop to the selected option. After the selection, make sure to reset the value of the `options` prop. | ||||||
| - Use the `onClear` prop to handle the clearing of the input field, and reset the input's `value` prop to reflect this change. | ||||||
| - The component can receive a flat list or a list of grouped options via the `options` prop. To ensure a consistent and user-friendly experience, keep the visual format uniform. For example, make sure all options include (or not) an image or a description. Keep labels and descriptions short and easy to scan. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -249,6 +249,23 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
| setActiveOption(undefined); | ||||||||||||||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const syncSearchOnDismiss = useCallback(() => { | ||||||||||||||||||||||||||||||||||||
| if (!Array.isArray(value)) { | ||||||||||||||||||||||||||||||||||||
| if (!value && searchText !== '') { | ||||||||||||||||||||||||||||||||||||
| changeInputValue(comboboxRef.current, ''); | ||||||||||||||||||||||||||||||||||||
| } else if (searchText !== value?.value) { | ||||||||||||||||||||||||||||||||||||
| setSearchText(value?.label ?? ''); | ||||||||||||||||||||||||||||||||||||
| if (isImmersive) { | ||||||||||||||||||||||||||||||||||||
| setPresentationFieldValue(value?.label ?? ''); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } else if (searchText) { | ||||||||||||||||||||||||||||||||||||
| changeInputValue(comboboxRef.current, ''); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+252
to
+264
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with the suggestions below, this can be simplified
Suggested change
|
||||||||||||||||||||||||||||||||||||
| onSearch?.(''); | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that whenever we close the suggestion box, we always call |
||||||||||||||||||||||||||||||||||||
| closeResults(); | ||||||||||||||||||||||||||||||||||||
| }, [value, searchText, isImmersive, onSearch, closeResults]); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const debouncedOnSearch = useMemo( | ||||||||||||||||||||||||||||||||||||
| () => | ||||||||||||||||||||||||||||||||||||
| debounce( | ||||||||||||||||||||||||||||||||||||
|
|
@@ -273,9 +290,10 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
| const onComboboxClear = useCallback( | ||||||||||||||||||||||||||||||||||||
| (event: ClickEvent) => { | ||||||||||||||||||||||||||||||||||||
| changeInputValue(comboboxRef.current, ''); | ||||||||||||||||||||||||||||||||||||
| onSearch?.(''); | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||||||||||||||||||||||||||||||||
| onClear?.(event); | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| [onClear], | ||||||||||||||||||||||||||||||||||||
| [onClear, onSearch], | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const onPresentationFieldClear = useCallback( | ||||||||||||||||||||||||||||||||||||
|
|
@@ -284,9 +302,10 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
| setSearchText(''); | ||||||||||||||||||||||||||||||||||||
| changeInputValue(presentationFieldRef.current, ''); | ||||||||||||||||||||||||||||||||||||
| setIsOpen(true); | ||||||||||||||||||||||||||||||||||||
| onSearch?.(''); | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment about |
||||||||||||||||||||||||||||||||||||
| onClear?.(event); | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| [onClear], | ||||||||||||||||||||||||||||||||||||
| [onClear, onSearch], | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const onPresentationFieldKeyDown = useCallback(() => { | ||||||||||||||||||||||||||||||||||||
|
|
@@ -449,7 +468,7 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
| }, [closeResults, isImmersive]); | ||||||||||||||||||||||||||||||||||||
| useClickOutside([inputWrapperRef, refs.floating], handleClickOutside); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| useEscapeKey(closeResults, isOpen); | ||||||||||||||||||||||||||||||||||||
| useEscapeKey(syncSearchOnDismiss, isOpen); | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||
| // if readOnly or disabled props become truthy, close the list box | ||||||||||||||||||||||||||||||||||||
|
|
@@ -504,15 +523,18 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const restoreValue: FocusEventHandler<HTMLInputElement> = useCallback( | ||||||||||||||||||||||||||||||||||||
| (event) => { | ||||||||||||||||||||||||||||||||||||
| if (!Array.isArray(value) && searchText !== value?.value) { | ||||||||||||||||||||||||||||||||||||
| setSearchText(value?.label ?? ''); | ||||||||||||||||||||||||||||||||||||
| if (isImmersive) { | ||||||||||||||||||||||||||||||||||||
| setPresentationFieldValue(value?.label ?? ''); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| const nextTarget = event.relatedTarget as Node | null; | ||||||||||||||||||||||||||||||||||||
| const movingInside = | ||||||||||||||||||||||||||||||||||||
| inputWrapperRef.current?.contains(nextTarget) || | ||||||||||||||||||||||||||||||||||||
| refs.floating.current?.contains(nextTarget) || | ||||||||||||||||||||||||||||||||||||
| resultsRef.current?.contains(nextTarget); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (!movingInside) { | ||||||||||||||||||||||||||||||||||||
| syncSearchOnDismiss(); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for catching this issue, @nicosommi 👏
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call on reusing changeInputValue for the clear-to-empty path — adopted that in syncSearchOnDismiss for !value && searchText !== '' (and multi-select filter reset), same as onComboboxClear. Kept setSearchText for the preselected-value restore path because firing a change to the label would debounce onSearch(label) and leave client-filtered options stale again. Still emit onSearch('') explicitly so consumers get the full list on reopen. movingInside + syncSearchOnDismiss on Escape/modal close remain for Tab-close and immersive dismiss. |
||||||||||||||||||||||||||||||||||||
| props.onBlur?.(event); | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| [value, searchText, isImmersive, props.onBlur], | ||||||||||||||||||||||||||||||||||||
| [syncSearchOnDismiss, refs.floating, props.onBlur], | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const comboboxProps = { | ||||||||||||||||||||||||||||||||||||
|
|
@@ -573,7 +595,7 @@ export const AutocompleteInput = forwardRef< | |||||||||||||||||||||||||||||||||||
| open={isOpen} | ||||||||||||||||||||||||||||||||||||
| className={classes.modal} | ||||||||||||||||||||||||||||||||||||
| contentClassName={classes['modal-content']} | ||||||||||||||||||||||||||||||||||||
| onClose={closeResults} | ||||||||||||||||||||||||||||||||||||
| onClose={syncSearchOnDismiss} | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||
| <div ref={inputWrapperRef} className={classes['modal-input']}> | ||||||||||||||||||||||||||||||||||||
| <ComboboxInput | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is best to create one changeset file per change. It helps with the visibility and discovery of the different changes.
In a separate changeset file:
Updated the AutocompleteInput focus management to close the suggestion box whenever the focus leaves the elements of the component (combobox or suggestion box).