[EuiSelectable] Focus option after search value change (#3670)

elastic · Jul 6, 2020 · 2431824 · 2431824
1 parent e9b8b21
commit 2431824
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 53 deletions.
diff --git a/src-docs/src/views/selectable/selectable_custom_render.js b/src-docs/src/views/selectable/selectable_custom_render.js
@@ -13,7 +13,6 @@ import { createDataStore } from '../tables/data_store';
 export default () => {
   const countries = createDataStore().countries.map(country => {
     return {
-      id: country.code,
       label: `${country.name}`,
       prepend: country.flag,
       append: <EuiBadge>{country.code}</EuiBadge>,

diff --git a/src/components/selectable/selectable.tsx b/src/components/selectable/selectable.tsx
@@ -86,7 +86,7 @@ export type EuiSelectableProps = Omit<
     /**
      * Array of EuiSelectableOption objects. See #EuiSelectableOptionProps
      */
-    options: EuiSelectableOption[];
+    options: Array<Exclude<EuiSelectableOption, 'id'>>;
     /**
      * Passes back the altered `options` array with selected options as
      */
@@ -127,6 +127,7 @@ export interface EuiSelectableState {
   activeOptionIndex?: number;
   searchValue: string;
   visibleOptions: EuiSelectableOption[];
+  isFocused: boolean;
 }
 
 export class EuiSelectable extends Component<
@@ -163,6 +164,7 @@ export class EuiSelectable extends Component<
       activeOptionIndex,
       searchValue: initialSearchValue,
       visibleOptions,
+      isFocused: false,
     };
   }
 
@@ -192,14 +194,23 @@ export class EuiSelectable extends Component<
   };
 
   onFocus = () => {
+    if (!this.state.visibleOptions.length || this.state.activeOptionIndex) {
+      return;
+    }
+
     const firstSelected = this.state.visibleOptions.findIndex(
-      option => option.checked && !option.disabled
+      option => option.checked && !option.disabled && !option.isGroupLabel
     );
 
     if (firstSelected > -1) {
-      this.setState({ activeOptionIndex: firstSelected });
+      this.setState({ activeOptionIndex: firstSelected, isFocused: true });
     } else {
-      this.incrementActiveOptionIndex(1);
+      this.setState({
+        activeOptionIndex: this.state.visibleOptions.findIndex(
+          option => !option.disabled && !option.isGroupLabel
+        ),
+        isFocused: true,
+      });
     }
   };
 
@@ -245,10 +256,8 @@ export class EuiSelectable extends Component<
         break;
 
       default:
-        if (this.props.onKeyDown) {
-          this.props.onKeyDown(event);
-        }
-        this.clearActiveOption();
+        this.setState({ activeOptionIndex: undefined }, this.onFocus);
+        break;
     }
   };
 
@@ -294,29 +303,35 @@ export class EuiSelectable extends Component<
     });
   };
 
