Skip to content

Commit

Permalink
[EuiSelectable] Fix searchable single selection selectables not corre…
Browse files Browse the repository at this point in the history
…ctly highlighting the checked option on initial render (#6072)

* Fix activeIndex being incorrectly set to undefined on mount for `singleSelection` `searchable` EuiSelectables

`Euiselectable.onSearchChange` being called on mount by `EuiSelectableSearch` which was setting `activeOptionIndex` to undefined. Moving the call to the `EuiSelectable.constructor` (which calls getMatchingOptions on mount anyway, so no need for `EuiSelectableSearch` to repeat it fixes the issue and leaves the found `activeOptionIndex` from the constructor intact.

* [Misc bugfix] Fix onFocus progressing when it shouldn't if `activeOptionIndex` is  a 0 index

since 0 is falsey but we really should be checking for undefined

* [Misc cleanup] Remove unused class method

- doesn't appear to be used anywhere

* Changelog
  • Loading branch information
Constance authored Jul 25, 2022
1 parent 5310bfb commit 4018c82
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 25 deletions.
14 changes: 14 additions & 0 deletions src/components/selectable/selectable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,20 @@ describe('EuiSelectable', () => {
).toEqual('second value');
});

it('calls the searchProps.onChange callback on mount', () => {
const onChange = jest.fn();
mount(
<EuiSelectable
options={options}
searchable
searchProps={{ value: 'pandora', onChange }}
>
{(_, search) => <>{search}</>}
</EuiSelectable>
);
expect(onChange).toHaveBeenCalledWith('pandora', [options[2]]);
});

it('defaults to an empty string if no value or defaultValue is passed from searchProps', () => {
const component = render(
<EuiSelectable
Expand Down
14 changes: 6 additions & 8 deletions src/components/selectable/selectable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export class EuiSelectable<T = {}> extends Component<
initialSearchValue,
isPreFiltered
);
searchProps?.onChange?.(initialSearchValue, visibleOptions);

// ensure that the currently selected single option is active if it is in the visibleOptions
const selectedOptions = options.filter((option) => option.checked);
Expand Down Expand Up @@ -277,10 +278,6 @@ export class EuiSelectable<T = {}> extends Component<
}
}

hasActiveOption = () => {
return this.state.activeOptionIndex != null;
};

onMouseDown = () => {
// Bypass onFocus when a click event originates from this.containerRef.
// Prevents onFocus from scrolling away from a clicked option and negating the selection event.
Expand All @@ -294,7 +291,10 @@ export class EuiSelectable<T = {}> extends Component<
return;
}

if (!this.state.visibleOptions.length || this.state.activeOptionIndex) {
if (
!this.state.visibleOptions.length ||
this.state.activeOptionIndex != null
) {
return;
}

Expand Down Expand Up @@ -436,9 +436,7 @@ export class EuiSelectable<T = {}> extends Component<
}
}
);
if (this.props.searchProps && this.props.searchProps.onChange) {
this.props.searchProps.onChange(searchValue, visibleOptions);
}
this.props.searchProps?.onChange?.(searchValue, visibleOptions);
};

onContainerBlur = (e: React.FocusEvent) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import React from 'react';
import { render, mount, shallow } from 'enzyme';
import { render, shallow } from 'enzyme';
import { requiredProps } from '../../../test/required_props';

import { EuiSelectableSearch } from './selectable_search';
Expand Down Expand Up @@ -50,11 +50,6 @@ describe('EuiSelectableSearch', () => {
expect(component).toMatchSnapshot();
});

it('calls the onChange callback on mount', () => {
mount(<EuiSelectableSearch {...baseProps} value="w" />);
expect(onChange).toHaveBeenCalledWith('w', [{ label: 'world' }]);
});

it("calls the onChange callback when the EuiFieldSearch's change event fires", () => {
const component = shallow(<EuiSelectableSearch {...baseProps} />);
component
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { useEffect, useCallback, ChangeEvent } from 'react';
import React, { useCallback, ChangeEvent } from 'react';
import classNames from 'classnames';
import { CommonProps } from '../../common';
import { EuiFieldSearch, EuiFieldSearchProps } from '../../form';
Expand Down Expand Up @@ -52,16 +52,6 @@ export const EuiSelectableSearch = <T,>({
className,
...rest
}: _EuiSelectableSearchProps<T>) => {
useEffect(() => {
const matchingOptions = getMatchingOptions<T>(
options,
value,
isPreFiltered
);
onChangeCallback(value, matchingOptions);
// Call on mount only
}, []); // eslint-disable-line react-hooks/exhaustive-deps

const onChange = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
const searchValue = e.target.value;
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/6072.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed searchable single selection `EuiSelectable`s not correctly highlighting the checked option on initial render

0 comments on commit 4018c82

Please sign in to comment.