[EuiSelectable] list not filtered when onChange or onSearch included in searchProps

[smhutch]

When searchable and one or more of searchProps.onChange and searchProps.onSearch is passed to EuiSelectable, the options are not correctly filtered in the UI. I created a sandbox showing both issues.

Code

I believe in both cases the searchProps are being spread over more important props, which seems unintended.

onChange

eui/src/components/selectable/selectable.tsx

Lines 534 to 551 in 01c6986

	const search = searchable ? (
	<EuiI18n token="euiSelectable.placeholderName" default="Filter options">
	{(placeholderName: string) => (
	<EuiSelectableSearch<T>
	key="listSearch"
	options={options}
	onChange={this.onSearchChange}
	listId={this.optionsListRef.current ? listId : undefined} // Only pass the listId if it exists on the page
	aria-activedescendant={makeOptionId(activeOptionIndex)} // the current faux-focused option
	placeholder={placeholderName}
	{...(searchHasAccessibleName
	? searchAccessibleName
	: { 'aria-label': placeholderName })}
	{...cleanedSearchProps}
	/>
	)}
	</EuiI18n>
	) : undefined;

• onChange={this.onSearchChange} appears to be key to updating the selectable internal state
• {...cleanedSearchProps} overrides it

onSearch

I'm less sure about the cause of this issue, but have a feeling it could be:

eui/src/components/selectable/selectable_search/selectable_search.tsx

Lines 113 to 125 in 01c6986

	return (
	<EuiFieldSearch
	className={classes}
	placeholder={placeholder}
	onSearch={this.onSearchChange}
	incremental
	defaultValue={defaultValue}
	fullWidth
	autoComplete="off"
	aria-haspopup="listbox"
	{...ariaPropsIfListIsPresent}
	{...rest}
	/>

• Same problem as above, where onSearch can exist in ...rest (which comes from cleanedSearchProps above), overriding the important call to this.onSearchChange

Context

The feature I'm working on extends the EuiSelectable component, by adding "Select all" and "Select none" buttons above the searchable EuiSelectable. Here's an example of the UI.

This works without any hassle. I update the local options state when a user clicks on either button, and pass this state down to EuiSelectable. After building this, I realized that it would be a better UX to combine this behaviour with the client-side search offered by adding searchable. In my current implementation the user could filter (let's say to view 5 of 100 items), then click "Select all". This would select not only the visible/filtered options, but all of them.

To fix this UX issue, I tried to access the currently filtered items. I found I could do this as shown:

searchProps={{
  onChange: (filteredOptions) => {
     // here I set some local state. Implementation details not important.
  },
}}

Elsewhere, I could use this data to compare the filteredOptions and options, and work out which options should be updated when a user clicks on one of the Select all/none buttons. Again, no problems here.

I believe the data-flow is working fine using the existing props, and the only issue is that my combination of props prevents the EuiSelectable from accurately reflecting the search filters in the UI.

[thompsongl]

Thanks for the investigation, @smhutch

It seems like your assessment is correct. We should be calling searchProps.onChange and searchProps.onSearch in addition to the "standard" onChange and onSearch callbacks.

[thompsongl]

Fixed in #4153