-  clearActiveOption = () => {
-    this.setState({
-      activeOptionIndex: undefined,
-    });
-  };
-
   onSearchChange = (
     visibleOptions: EuiSelectableOption[],
     searchValue: string
   ) => {
-    this.setState({
-      visibleOptions,
-      searchValue,
-    });
+    this.setState(
+      {
+        visibleOptions,
+        searchValue,
+        activeOptionIndex: undefined,
+      },
+      () => {
+        if (this.state.isFocused) {
+          this.onFocus();
+        }
+      }
+    );
   };
 
   onContainerBlur = () => {
-    this.clearActiveOption();
+    this.setState({
+      activeOptionIndex: undefined,
+      isFocused: false,
+    });
   };
 
   onOptionClick = (options: EuiSelectableOption[]) => {
     this.setState(state => ({
       visibleOptions: getMatchingOptions(options, state.searchValue),
+      activeOptionIndex: this.state.activeOptionIndex,
     }));
     if (this.props.onChange) {
       this.props.onChange(options);
@@ -508,9 +523,9 @@ export class EuiSelectable extends Component<
             visibleOptions={visibleOptions}
             searchValue={searchValue}
             activeOptionIndex={activeOptionIndex}
-            setActiveOptionIndex={index =>
-              this.setState({ activeOptionIndex: index })
-            }
+            setActiveOptionIndex={(index, cb) => {
+              this.setState({ activeOptionIndex: index }, cb);
+            }}
             onOptionClick={this.onOptionClick}
             singleSelection={singleSelection}
             ref={this.optionsListRef}

diff --git a/src/components/selectable/selectable_list/selectable_list.tsx b/src/components/selectable/selectable_list/selectable_list.tsx
@@ -106,7 +106,7 @@ export type EuiSelectableListProps = EuiSelectableOptionsListProps & {
   searchable?: boolean;
   makeOptionId: (index: number | undefined) => string;
   listId: string;
-  setActiveOptionIndex: (index: number) => void;
+  setActiveOptionIndex: (index: number, cb?: () => void) => void;
 };
 
 export class EuiSelectableList extends Component<EuiSelectableListProps> {
@@ -223,6 +223,9 @@ export class EuiSelectableList extends Component<EuiSelectableListProps> {
         id={this.props.makeOptionId(index)}
         style={style}
         key={key || label.toLowerCase()}
+        onMouseDown={() => {
+          this.props.setActiveOptionIndex(index);
+        }}
         onClick={() => this.onAddOrRemoveOption(option)}
         ref={ref ? ref.bind(null, index) : undefined}
         isFocused={this.props.activeOptionIndex === index}
@@ -333,19 +336,20 @@ export class EuiSelectableList extends Component<EuiSelectableListProps> {
       return;
     }
 
-    const { allowExclusions } = this.props;
+    const { allowExclusions, options, visibleOptions = options } = this.props;
 
     this.props.setActiveOptionIndex(
-      this.props.options.findIndex(({ label }) => label === option.label)
+      visibleOptions.findIndex(({ label }) => label === option.label),
+      () => {
+        if (option.checked === 'on' && allowExclusions) {
+          this.onExcludeOption(option);
+        } else if (option.checked === 'on' || option.checked === 'off') {
+          this.onRemoveOption(option);
+        } else {
+          this.onAddOption(option);
+        }
+      }
     );
-
-    if (option.checked === 'on' && allowExclusions) {
-      this.onExcludeOption(option);
-    } else if (option.checked === 'on' || option.checked === 'off') {
-      this.onRemoveOption(option);
-    } else {
-      this.onAddOption(option);
-    }
   };
 
   private onAddOption = (addedOption: EuiSelectableOption) => {

diff --git a/src/components/selectable/selectable_list/selectable_list_item.tsx b/src/components/selectable/selectable_list/selectable_list_item.tsx
@@ -163,10 +163,6 @@ export class EuiSelectableListItem extends Component<
       );
     }
 
-    if (allowExclusions) {
-      console.log({ state, instruction });
-    }
-
     return (
       <li
         // eslint-disable-next-line jsx-a11y/no-noninteractive-element-to-interactive-role

diff --git a/src/components/selectable/selectable_search/selectable_search.test.tsx b/src/components/selectable/selectable_search/selectable_search.test.tsx
@@ -26,7 +26,12 @@ import { EuiSelectableSearch } from './selectable_search';
 describe('EuiSelectableSearch', () => {
   test('is rendered', () => {
     const component = render(
-      <EuiSelectableSearch options={[]} listId="list" {...requiredProps} />
+      <EuiSelectableSearch
+        options={[]}
+        listId="list"
+        onChange={() => {}}
+        {...requiredProps}
+      />
     );
 
     expect(component).toMatchSnapshot();
@@ -35,7 +40,12 @@ describe('EuiSelectableSearch', () => {
   describe('props', () => {
     test('defaultValue', () => {
       const component = render(
-        <EuiSelectableSearch options={[]} listId="list" defaultValue="Mi" />
+        <EuiSelectableSearch
+          options={[]}
+          listId="list"
+          onChange={() => {}}
+          defaultValue="Mi"
+        />
       );
 
       expect(component).toMatchSnapshot();

diff --git a/src/components/selectable/selectable_search/selectable_search.tsx b/src/components/selectable/selectable_search/selectable_search.tsx
@@ -29,7 +29,7 @@ export type EuiSelectableSearchProps = Omit<EuiFieldSearchProps, 'onChange'> &
     /**
      * Passes back (matchingOptions, searchValue)
      */
-    onChange?: (
+    onChange: (
       matchingOptions: EuiSelectableOption[],
       searchValue: string
     ) => void;
@@ -59,22 +59,17 @@ export class EuiSelectableSearch extends Component<
   }
 
   componentDidMount() {
-    const { options } = this.props;
     const { searchValue } = this.state;
-    const matchingOptions = getMatchingOptions(options, searchValue);
-    this.passUpMatches(matchingOptions, searchValue);
+    const matchingOptions = getMatchingOptions(this.props.options, searchValue);
+    this.props.onChange(matchingOptions, searchValue);
   }
 
   onSearchChange = (value: string) => {
-    this.setState({ searchValue: value });
-    const { options } = this.props;
-    const matchingOptions = getMatchingOptions(options, value);
-    this.passUpMatches(matchingOptions, value);
-  };
-
-  passUpMatches = (matches: EuiSelectableOption[], searchValue: string) => {
-    if (this.props.onChange) {
-      this.props.onChange(matches, searchValue);
+    if (value !== this.state.searchValue) {
+      this.setState({ searchValue: value }, () => {
+        const matchingOptions = getMatchingOptions(this.props.options, value);
+        this.props.onChange(matchingOptions, value);
+      });
     }
   };