Restore UX where auto-sorted items should be scrolled to after being …

…checked + retain selected/active index highlight (better UX than before, where focus/active index was lost) + simplify stateful test components and update autosort tests to be simpler and check for active attributes
elastic · Aug 30, 2024 · fef4ec8 · fef4ec8
1 parent 0733b47
commit fef4ec8
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 97 deletions.
diff --git a/packages/eui/src/components/search_bar/filters/field_value_selection_filter.spec.tsx b/packages/eui/src/components/search_bar/filters/field_value_selection_filter.spec.tsx
@@ -15,6 +15,7 @@ import { requiredProps } from '../../../test';
 import {
   FieldValueSelectionFilter,
   FieldValueSelectionFilterProps,
+  FieldValueSelectionFilterConfigType,
 } from './field_value_selection_filter';
 import { Query } from '../query';
 
@@ -34,6 +35,29 @@ const staticOptions = [
 ];
 
 describe('FieldValueSelectionFilter', () => {
+  const FieldValueSelectionFilterWithState = (
+    config: Partial<FieldValueSelectionFilterConfigType>
+  ) => {
+    const [query, setQuery] = useState(Query.parse(''));
+    const onChange = (newQuery: Query) => setQuery(newQuery);
+
+    const props: FieldValueSelectionFilterProps = {
+      ...requiredProps,
+      index: 0,
+      onChange,
+      query,
+      config: {
+        type: 'field_value_selection',
+        field: 'tag',
+        name: 'Tag',
+        options: staticOptions,
+        ...config,
+      },
+    };
+
+    return <FieldValueSelectionFilter {...props} />;
+  };
+
   it('allows options as a function', () => {
     const props: FieldValueSelectionFilterProps = {
       ...requiredProps,
@@ -140,31 +164,6 @@ describe('FieldValueSelectionFilter', () => {
   });
 
   describe('multi-select testing', () => {
-    const FieldValueSelectionFilterWithState = ({
-      multiSelect,
-    }: {
-      multiSelect: 'or' | boolean;
-    }) => {
-      const [query, setQuery] = useState(Query.parse(''));
-      const onChange = (newQuery: Query) => setQuery(newQuery);
-
-      const props: FieldValueSelectionFilterProps = {
-        ...requiredProps,
-        index: 0,
-        onChange,
-        query,
-        config: {
-          type: 'field_value_selection',
-          field: 'tag',
-          name: 'Tag',
-          multiSelect,
-          options: staticOptions,
-        },
-      };
-
-      return <FieldValueSelectionFilter {...props} />;
-    };
-
     it('uses multi-select OR', () => {
       cy.mount(<FieldValueSelectionFilterWithState multiSelect="or" />);
       cy.get('button').click();
@@ -226,33 +225,6 @@ describe('FieldValueSelectionFilter', () => {
   });
 
   describe('auto-close testing', () => {
-    const FieldValueSelectionFilterWithState = ({
-      autoClose,
-      multiSelect,
-    }: {
-      autoClose: undefined | boolean;
-      multiSelect: 'or' | boolean;
-    }) => {
-      const [query, setQuery] = useState(Query.parse(''));
-      const onChange = (newQuery: Query) => setQuery(newQuery);
-
-      const props: FieldValueSelectionFilterProps = {
-        ...requiredProps,
-        index: 0,
-        onChange,
-        query,
-        config: {
-          type: 'field_value_selection',
-          field: 'tag',
-          name: 'Tag',
-          multiSelect,
-          autoClose,
-          options: staticOptions,
-        },
-      };
-
-      return <FieldValueSelectionFilter {...props} />;
-    };
     const selectFilter = () => {
       // Open popover
       cy.get('button').click();
@@ -339,51 +311,33 @@ describe('FieldValueSelectionFilter', () => {
   });
 
   describe('autoSortOptions', () => {
-    it('sorts selected options to the top by default', () => {
-      const props: FieldValueSelectionFilterProps = {
-        index: 0,
-        onChange: () => {},
-        query: Query.parse('tag:bug'),
-        config: {
-          type: 'field_value_selection',
-          field: 'tag',
-          name: 'Tag',
-          options: staticOptions,
-        },
-      };
-      cy.mount(<FieldValueSelectionFilter {...props} />);
-      cy.get('.euiNotificationBadge').should('exist');
+    const getOptions = () => cy.get('.euiSelectableListItem');
 
+    it('sorts selected options to the top by default', () => {
+      cy.mount(<FieldValueSelectionFilterWithState />);
       cy.get('button').click();
-      const getOptions = () => cy.get('.euiSelectableListItem');
-
       getOptions().should('have.length', 3);
-      getOptions().eq(0).should('have.attr', 'title', 'Bug');
-      getOptions().eq(1).should('have.attr', 'title', 'feature');
+
+      getOptions().last().should('have.attr', 'title', 'Bug').click();
+      // Should have moved to the top of the list and retained active focus
+      getOptions()
+        .first()
+        .should('have.attr', 'title', 'Bug')
+        .should('have.attr', 'aria-checked', 'true')
+        .should('have.attr', 'aria-selected', 'true');
     });
 
     it('does not sort selected options to the top when set to false', () => {
-      const props: FieldValueSelectionFilterProps = {
-        index: 0,
-        onChange: () => {},
-        query: Query.parse('tag:bug'),
-        config: {
-          type: 'field_value_selection',
-          field: 'tag',
-          name: 'Tag',
-          options: staticOptions,
-          autoSortOptions: false,
-        },
-      };
-      cy.mount(<FieldValueSelectionFilter {...props} />);
-      cy.get('.euiNotificationBadge').should('exist');
-
+      cy.mount(<FieldValueSelectionFilterWithState autoSortOptions={false} />);
       cy.get('button').click();
-      const getOptions = () => cy.get('.euiSelectableListItem');
-
       getOptions().should('have.length', 3);
-      getOptions().eq(2).should('have.attr', 'title', 'Bug');
-      getOptions().eq(0).should('have.attr', 'title', 'feature');
+
+      getOptions().last().should('have.attr', 'title', 'Bug').click();
+      getOptions()
+        .last()
+        .should('have.attr', 'title', 'Bug')
+        .should('have.attr', 'aria-checked', 'true')
+        .should('have.attr', 'aria-selected', 'true');
     });
   });
 

diff --git a/packages/eui/src/components/search_bar/filters/field_value_selection_filter.tsx b/packages/eui/src/components/search_bar/filters/field_value_selection_filter.tsx
@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import React, { Component, ReactNode } from 'react';
+import React, { Component, ReactNode, createRef } from 'react';
 
 import { RenderWithEuiTheme } from '../../../services';
 import { isArray, isNil } from '../../../services/predicate';
@@ -81,12 +81,14 @@ interface State {
   } | null;
   cachedOptions?: FieldValueOptionType[] | null;
   activeItemsCount: number;
+  lastCheckedValue?: Value;
 }
 
 export class FieldValueSelectionFilter extends Component<
   FieldValueSelectionFilterProps,
   State
 > {
+  selectableClassRef = createRef<EuiSelectable>();
   cacheTimeout: ReturnType<typeof setTimeout> | undefined;
 
   constructor(props: FieldValueSelectionFilterProps) {
@@ -181,14 +183,35 @@ export class FieldValueSelectionFilter extends Component<
       });
     }
 
-    this.setState({
-      error: null,
-      activeItemsCount: items.on.length,
-      options: {
-        unsorted: loadedOptions,
-        sorted: [...items.on, ...items.off, ...items.rest],
+    this.setState(
+      {
+        error: null,
+        activeItemsCount: items.on.length,
+        options: {
+          unsorted: loadedOptions,
+          sorted: [...items.on, ...items.off, ...items.rest],
+        },
       },
-    });
+      this.scrollToAutoSortedOption
+    );
+  };
+
+  scrollToAutoSortedOption = () => {
+    if (!this.autoSortOptions) return;
+
+    const { lastCheckedValue, options } = this.state;
+    if (lastCheckedValue) {
+      const sortedIndex = options!.sorted.findIndex(
+        (option) => option.value === lastCheckedValue
+      );
+      if (sortedIndex >= 0) {
+        // EuiSelectable should automatically handle scrolling its list to the new
+        this.selectableClassRef.current?.setState({
+          activeOptionIndex: sortedIndex,
+        });
+      }
+      this.setState({ lastCheckedValue: undefined });
+    }
   };
 
   resolveOptionName(option: FieldValueOptionType) {
@@ -204,6 +227,10 @@ export class FieldValueSelectionFilter extends Component<
       config: { autoClose, operator = Operator.EQ },
     } = this.props;
 
+    if (checked && this.autoSortOptions) {
+      this.setState({ lastCheckedValue: value });
+    }
+
     // If the consumer explicitly sets `autoClose`, always defer to that.
     // Otherwise, default to auto-closing for single selections and leaving the
     // popover open for multi-select (so users can continue selecting options)
@@ -351,6 +378,7 @@ export class FieldValueSelectionFilter extends Component<
             }}
           >
             <EuiSelectable<Partial<(typeof items)[number]['data']>>
+              ref={this.selectableClassRef}
               singleSelection={!this.multiSelect}
               aria-label={config.name}
               options={items}