[EuiSuggest] Fire onItemClick callback on Enter key presses; [EuiSelectable] Fix Enter and Space key to select items on non-searchable listboxes

CHANGELOG.md

@@ -1,6 +1,10 @@
 ## [`main`](https://github.com/elastic/eui/tree/main)
 
-No public interface changes since `50.0.0`.
+- Enhanced `EuiSuggest` to fire the `onItemClick` callback on Enter key press as well as clicks ([#5693](https://github.com/elastic/eui/pull/5693))
+
+**Bug fixes**
+
+- Fixed non-searchable `EuiSelectable`s not selecting items with the Enter & Space keys ([#5693](https://github.com/elastic/eui/pull/5693))
 
 ## [`50.0.0`](https://github.com/elastic/eui/tree/v50.0.0)
 

src/components/selectable/selectable.spec.tsx

@@ -123,5 +123,137 @@ describe('EuiSelectable', () => {
             .should('have.attr', 'title', 'Titan');
         });
     });
+
+    it('allows pressing the Enter key to select an item', () => {
+      const onChange = cy.stub();
+      cy.realMount(
+        <EuiSelectable searchable options={options} onChange={onChange}>
+          {(list, search) => (
+            <>
+              {search}
+              {list}
+            </>
+          )}
+        </EuiSelectable>
+      );
+
+      cy.realPress('Tab');
+      cy.realPress('Enter').then(() => {
+        expect(onChange).to.have.been.calledWith([
+          { ...options[0], checked: 'on' },
+          options[1],
+          options[2],
+        ]);
+      });
+    });
+
+    it('does not allow pressing the Space key to select an item while in the searchbox (should filter instead)', () => {
+      const onItemChange = cy.stub();
+      const onSearchChange = cy.stub();
+      cy.realMount(
+        <EuiSelectable
+          searchable
+          options={options}
+          onChange={onItemChange}
+          searchProps={{ onChange: onSearchChange }}
+        >
+          {(list, search) => (
+            <>
+              {search}
+              {list}
+            </>
+          )}
+        </EuiSelectable>
+      );
+
+      cy.realPress('Tab');
+      cy.realPress('Space').then(() => {
+        expect(onItemChange).not.have.been.called;
+        expect(onSearchChange).to.have.been.calledWith(' ');
+      });
+    });
+
+    // mouse+keyboard combo users
+    it('allows users to click into an list item and still press Enter or Space to toggle list items', () => {
+      const onChange = cy.stub();
+      cy.realMount(
+        <EuiSelectable searchable options={options} onChange={onChange}>
+          {(list, search) => (
+            <>
+              {search}
+              {list}
+            </>
+          )}
+        </EuiSelectable>
+      );
+
+      cy.get('li[role=option]')
+        .first()
+        .click()
+        .then(() => {
+          expect(onChange).to.have.been.calledWith([
+            { ...options[0], checked: 'on' },
+            options[1],
+            options[2],
+          ]);
+        });
+      cy.realPress('ArrowDown')
+        .realPress('Enter')
+        .then(() => {
+          expect(onChange).to.have.been.calledWith([
+            options[0], // FYI: doesn't remain `on` because `options` is not controlled to remember state
+            { ...options[1], checked: 'on' },
+            options[2],
+          ]);
+        });
+      cy.realPress('ArrowDown')
+        .realPress('Space')
+        .then(() => {
+          expect(onChange).to.have.been.calledWith([
+            options[0],
+            options[1],
+            { ...options[2], checked: 'on' },
+          ]);
+        });
+    });
+  });
+
+  describe('without a `searchable` configuration', () => {
+    it('allows pressing the Enter key to select an item', () => {
+      const onChange = cy.stub();
+      cy.realMount(
+        <EuiSelectable options={options} onChange={onChange}>
+          {(list) => <>{list}</>}
+        </EuiSelectable>
+      );
+
+      cy.realPress('Tab');
+      cy.realPress('ArrowDown');
+      cy.realPress('Enter').then(() => {
+        expect(onChange).to.have.been.calledWith([
+          options[0],
+          { ...options[1], checked: 'on' },
+          options[2],
+        ]);
+      });
+    });
+
+    it('allows pressing the Space key to select an item', () => {
+      const onChange = cy.stub();
+      cy.realMount(
+        <EuiSelectable options={options} onChange={onChange}>
+          {(list) => <>{list}</>}
+        </EuiSelectable>
+      );
+      cy.realPress('Tab');
+      cy.repeatRealPress('ArrowDown', 2);
+      cy.realPress('Space').then(() => {
+        expect(onChange).to.have.been.calledWith([
+          options[0],
+          options[1],
+          { ...options[2], checked: 'on' },
+        ]);
+      });
+    });
   });
 });

