fix: issue with autocomplete input blur#3690
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Size Change: +145 B (+0.02%) Total Size: 829 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3690 +/- ##
==========================================
- Coverage 93.69% 93.64% -0.05%
==========================================
Files 199 199
Lines 4486 4502 +16
Branches 1763 1775 +12
==========================================
+ Hits 4203 4216 +13
- Misses 256 258 +2
- Partials 27 28 +1
🚀 New features to boost your workflow:
|
|
|
||
| if (!movingInside) { | ||
| syncSearchOnDismiss(); | ||
| } |
There was a problem hiding this comment.
Thank you for catching this issue, @nicosommi 👏
I believe there could be a simpler way to address it. If we fire a change event here (like we do in the onComboboxClear callback) I believe we could achieve the same thing:
| } | |
| } | |
| if (!value && searchText !== '') { | |
| changeInputValue(comboboxRef.current, ''); | |
| } |
There was a problem hiding this comment.
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.
| } else if (searchText) { | ||
| changeInputValue(comboboxRef.current, ''); | ||
| } | ||
| onSearch?.(''); |
There was a problem hiding this comment.
I noticed that whenever we close the suggestion box, we always call onSearch('') , which actually makes sense. How about moving it into the closeResults method ?
| const onComboboxClear = useCallback( | ||
| (event: ClickEvent) => { | ||
| changeInputValue(comboboxRef.current, ''); | ||
| onSearch?.(''); |
There was a problem hiding this comment.
I think onClear is enough to allow consumers to do the necessary (reset suggestions) I think calling onSearch here is superfluous
| setSearchText(''); | ||
| changeInputValue(presentationFieldRef.current, ''); | ||
| setIsOpen(true); | ||
| onSearch?.(''); |
| useClickOutside([inputWrapperRef, refs.floating], handleClickOutside); | ||
|
|
||
| useEscapeKey(closeResults, isOpen); | ||
| useEscapeKey(syncSearchOnDismiss, isOpen); |
There was a problem hiding this comment.
I think closeResults (with onSearch('') ) will do the trick here
| onClick: !readOnly && !disabled ? onComboboxClick : undefined, | ||
| readOnly, | ||
| disabled, | ||
| onBlur: allowNewItems ? undefined : restoreValue, |
There was a problem hiding this comment.
| onBlur: allowNewItems ? props.onBlur : restoreValue, |
| className={classes.modal} | ||
| contentClassName={classes['modal-content']} | ||
| onClose={closeResults} | ||
| onClose={syncSearchOnDismiss} |
There was a problem hiding this comment.
I think closeResults (with onSearch('') ) will do the trick here
| 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, ''); | ||
| } |
There was a problem hiding this comment.
with the suggestions below, this can be simplified
| 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, ''); | |
| } | |
| const syncSearchOnDismiss = useCallback(() => { | |
| if (!Array.isArray(value) && searchText !== value?.value) { | |
| setSearchText(value?.label ?? ''); | |
| } |
| ## 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. |
There was a problem hiding this comment.
| - 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. | |
| - 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. Whenever the suggestion box is closed, the `onSearch` callback is fired with an empty string, allowing consumers to reset the suggestion list. |
| **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. |
There was a problem hiding this comment.
It is best to create one changeset file per change. It helps with the visibility and discovery of the different changes.
| **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. | |
| Updated the AutocompleteInput to fire the `onSearch` callback whenever the suggestion box is closed, allowing consumers to reset the suggestion list. |
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).
No ticket yet
How to reproduce the issue
Purpose
Fix the issue mentioned above, spotted here
Approach and changes
Definition of done