eui

eui copied to clipboard

When using an EuiComboBox, with onSearchChange, after a user selects an option, then it will trigger a search with a blank string (''):

https://github.com/elastic/eui/blob/bc24f11a13980e61300e27ed4f733018ce50fee5/src/components/combo_box/combo_box.tsx#L690-L694

https://github.com/elastic/eui/blob/bc24f11a13980e61300e27ed4f733018ce50fee5/src/components/combo_box/combo_box.tsx#L423-L425

This is unexpected and it requires the implementor of onSearchChange to ignore '' strings in order to avoid the selection of an item triggering a search. I would expect no search to be triggered as the result of a selection or, at most, the search to be the newly selected value.

I'm not 100% seeing the issue here. When you type to start searching and select what you've searched for, the search does clear and therefore the onSearchChange callback is valid:

Are you describing the scenario where the search value is already empty/'' and clicking an option sends another '' value? So basically you want to check if the search value actually changed, and not fire onSearchChange event if so?

In my case, we were using a single selection, not a multiselect, which triggered the unexpected behavior.

Ahhh, I see that UX confusion especially with how single selection displays plain text selections vs user search.

To be honest I still think the '' case is very obviously valid for multi select, and is still mostly valid technically for single select as well (the selection is not the search, the search is what the user is typing, and we are clearing what the user typed).

@cchaos / @miukimiu any thoughts here?

I definitely agree with the multi-select scenario and it makes sense how it appeared. I would not expect a search after a selection for a single select though because they've selected their single selection -- why potentially search for something else.

It's not that the user is searching after selection, it's that we're reporting whenever the value of the search text changes, even when EUI is programmatically clearing it. So perhaps onSearchChange is not as accurate a name as onSearchValueChange. The empty string is still potentially important/useful for some consumers who may want to react to all changes (e.g. in the case of a completely controlled input), even if other consumers don't need that information.

To be honest it's extremely unlikely we'll change that current technical behavior, since it would be a fairly breaking change, and since consumers who don't need that information can simply ignore empty strings.

Based on the conversation above, this is working as we've intended.