src/components/selectable/selectable.tsx

@@ -305,22 +305,30 @@ export class EuiSelectable<T = {}> extends Component<
         this.incrementActiveOptionIndex(1);
         break;
 
+      // For non-searchable instances, SPACE interaction should align with
+      // the user expectation of selection toggling (e.g., input[type=checkbox]).
+      // ENTER is also a valid selection mechanism in this case.
       case keys.ENTER:
       case keys.SPACE:
-        if (event.key === keys.SPACE && this.props.searchable) {
-          // For non-searchable instances, SPACE interaction should align with
-          // the user expectation of selection toggling (e.g., input[type=checkbox]).
-          // ENTER is also a valid selection mechanism in this case.
-          //
+        if (this.props.searchable) {
           // For searchable instances, SPACE is reserved as a character for filtering
           // via the input box, and as such only ENTER will toggle selection.
-          return;
-        }
-        if (event.target !== this.inputRef) {
-          // The captured event is not derived from the searchbox.
-          // The user is attempting to interact with an internal button,
-          // such as the clear button, and the event should not be altered.
-          return;
+          if (event.target === this.inputRef && event.key === keys.SPACE) {
+            return;
+          }
+          // Check if the user is interacting with something other than the
+          // searchbox or selection list. If not, the user is attempting to
+          // interact with an internal button such as the clear button,
+          // and the event should not be altered.
+          if (
+            !(
+              event.target === this.inputRef ||
+              event.target ===
+                this.optionsListRef.current?.listBoxRef?.parentElement
+            )
+          ) {
+            return;
+          }
         }
         event.preventDefault();
         event.stopPropagation();

src/components/suggest/__snapshots__/suggest.test.tsx.snap

@@ -158,6 +158,7 @@ exports[`EuiSuggest props isVirtualized 1`] = `
         "textWrap": "truncate",
       }
     }
+    onChange={[Function]}
     options={
       Array [
         Object {
@@ -489,6 +490,7 @@ exports[`EuiSuggest props maxHeight 1`] = `
         "textWrap": "wrap",
       }
     }
+    onChange={[Function]}
     options={
       Array [
         Object {

src/components/suggest/_suggest_item.scss

@@ -15,23 +15,6 @@
   }
 }
 
-// When `onClick` is provided, EuiSuggestItem renders as a <button>
-// and we want to adjust styles to better afford the interaction
-button.euiSuggestItem { // sass-lint:disable-line no-qualifying-elements
-  width: 100%;
-  text-align: left;
-
-  &:hover,
-  &:focus {
-    cursor: pointer;
-    background-color: $euiFocusBackgroundColor;
-
-    .euiSuggestItem__label {
-      text-decoration: underline;
-    }
-  }
-}
-
 @each $name, $color in $euiSuggestItemColors {
   .euiSuggestItem__type--#{$name} {
     $backgroundColor: tintOrShade($color, 82%, 70%);

src/components/suggest/suggest.spec.tsx

@@ -46,9 +46,30 @@ describe('EuiSuggest', () => {
         .first()
         .click()
         .then(() => {
-          expect(handler).to.be.called;
+          expect(handler).to.be.calledWith(sampleItems[0]);
         });
     });
+
+    it('is called when an option is pressed via the Enter key', () => {
+      const handler = cy.stub();
+      cy.mount(
+        <EuiSuggest
+          aria-label="onItemClick"
+          suggestions={sampleItems}
+          onItemClick={handler}
+        />
+      );
+
+      cy.get('input')
+        .click()
+        .then(() => {
+          expect(cy.get('ul')).to.exist;
+        });
+      cy.realPress('ArrowDown');
+      cy.realPress('Enter').then(() => {
+        expect(handler).to.be.calledWith(sampleItems[0]);
+      });
+    });
   });
 
   describe('onInput', () => {

src/components/suggest/suggest.test.tsx

@@ -7,7 +7,7 @@
  */
 
 import React from 'react';
-import { render, mount } from 'enzyme';
+import { render, mount, shallow } from 'enzyme';
 import { requiredProps } from '../../test/required_props';
 
 import { EuiSuggest, EuiSuggestionProps } from './suggest';
@@ -138,6 +138,25 @@ describe('EuiSuggest', () => {
       });
     });
 
