Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EuiComboBox now gives focus to the search input when the user clicks the clear button #918

Merged
merged 1 commit into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
123 changes: 72 additions & 51 deletions src/components/combo_box/__snapshots__/combo_box.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,69 @@
exports[`EuiComboBox is rendered 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
class="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
>
<div
class="euiFormControlLayout"
>
<div
class="euiComboBox__inputWrap"
data-test-subj="comboBoxInput"
>
<div
class="euiComboBox__input"
style="font-size:14px;display:inline-block"
>
<input
aria-hidden="true"
data-test-subj="comboBoxSearchInput"
style="box-sizing:content-box;width:1px"
value=""
/>
<div
style="position:absolute;top:0;left:0;visibility:hidden;height:0;overflow:scroll;white-space:pre"
/>
</div>
</div>
<div
class="euiFormControlLayoutIcons euiFormControlLayoutIcons--right"
>
<button
aria-label="Open list of options"
class="euiFormControlLayoutCustomIcon euiFormControlLayoutCustomIcon--clickable"
data-test-subj="comboBoxToggleListButton"
>
<svg
aria-hidden="true"
class="euiIcon euiIcon--medium euiFormControlLayoutCustomIcon__icon"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xlink="http://www.w3.org/1999/xlink"
xmlns="http://www.w3.org/2000/svg"
>
<defs>
<path
d="M13.069 5.157L8.384 9.768a.546.546 0 0 1-.768 0L2.93 5.158a.552.552 0 0 0-.771 0 .53.53 0 0 0 0 .759l4.684 4.61c.641.631 1.672.63 2.312 0l4.684-4.61a.53.53 0 0 0 0-.76.552.552 0 0 0-.771 0z"
id="arrow_down-a"
/>
</defs>
<use
fill-rule="nonzero"
href="#arrow_down-a"
/>
</svg>
</button>
</div>
</div>
</div>
`;

exports[`props isClearable=false disallows user from clearing input when no options are selected 1`] = `
<div
className="euiComboBox"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand All @@ -14,7 +75,6 @@ exports[`EuiComboBox is rendered 1`] = `
inputRef={[Function]}
isListOpen={false}
onChange={[Function]}
onClear={[Function]}
onClick={[Function]}
onCloseListClick={[Function]}
onFocus={[Function]}
Expand All @@ -30,11 +90,9 @@ exports[`EuiComboBox is rendered 1`] = `
</div>
`;

exports[`props isClearable is false with selectedOptions 1`] = `
exports[`props isClearable=false disallows user from clearing input when options are selected 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
className="euiComboBox"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand Down Expand Up @@ -68,40 +126,9 @@ exports[`props isClearable is false with selectedOptions 1`] = `
</div>
`;

exports[`props isClearable is false without selectedOptions 1`] = `
exports[`props isDisabled is rendered 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
onFocus={[Function]}
onKeyDown={[Function]}
>
<EuiComboBoxInput
autoSizeInputRef={[Function]}
hasSelectedOptions={false}
inputRef={[Function]}
isListOpen={false}
onChange={[Function]}
onClick={[Function]}
onCloseListClick={[Function]}
onFocus={[Function]}
onOpenListClick={[Function]}
onRemoveOption={[Function]}
searchValue=""
selectedOptions={Array []}
singleSelection={false}
toggleButtonRef={[Function]}
updatePosition={[Function]}
value=""
/>
</div>
`;

exports[`props isDisabled 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2 euiComboBox-isDisabled"
data-test-subj="test subject string"
className="euiComboBox euiComboBox-isDisabled"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand Down Expand Up @@ -133,11 +160,9 @@ exports[`props isDisabled 1`] = `
</div>
`;

exports[`props options 1`] = `
exports[`props options are rendered 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
className="euiComboBox"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand All @@ -163,11 +188,9 @@ exports[`props options 1`] = `
</div>
`;

exports[`props selectedOptions 1`] = `
exports[`props selectedOptions are rendered 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
className="euiComboBox"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand Down Expand Up @@ -202,11 +225,9 @@ exports[`props selectedOptions 1`] = `
</div>
`;

exports[`props singleSelection 1`] = `
exports[`props singleSelection is rendered 1`] = `
<div
aria-label="aria-label"
className="euiComboBox testClass1 testClass2"
data-test-subj="test subject string"
className="euiComboBox"
onFocus={[Function]}
onKeyDown={[Function]}
>
Expand Down
3 changes: 3 additions & 0 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
65 changes: 46 additions & 19 deletions src/components/combo_box/combo_box.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -29,7 +30,7 @@ const options = [{

describe('EuiComboBox', () => {
test('is rendered', () => {
const component = shallow(
const component = render(
<EuiComboBox {...requiredProps} />
);

Expand All @@ -38,79 +39,105 @@ 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(
<EuiComboBox
options={options}
{...requiredProps}
/>
<EuiComboBox options={options} />
);

expect(component).toMatchSnapshot();
});

test('selectedOptions', () => {
test('selectedOptions are rendered', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2], options[4]]}
{...requiredProps}
/>
);

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(
<EuiComboBox
options={options}
isClearable={false}
{...requiredProps}
/>
);

expect(component).toMatchSnapshot();
});

test('with selectedOptions', () => {
test('when options are selected', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2], options[4]]}
isClearable={false}
{...requiredProps}
/>
);

expect(component).toMatchSnapshot();
});
});

test('singleSelection', () => {
test('singleSelection is rendered', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
singleSelection={true}
{...requiredProps}
/>
);

expect(component).toMatchSnapshot();
});

test('isDisabled', () => {
test('isDisabled is rendered', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
isDisabled={true}
{...requiredProps}
/>
);

expect(component).toMatchSnapshot();
});
});

describe('behavior', () => {
describe('clear button', () => {
test('calls onChange callback with empty array', () => {
const onChangeHandler = sinon.spy();
const component = mount(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
onChange={onChangeHandler}
/>
);

findTestSubject(component, 'comboBoxClearButton').simulate('click');
sinon.assert.calledOnce(onChangeHandler);
sinon.assert.calledWith(onChangeHandler, []);
});

test('focuses the input', () => {
const component = mount(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
onChange={() => {}}
/>
);

findTestSubject(component, 'comboBoxClearButton').simulate('click');
expect(findTestSubject(component, 'comboBoxSearchInput').getDOMNode()).toBe(document.activeElement);
});
});
});
5 changes: 4 additions & 1 deletion src/components/combo_box/combo_box_input/combo_box_input.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export class EuiComboBoxInput extends Component {
if (!isDisabled && onClear && hasSelectedOptions) {
clickProps.clear = {
onClick: onClear,
'data-test-subj': 'comboBoxClearButton',
};
}

Expand All @@ -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 (
Expand All @@ -188,6 +190,7 @@ export class EuiComboBoxInput extends Component {
ref={autoSizeInputRef}
inputRef={inputRef}
disabled={isDisabled}
data-test-subj="comboBoxSearchInput"
/>
{removeOptionMessage}
</div>
Expand Down