From 23485362d25d9be81f8d34c94d67a812077db91e Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 11 Jun 2018 10:38:11 -0700 Subject: [PATCH] * EuiComboBox now gives focus to the search input when the user clicks the clear button, to prevent focus from defaulting to the body. * EuiComboBox is now decorated with data-test-subj selectors for the search input, toggle list button, and clear button. * Updated snapshots. --- CHANGELOG.md | 3 + .../__snapshots__/combo_box.test.js.snap | 123 ++++++++++-------- src/components/combo_box/combo_box.js | 3 + src/components/combo_box/combo_box.test.js | 65 ++++++--- .../combo_box_input/combo_box_input.js | 5 +- 5 files changed, 128 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b401a4b7f8..0112479698e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## [`master`](https://github.com/elastic/eui/tree/master) +- `EuiComboBox` is now decorated with `data-test-subj` selectors for the search input (`comboxBoxSearchInput`), toggle button (`comboBoxToggleListButton`), and clear button (`comboBoxClearButton`) ([#918](https://github.com/elastic/eui/pull/918)) +- `EuiComboBox` now gives focus to the search input when the user clicks the clear button, to prevent focus from defaulting to the body ([#918](https://github.com/elastic/eui/pull/918)) + **Bug fixes** - Added `role="dialog"` to `EuiFlyout` to improve screen reader accessibility ([#916](https://github.com/elastic/eui/pull/916)) diff --git a/src/components/combo_box/__snapshots__/combo_box.test.js.snap b/src/components/combo_box/__snapshots__/combo_box.test.js.snap index 0d50dc5a6ad..61d5b2c3973 100644 --- a/src/components/combo_box/__snapshots__/combo_box.test.js.snap +++ b/src/components/combo_box/__snapshots__/combo_box.test.js.snap @@ -3,8 +3,69 @@ exports[`EuiComboBox is rendered 1`] = `
+
+
+
+ +
+
+
+
+ +
+
+
+`; + +exports[`props isClearable=false disallows user from clearing input when no options are selected 1`] = ` +
@@ -14,7 +75,6 @@ exports[`EuiComboBox is rendered 1`] = ` inputRef={[Function]} isListOpen={false} onChange={[Function]} - onClear={[Function]} onClick={[Function]} onCloseListClick={[Function]} onFocus={[Function]} @@ -30,11 +90,9 @@ exports[`EuiComboBox is rendered 1`] = `
`; -exports[`props isClearable is false with selectedOptions 1`] = ` +exports[`props isClearable=false disallows user from clearing input when options are selected 1`] = `
@@ -68,40 +126,9 @@ exports[`props isClearable is false with selectedOptions 1`] = `
`; -exports[`props isClearable is false without selectedOptions 1`] = ` +exports[`props isDisabled is rendered 1`] = `
- -
-`; - -exports[`props isDisabled 1`] = ` -
@@ -133,11 +160,9 @@ exports[`props isDisabled 1`] = `
`; -exports[`props options 1`] = ` +exports[`props options are rendered 1`] = `
@@ -163,11 +188,9 @@ exports[`props options 1`] = `
`; -exports[`props selectedOptions 1`] = ` +exports[`props selectedOptions are rendered 1`] = `
@@ -202,11 +225,9 @@ exports[`props selectedOptions 1`] = `
`; -exports[`props singleSelection 1`] = ` +exports[`props singleSelection is rendered 1`] = `
diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 94bc1509a5c..4de12691c19 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -401,6 +401,9 @@ export class EuiComboBox extends Component { clearSelectedOptions = () => { this.props.onChange([]); + // Clicking the clear button will also cause it to disappear. This would result in focus + // shifting unexpectedly to the body element so we set it to the input which is more reasonable, + this.searchInput.focus(); } onComboBoxClick = () => { diff --git a/src/components/combo_box/combo_box.test.js b/src/components/combo_box/combo_box.test.js index f1a0c3cc5d4..63797c23bd7 100644 --- a/src/components/combo_box/combo_box.test.js +++ b/src/components/combo_box/combo_box.test.js @@ -1,6 +1,7 @@ import React from 'react'; -import { shallow } from 'enzyme'; -import { requiredProps } from '../../test'; +import { shallow, render, mount } from 'enzyme'; +import sinon from 'sinon'; +import { requiredProps, findTestSubject } from '../../test'; import { EuiComboBox } from './combo_box'; @@ -29,7 +30,7 @@ const options = [{ describe('EuiComboBox', () => { test('is rendered', () => { - const component = shallow( + const component = render( ); @@ -38,49 +39,45 @@ describe('EuiComboBox', () => { }); describe('props', () => { - test('options', () => { + test('options are rendered', () => { + // NOTE: It's tough to test this because the dropdown containing the options opens up in + // a portal. const component = shallow( - + ); expect(component).toMatchSnapshot(); }); - test('selectedOptions', () => { + test('selectedOptions are rendered', () => { const component = shallow( ); expect(component).toMatchSnapshot(); }); - describe('isClearable is false', () => { - test('without selectedOptions', () => { + describe('isClearable=false disallows user from clearing input', () => { + test('when no options are selected', () => { const component = shallow( ); expect(component).toMatchSnapshot(); }); - test('with selectedOptions', () => { + test('when options are selected', () => { const component = shallow( ); @@ -88,29 +85,59 @@ describe('props', () => { }); }); - test('singleSelection', () => { + test('singleSelection is rendered', () => { const component = shallow( ); expect(component).toMatchSnapshot(); }); - test('isDisabled', () => { + test('isDisabled is rendered', () => { const component = shallow( ); expect(component).toMatchSnapshot(); }); }); + +describe('behavior', () => { + describe('clear button', () => { + test('calls onChange callback with empty array', () => { + const onChangeHandler = sinon.spy(); + const component = mount( + + ); + + findTestSubject(component, 'comboBoxClearButton').simulate('click'); + sinon.assert.calledOnce(onChangeHandler); + sinon.assert.calledWith(onChangeHandler, []); + }); + + test('focuses the input', () => { + const component = mount( + {}} + /> + ); + + findTestSubject(component, 'comboBoxClearButton').simulate('click'); + expect(findTestSubject(component, 'comboBoxSearchInput').getDOMNode()).toBe(document.activeElement); + }); + }); +}); diff --git a/src/components/combo_box/combo_box_input/combo_box_input.js b/src/components/combo_box/combo_box_input/combo_box_input.js index 80612d61843..335bb752cc4 100644 --- a/src/components/combo_box/combo_box_input/combo_box_input.js +++ b/src/components/combo_box/combo_box_input/combo_box_input.js @@ -152,6 +152,7 @@ export class EuiComboBoxInput extends Component { if (!isDisabled && onClear && hasSelectedOptions) { clickProps.clear = { onClick: onClear, + 'data-test-subj': 'comboBoxClearButton', }; } @@ -161,7 +162,8 @@ export class EuiComboBoxInput extends Component { onClick: isListOpen && !isDisabled ? onCloseListClick : onOpenListClick, ref: toggleButtonRef, 'aria-label': isListOpen ? 'Close list of options' : 'Open list of options', - disabled: isDisabled + disabled: isDisabled, + 'data-test-subj': 'comboBoxToggleListButton', }; return ( @@ -188,6 +190,7 @@ export class EuiComboBoxInput extends Component { ref={autoSizeInputRef} inputRef={inputRef} disabled={isDisabled} + data-test-subj="comboBoxSearchInput" /> {removeOptionMessage}