+    describe('onItemClick', () => {
+      it('passes an onChange callback to the underlying EuiSelectable, which will fire on list item clicks and enter keypresses', () => {
+        const onItemClick = jest.fn();
+        const component = shallow(
+          <EuiSuggest
+            {...requiredProps}
+            suggestions={sampleItems}
+            onItemClick={onItemClick}
+          />
+        );
+
+        const options: any = component.find('EuiSelectable').prop('options');
+        options[1].checked = 'on';
+        component.find('EuiSelectable').simulate('change', options);
+
+        expect(onItemClick).toHaveBeenCalledWith(sampleItems[1]);
+      });
+    });
+
     test('remaining EuiFieldSearch props are spread to the search input', () => {
       const component = render(
         <EuiSuggest

src/components/suggest/suggest.tsx

@@ -11,6 +11,7 @@ import React, {
   FormEvent,
   FunctionComponent,
   useState,
+  useCallback,
 } from 'react';
 import classNames from 'classnames';
 import { CommonProps, ExclusiveUnion } from '../common';
@@ -22,12 +23,12 @@ import { useEuiI18n } from '../i18n';
 import { EuiInputPopover } from '../popover';
 import {
   EuiSelectable,
-  EuiSelectableListItemProps,
+  EuiSelectableOption,
   EuiSelectableSearchableSearchProps,
 } from '../selectable';
 import { EuiToolTip } from '../tool_tip';
 
-import { EuiSuggestItem, _EuiSuggestItemPropsBase } from './suggest_item';
+import { EuiSuggestItem, EuiSuggestItemProps } from './suggest_item';
 import { EuiSuggestStatus, _EuiSuggestStatusMap } from './types';
 
 const statusMap: _EuiSuggestStatusMap = {
@@ -56,11 +57,7 @@ const suggestItemPropsKeys = [
   'descriptionDisplay',
 ];
 
-export interface EuiSuggestionProps
-  extends CommonProps,
-    _EuiSuggestItemPropsBase {
-  onClick?: EuiSelectableListItemProps['onClick'];
-}
+export type EuiSuggestionProps = CommonProps & EuiSuggestItemProps;
 
 type _EuiSuggestProps = CommonProps &
   Omit<
@@ -241,12 +238,7 @@ export const EuiSuggest: FunctionComponent<EuiSuggestProps> = ({
   /**
    * Options list
    */
-  const suggestionList = suggestions.map((item: EuiSuggestionProps) => {
-    const { className, ...props } = item;
-    if (onItemClick) {
-      props.onClick = () => onItemClick(item);
-    }
-
+  const suggestionList = suggestions.map((props: EuiSuggestionProps) => {
     // Omit props destined for the EuiSuggestItem so that they don't
     // cause warnings or render in the DOM of the EuiSelectableItem
     const data = {};
@@ -263,17 +255,26 @@ export const EuiSuggest: FunctionComponent<EuiSuggestProps> = ({
     return {
       ...(liProps as typeof props),
       data,
-      className: classNames(className, 'euiSuggestItemOption'),
+      className: classNames(props.className, 'euiSuggestItemOption'),
       // Force truncation if `isVirtualized` is true
       truncate: isVirtualized ? true : props.truncate,
     };
   });
 
-  const renderOption = (option: EuiSuggestionProps) => {
-    // `onClick` handled by EuiSelectable
-    const { onClick, ...props } = option;
-    return <EuiSuggestItem {...props} />;
-  };
+  const onItemSelect = useCallback(
+    (options: EuiSelectableOption[]) => {
+      if (onItemClick) {
+        const selectedIndex = options.findIndex(
+          (option) => option.checked === 'on'
+        );
+        if (selectedIndex >= 0) {
+          const selectedSuggestion = suggestions[selectedIndex];
+          onItemClick(selectedSuggestion);
+        }
+      }
+    },
+    [onItemClick, suggestions]
+  );
 
   const classes = classNames('euiInputPopover', {
     'euiInputPopover--fullWidth': fullWidth,
@@ -285,7 +286,8 @@ export const EuiSuggest: FunctionComponent<EuiSuggestProps> = ({
         singleSelection={true}
         height={isVirtualized ? undefined : 'full'}
         options={suggestionList}
-        renderOption={renderOption}
+        renderOption={EuiSuggestItem}
+        onChange={onItemSelect}
         listProps={{
           bordered: false,
           showIcons: false,