From 04c5e99a56acae328d9878af74af84104c282ae4 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Feb 2019 11:48:39 -0700 Subject: [PATCH 1/9] Allow keyboard navigation of single select combobox --- src/components/combo_box/combo_box.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index f9dc262cea0..579d90397c3 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -94,9 +94,19 @@ export class EuiComboBox extends Component { } openList = () => { - this.setState({ - isListOpen: true, - }); + const { selectedOptions, singleSelection } = this.props; + const { matchingOptions } = this.state; + + const stateUpdate = { isListOpen: true }; + + // ensure that the currently selected single option is active if it is in the matchingOptions + if (singleSelection && selectedOptions.length === 1) { + if (matchingOptions.includes(selectedOptions[0])) { + stateUpdate.activeOptionIndex = matchingOptions.indexOf(selectedOptions[0]); + } + } + + this.setState(stateUpdate); }; closeList = () => { @@ -186,7 +196,7 @@ export class EuiComboBox extends Component { }; hasActiveOption = () => { - return this.state.activeOptionIndex != null; + return this.state.activeOptionIndex != null && this.state.activeOptionIndex < this.state.matchingOptions.length; }; clearActiveOption = () => { @@ -486,14 +496,6 @@ export class EuiComboBox extends Component { // Calculate and cache the options which match the searchValue, because we use this information // in multiple places and it would be expensive to calculate repeatedly. const matchingOptions = getMatchingOptions(options, selectedOptions, searchValue, nextProps.async, singleSelection); - // ensure that the currently selected single option is active if it is in the matchingOptions - if (singleSelection && selectedOptions.length === 1) { - let nextActiveOptionIndex; - if (matchingOptions.includes(selectedOptions[0])) { - nextActiveOptionIndex = matchingOptions.indexOf(selectedOptions[0]); - } - return { matchingOptions, activeOptionIndex: nextActiveOptionIndex }; - } return { matchingOptions }; } From 40b99c0feed903b8cb75f42b71925527f141ca2d Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Feb 2019 11:53:26 -0700 Subject: [PATCH 2/9] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b199ae81720..464d7874b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ **Bug fixes** +- Fixes keyboard navigation of `EuiComboBox` items in single selection mode ([#1619](https://github.com/elastic/eui/pull/1619)) - `EuiBasicTable` select all shows up on mobile ([#1462](https://github.com/elastic/eui/pull/1462)) - Adds missing `hasActiveFilters` prop for `EuiFilterButton` type and fixes `onChange` signature for `EuiButtonGroup` ([#1603](https://github.com/elastic/eui/pull/1603)) - Prevent `EuiGlobalToastList` from attempting calculations on `null` DOM elements ([#1606](https://github.com/elastic/eui/pull/1606)) From 5d8dce66ba9e72dbbb53e5f38d0ea4ee83c67e8b Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Feb 2019 12:39:27 -0700 Subject: [PATCH 3/9] Guard against invalid activeOptionIndex --- src/components/combo_box/combo_box.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 579d90397c3..6884b6600f5 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -491,13 +491,19 @@ export class EuiComboBox extends Component { static getDerivedStateFromProps(nextProps, prevState) { const { options, selectedOptions, singleSelection } = nextProps; - const { searchValue } = prevState; + const { activeOptionIndex, searchValue } = prevState; // Calculate and cache the options which match the searchValue, because we use this information // in multiple places and it would be expensive to calculate repeatedly. const matchingOptions = getMatchingOptions(options, selectedOptions, searchValue, nextProps.async, singleSelection); - return { matchingOptions }; + const stateUpdate = { matchingOptions }; + + if (activeOptionIndex >= matchingOptions.length) { + stateUpdate.activeOptionIndex = undefined; + } + + return stateUpdate; } updateMatchingOptionsIfDifferent(newMatchingOptions) { From b8dd4bebb02c3a46b104c5cd2a5f2efbaa41bc7d Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Feb 2019 14:03:58 -0700 Subject: [PATCH 4/9] Mark combobox as invalid if it loses focus and search value is present; hide selected item if user has search text entered and is a singleSelection box --- src/components/combo_box/combo_box.js | 8 ++++++-- .../combo_box/combo_box_input/combo_box_input.js | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 6884b6600f5..f7e9dc0bdde 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -74,6 +74,7 @@ export class EuiComboBox extends Component { isListOpen: false, listPosition: 'bottom', activeOptionIndex: undefined, + hasFocus: false, }; // ensure that the currently selected single option is active if it is in the matchingOptions @@ -287,6 +288,7 @@ export class EuiComboBox extends Component { this.props.onFocus(); } this.openList(); + this.setState({ hasFocus: true }); } onContainerBlur = (e) => { @@ -308,6 +310,7 @@ export class EuiComboBox extends Component { if (this.props.onBlur) { this.props.onBlur(); } + this.setState({ hasFocus: false }); } onKeyDown = (e) => { @@ -581,12 +584,13 @@ export class EuiComboBox extends Component { 'data-test-subj': dataTestSubj, ...rest } = this.props; + const { hasFocus, searchValue, isListOpen, listPosition, width, activeOptionIndex } = this.state; - const { searchValue, isListOpen, listPosition, width, activeOptionIndex } = this.state; + const markAsInvalid = isInvalid || (hasFocus === false && searchValue); const classes = classNames('euiComboBox', className, { 'euiComboBox-isOpen': isListOpen, - 'euiComboBox-isInvalid': isInvalid, + 'euiComboBox-isInvalid': markAsInvalid, 'euiComboBox-isDisabled': isDisabled, 'euiComboBox--fullWidth': fullWidth, 'euiComboBox--compressed': compressed, 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 995c09f3a62..2638722074c 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 @@ -215,7 +215,7 @@ export class EuiComboBoxInput extends Component { tabIndex="-1" // becomes onBlur event's relatedTarget, otherwise relatedTarget is null when clicking on this div data-test-subj="comboBoxInput" > - {pills} + {!singleSelection || !searchValue ? pills : null} {placeholderMessage} Date: Wed, 27 Feb 2019 15:47:40 -0700 Subject: [PATCH 5/9] Open combobox options list on search value change --- src/components/combo_box/combo_box.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index f7e9dc0bdde..ee1ec7fb0df 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -132,6 +132,12 @@ export class EuiComboBox extends Component { return; } + // it's possible that updateListPosition is called when listElement is becoming visible, but isn't yet + const listElementBounds = listElement.getBoundingClientRect(); + if (listElementBounds.width === 0 || listElementBounds.height === 0) { + return; + } + const comboBoxBounds = this.comboBox.getBoundingClientRect(); const { position, top } = findPopoverPosition({ @@ -437,7 +443,13 @@ export class EuiComboBox extends Component { if (this.props.onSearchChange) { this.props.onSearchChange(searchValue); } - this.setState({ searchValue }); + + this.setState( + { searchValue }, + () => { + if (this.state.isListOpen === false) this.openList(); + } + ); }; comboBoxRef = node => { From 3d38f206fda7eac07bd1b9c1d219815ab80f588c Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 27 Feb 2019 15:56:42 -0700 Subject: [PATCH 6/9] Allow tabbing off of singleSelection combo boxes when tabbing through form elements --- .../__snapshots__/combo_box.test.js.snap | 6 ++--- src/components/combo_box/combo_box.js | 23 +++---------------- 2 files changed, 5 insertions(+), 24 deletions(-) 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 0eabb11de26..40176609338 100644 --- a/src/components/combo_box/__snapshots__/combo_box.test.js.snap +++ b/src/components/combo_box/__snapshots__/combo_box.test.js.snap @@ -379,7 +379,7 @@ exports[`props singleSelection is rendered 1`] = ` { - const { selectedOptions, singleSelection } = this.props; - const { matchingOptions } = this.state; - - const stateUpdate = { isListOpen: true }; - - // ensure that the currently selected single option is active if it is in the matchingOptions - if (singleSelection && selectedOptions.length === 1) { - if (matchingOptions.includes(selectedOptions[0])) { - stateUpdate.activeOptionIndex = matchingOptions.indexOf(selectedOptions[0]); - } - } - - this.setState(stateUpdate); + this.setState({ + isListOpen: true, + }); }; closeList = () => { From fe68fd8d746eb91c9071b26f11deeff07a35a576 Mon Sep 17 00:00:00 2001 From: cchaos Date: Thu, 28 Feb 2019 10:07:26 -0500 Subject: [PATCH 7/9] Fix padding --- src/components/combo_box/_combo_box.scss | 5 +++-- .../combo_box/combo_box_input/_combo_box_pill.scss | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/combo_box/_combo_box.scss b/src/components/combo_box/_combo_box.scss index da7d5473358..5dc6d58d95d 100644 --- a/src/components/combo_box/_combo_box.scss +++ b/src/components/combo_box/_combo_box.scss @@ -17,9 +17,9 @@ @include euiFormControlStyle($includeStates: false, $includeSizes: false); @include euiFormControlWithIcon($isIconOptional: true); @include euiFormControlSize(auto, $includeAlternates: true); - padding: $euiSizeXS; + padding: $euiSizeXS $euiSizeS; // sass-lint:disable-block mixins-before-declarations - @include euiFormControlLayoutPadding(1); /* 2 */ // + @include euiFormControlLayoutPadding(1); /* 2 */ display: flex; /* 1 */ @@ -28,6 +28,7 @@ } &:not(.euiComboBox__inputWrap--noWrap) { + padding: $euiSizeXS; height: auto; /* 3 */ flex-wrap: wrap; /* 1 */ align-content: flex-start; diff --git a/src/components/combo_box/combo_box_input/_combo_box_pill.scss b/src/components/combo_box/combo_box_input/_combo_box_pill.scss index 867e5f47cf4..b45de89e028 100644 --- a/src/components/combo_box/combo_box_input/_combo_box_pill.scss +++ b/src/components/combo_box/combo_box_input/_combo_box_pill.scss @@ -11,7 +11,7 @@ line-height: $euiSizeL; font-size: $euiFontSizeS; - padding: 0 $euiSizeXS; + padding: 0; color: $euiTextColor; vertical-align: middle; display: inline-block; From cee925b6cd8c9cbdb943e977a0e4e0a2234d71ad Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 28 Feb 2019 08:59:30 -0700 Subject: [PATCH 8/9] Fix combo box not closing when selecting an option --- src/components/combo_box/combo_box.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index a6f6ee946c0..9a7126a3d07 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -430,7 +430,7 @@ export class EuiComboBox extends Component { this.setState( { searchValue }, () => { - if (this.state.isListOpen === false) this.openList(); + if (searchValue && this.state.isListOpen === false) this.openList(); } ); }; From a96112764ae6a99a3a0ddad0472732851c6a0b92 Mon Sep 17 00:00:00 2001 From: Caroline Horn <549577+cchaos@users.noreply.github.com> Date: Thu, 28 Feb 2019 09:36:50 -0700 Subject: [PATCH 9/9] Update CHANGELOG.md Co-Authored-By: chandlerprall --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 464d7874b84..8674e924797 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ **Bug fixes** -- Fixes keyboard navigation of `EuiComboBox` items in single selection mode ([#1619](https://github.com/elastic/eui/pull/1619)) +- Fixed keyboard navigation and UI of `EuiComboBox` items in single selection mode ([#1619](https://github.com/elastic/eui/pull/1619)) - `EuiBasicTable` select all shows up on mobile ([#1462](https://github.com/elastic/eui/pull/1462)) - Adds missing `hasActiveFilters` prop for `EuiFilterButton` type and fixes `onChange` signature for `EuiButtonGroup` ([#1603](https://github.com/elastic/eui/pull/1603)) - Prevent `EuiGlobalToastList` from attempting calculations on `null` DOM elements ([#1606](https://github.com/elastic/eui/pull/1606))