diff --git a/CHANGELOG.md b/CHANGELOG.md index 7620dd529df..72ce5c755ec 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/components/selectable/selectable.spec.tsx b/src/components/selectable/selectable.spec.tsx index 011190a2e9f..df467baf047 100644 --- a/src/components/selectable/selectable.spec.tsx +++ b/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( + + {(list, search) => ( + <> + {search} + {list} + + )} + + ); + + 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( + + {(list, search) => ( + <> + {search} + {list} + + )} + + ); + + 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( + + {(list, search) => ( + <> + {search} + {list} + + )} + + ); + + 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( + + {(list) => <>{list}} + + ); + + 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( + + {(list) => <>{list}} + + ); + 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' }, + ]); + }); + }); }); }); diff --git a/src/components/selectable/selectable.tsx b/src/components/selectable/selectable.tsx index b54c5f28826..8f98ccdeb86 100644 --- a/src/components/selectable/selectable.tsx +++ b/src/components/selectable/selectable.tsx @@ -305,22 +305,30 @@ export class EuiSelectable 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(); diff --git a/src/components/suggest/__snapshots__/suggest.test.tsx.snap b/src/components/suggest/__snapshots__/suggest.test.tsx.snap index 25136f20b66..fd5cf887998 100644 --- a/src/components/suggest/__snapshots__/suggest.test.tsx.snap +++ b/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 { diff --git a/src/components/suggest/suggest.spec.tsx b/src/components/suggest/suggest.spec.tsx index e59d520dcd8..c2dff16673d 100644 --- a/src/components/suggest/suggest.spec.tsx +++ b/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( + + ); + + 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', () => { diff --git a/src/components/suggest/suggest.test.tsx b/src/components/suggest/suggest.test.tsx index 2b2f7310475..79185c42195 100644 --- a/src/components/suggest/suggest.test.tsx +++ b/src/components/suggest/suggest.test.tsx @@ -7,9 +7,10 @@ */ import React from 'react'; -import { render, mount } from 'enzyme'; +import { render, mount, shallow } from 'enzyme'; import { requiredProps } from '../../test/required_props'; +import { EuiSelectable } from '../selectable'; import { EuiSuggest, EuiSuggestionProps } from './suggest'; import { ALL_STATUSES } from './types'; @@ -138,6 +139,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( + + ); + + const options = 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( = ({ /** * 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,30 @@ export const EuiSuggest: FunctionComponent = ({ 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; + const renderOption = useCallback((props: EuiSuggestionProps) => { return ; - }; + }, []); + + 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, @@ -286,6 +291,7 @@ export const EuiSuggest: FunctionComponent = ({ height={isVirtualized ? undefined : 'full'} options={suggestionList} renderOption={renderOption} + onChange={onItemSelect} listProps={{ bordered: false, showIcons: false,