[EuiComboBox] Fix enter key not working correctly on selection clear/…

…deletion buttons (#8105)
elastic · Oct 30, 2024 · 75b0559 · 75b0559
1 parent 4156987
commit 75b0559
Show file tree

Hide file tree

Showing 3 changed files with 149 additions and 113 deletions.
diff --git a/packages/eui/changelogs/upcoming/8105.md b/packages/eui/changelogs/upcoming/8105.md
@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed an `EuiComboBox` bug where Enter keypresses were not working correctly on selection clear buttons 
diff --git a/packages/eui/src/components/combo_box/combo_box.spec.tsx b/packages/eui/src/components/combo_box/combo_box.spec.tsx
@@ -21,6 +21,31 @@ import {
 } from './index';
 
 describe('EuiComboBox', () => {
+  const defaultOptions: Array<EuiComboBoxOptionOption<{}>> = [
+    { label: 'Item 1' },
+    { label: 'Item 2' },
+    { label: 'Item 3' },
+  ];
+  const StatefulComboBox: FunctionComponent<Partial<EuiComboBoxProps<{}>>> = ({
+    options = defaultOptions,
+    selectedOptions: _selectedOptions = [],
+    ...rest
+  }) => {
+    const [selectedOptions, setSelected] =
+      useState<typeof options>(_selectedOptions);
+    const onChange = (selectedOptions: typeof options) => {
+      setSelected(selectedOptions);
+    };
+    return (
+      <EuiComboBox
+        options={options}
+        selectedOptions={selectedOptions}
+        {...rest}
+        onChange={onChange}
+      />
+    );
+  };
+
   describe('focus management', () => {
     it('keeps focus on the input box when clicking a disabled item', () => {
       cy.realMount(
@@ -44,6 +69,116 @@ describe('EuiComboBox', () => {
     });
   });
 
+  describe('keyboard UX', () => {
+    it('allows the enter key to delete items and clear selections', () => {
+      cy.realMount(
+        <StatefulComboBox
+          data-test-subj="combobox"
+          selectedOptions={[{ label: 'Item 1' }, { label: 'Item 2' }]}
+          options={defaultOptions}
+        />
+      );
+      cy.get('.euiComboBoxPill').should('have.length', 2);
+
+      cy.realPress('Tab');
+      cy.focused().should(
+        'have.attr',
+        'aria-label',
+        'Remove Item 1 from selection in this group'
+      );
+      cy.realPress('Enter');
+      cy.get('.euiComboBoxPill').should('have.length', 1);
+
+      cy.realPress('Tab');
+      cy.focused().should('have.attr', 'data-test-subj', 'comboBoxClearButton');
+      cy.realPress('Enter');
+      cy.get('.euiComboBoxPill').should('have.length', 0);
+    });
+
+    describe('backspace to delete last pill', () => {
+      it('does not delete the last pill if there is search text', () => {
+        cy.realMount(<StatefulComboBox />);
+        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.get('.euiComboBoxPill').should('have.length', 2);
+
+        cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
+        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
+
+        cy.get('[data-test-subj=comboBoxSearchInput]')
+          .invoke('val')
+          .should('equal', 'tes');
+        cy.get('.euiComboBoxPill').should('have.length', 2);
+      });
+
+      it('does not delete the last pill if the input is not active when backspace is pressed', () => {
+        cy.realMount(<StatefulComboBox />);
+        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
+        cy.realPress('Escape');
+        cy.get('.euiComboBoxPill').should('have.length', 1);
+
+        cy.realPress(['Shift', 'Tab']); // Should be focused on the first pill's X button
+        cy.realPress('Backspace');
+        cy.get('.euiComboBoxPill').should('have.length', 1);
+
+        cy.repeatRealPress('Tab', 2); // Should be focused on the clear button
+        cy.realPress('Backspace');
+        cy.get('.euiComboBoxPill').should('have.length', 1);
+      });
+
+      it('deletes the last pill added when backspace on the input is pressed ', () => {
+        cy.realMount(<StatefulComboBox />);
+        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.get('.euiComboBoxPill').should('have.length', 2);
+
+        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
+        cy.get('.euiComboBoxPill').should('have.length', 1);
+      });
+
+      it('`asPlainText`: deletes the selection and only a single character', () => {
+        cy.realMount(
+          // @ts-ignore - not totally sure why TS is kicking up a fuss here
+          <StatefulComboBox singleSelection={{ asPlainText: true }} />
+        );
+        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+        cy.get('[data-test-subj=comboBoxSearchInput]').should(
+          'have.value',
+          'Item 1'
+        );
+        cy.get('[data-test-subj="comboBoxClearButton"]').should('exist'); // indicates selection
+
+        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
+        cy.get('[data-test-subj="comboBoxClearButton"]').should('not.exist'); // selection removed
+        cy.get('[data-test-subj=comboBoxSearchInput]').should(
+          'have.value',
+          'Item '
+        );
+      });
+
+      it('opens up the selection list again after deleting the active single selection ', () => {
+        cy.realMount(<StatefulComboBox singleSelection />);
+        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
+        cy.realPress('{downarrow}');
+        cy.realPress('Enter');
+
+        cy.realPress('Backspace');
+        cy.get('[data-test-subj=comboBoxOptionsList]').should('have.length', 1);
+      });
+    });
+  });
+
   describe('input auto sizing', () => {
     it('resizes the width of the input based to fit the search text', () => {
       cy.realMount(<EuiComboBox options={[]} />);
@@ -221,28 +356,6 @@ describe('EuiComboBox', () => {
   });
 
   describe('selection', () => {
-    const defaultOptions: Array<EuiComboBoxOptionOption<{}>> = [
-      { label: 'Item 1' },
-      { label: 'Item 2' },
-      { label: 'Item 3' },
-    ];
-    const StatefulComboBox: FunctionComponent<
-      Partial<EuiComboBoxProps<{}>>
-    > = ({ options = defaultOptions, ...rest }) => {
-      const [selectedOptions, setSelected] = useState<typeof options>([]);
-      const onChange = (selectedOptions: typeof options) => {
-        setSelected(selectedOptions);
-      };
-      return (
-        <EuiComboBox
-          options={options}
-          selectedOptions={selectedOptions}
-          {...rest}
-          onChange={onChange}
-        />
-      );
-    };
-
     describe('delimiter', () => {
       it('selects the option when the delimiter option is typed into the search', () => {
         cy.mount(<StatefulComboBox delimiter="," />);
@@ -343,89 +456,6 @@ describe('EuiComboBox', () => {
         });
       });
     });
-
-    describe('backspace to delete last pill', () => {
-      it('does not delete the last pill if there is search text', () => {
-        cy.realMount(<StatefulComboBox />);
-        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.get('.euiComboBoxPill').should('have.length', 2);
-
-        cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
-        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
-
-        cy.get('[data-test-subj=comboBoxSearchInput]')
-          .invoke('val')
-          .should('equal', 'tes');
-        cy.get('.euiComboBoxPill').should('have.length', 2);
-      });
-
-      it('does not delete the last pill if the input is not active when backspace is pressed', () => {
-        cy.realMount(<StatefulComboBox />);
-        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
-        cy.realPress('Escape');
-        cy.get('.euiComboBoxPill').should('have.length', 1);
-
-        cy.realPress(['Shift', 'Tab']); // Should be focused on the first pill's X button
-        cy.realPress('Backspace');
-        cy.get('.euiComboBoxPill').should('have.length', 1);
-
-        cy.repeatRealPress('Tab', 2); // Should be focused on the clear button
-        cy.realPress('Backspace');
-        cy.get('.euiComboBoxPill').should('have.length', 1);
-      });
-
-      it('deletes the last pill added when backspace on the input is pressed ', () => {
-        cy.realMount(<StatefulComboBox />);
-        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.get('.euiComboBoxPill').should('have.length', 2);
-
-        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
-        cy.get('.euiComboBoxPill').should('have.length', 1);
-      });
-
-      it('`asPlainText`: deletes the selection and only a single character', () => {
-        cy.realMount(
-          // @ts-ignore - not totally sure why TS is kicking up a fuss here
-          <StatefulComboBox singleSelection={{ asPlainText: true }} />
-        );
-        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-        cy.get('[data-test-subj=comboBoxSearchInput]').should(
-          'have.value',
-          'Item 1'
-        );
-        cy.get('[data-test-subj="comboBoxClearButton"]').should('exist'); // indicates selection
-
-        cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
-        cy.get('[data-test-subj="comboBoxClearButton"]').should('not.exist'); // selection removed
-        cy.get('[data-test-subj=comboBoxSearchInput]').should(
-          'have.value',
-          'Item '
-        );
-      });
-
-      it('opens up the selection list again after deleting the active single selection ', () => {
-        cy.realMount(<StatefulComboBox singleSelection />);
-        cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
-        cy.realPress('{downarrow}');
-        cy.realPress('Enter');
-
-        cy.realPress('Backspace');
-        cy.get('[data-test-subj=comboBoxOptionsList]').should('have.length', 1);
-      });
-    });
   });
 
   describe('z-index regression testing', () => {

diff --git a/packages/eui/src/components/combo_box/combo_box.tsx b/packages/eui/src/components/combo_box/combo_box.tsx
@@ -536,14 +536,17 @@ export class EuiComboBox<T> extends Component<
         break;
 
       case keys.ENTER:
-        event.preventDefault();
-        event.stopPropagation();
-        if (this.hasActiveOption()) {
-          this.onAddOption(
-            this.state.matchingOptions[this.state.activeOptionIndex]
-          );
-        } else {
-          this.setCustomOptions(false);
+        // Do not block enter keypresses for the clear button or delete selection buttons
+        if (event.target === this.searchInputRefInstance) {
+          event.preventDefault();
+          event.stopPropagation();
+          if (this.hasActiveOption()) {
+            this.onAddOption(
+              this.state.matchingOptions[this.state.activeOptionIndex]
+            );
+          } else {
+            this.setCustomOptions(false);
+          }
         }
         break;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		Bug fixes

		- Fixed an `EuiComboBox` bug where Enter keypresses were not working correctly on selection clear buttons