Skip to content

Commit

Permalink
Allow keyboard navigation of single select combobox (elastic#1619)
Browse files Browse the repository at this point in the history
* Allow keyboard navigation of single select combobox

* changelog

* Guard against invalid activeOptionIndex

* 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

* Open combobox options list on search value change

* Allow tabbing off of singleSelection combo boxes when tabbing through form elements

* Fix padding

* Fix combo box not closing when selecting an option

* Update CHANGELOG.md
  • Loading branch information
chandlerprall committed Mar 14, 2019
1 parent 552f371 commit 6c286ea
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 29 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
## [`master`](https://github.com/elastic/eui/tree/master)

No public interface changes since `6.10.4`.
**Note: this release is a backport containing changes originally made in `9.0.0`**

**Bug fixes**

- Fixed keyboard navigation and UI of `EuiComboBox` items in single selection mode ([#1619](https://github.com/elastic/eui/pull/1619))

## [`6.10.4`](https://github.com/elastic/eui/tree/v6.10.4)

Expand Down
6 changes: 2 additions & 4 deletions src/components/combo_box/__snapshots__/combo_box.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ exports[`props singleSelection is rendered 1`] = `
<EuiComboBoxInput
autoSizeInputRef={[Function]}
compressed={false}
focusedOptionId="htmlid__option-2"
focusedOptionId={null}
fullWidth={false}
hasSelectedOptions={true}
inputRef={[Function]}
Expand Down Expand Up @@ -421,7 +421,7 @@ exports[`props singleSelection selects existing option when opened 1`] = `
<EuiComboBoxInput
autoSizeInputRef={[Function]}
compressed={false}
focusedOptionId="htmlid__option-2"
focusedOptionId={null}
fullWidth={false}
hasSelectedOptions={true}
inputRef={[Function]}
Expand Down Expand Up @@ -451,7 +451,6 @@ exports[`props singleSelection selects existing option when opened 1`] = `
/>
<EuiPortal>
<EuiComboBoxOptionsList
activeOptionIndex={2}
areAllOptionsSelected={false}
data-test-subj=""
fullWidth={false}
Expand Down Expand Up @@ -533,7 +532,6 @@ exports[`props singleSelection selects existing option when opened 1`] = `
position="bottom"
rootId={[Function]}
rowHeight={27}
scrollToIndex={2}
searchValue=""
selectedOptions={
Array [
Expand Down
5 changes: 3 additions & 2 deletions src/components/combo_box/_combo_box.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -28,6 +28,7 @@
}

&:not(.euiComboBox__inputWrap--noWrap) {
padding: $euiSizeXS;
height: auto; /* 3 */
flex-wrap: wrap; /* 1 */
align-content: flex-start;
Expand Down
47 changes: 27 additions & 20 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,9 @@ 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
if (singleSelection && selectedOptions.length === 1) {
if (this.state.matchingOptions.includes(selectedOptions[0])) {
this.state.activeOptionIndex = this.state.matchingOptions.indexOf(selectedOptions[0]);
}
}

this.rootId = htmlIdGenerator();

// Refs.
Expand Down Expand Up @@ -121,6 +115,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({
Expand Down Expand Up @@ -186,7 +186,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 = () => {
Expand Down Expand Up @@ -277,6 +277,7 @@ export class EuiComboBox extends Component {
this.props.onFocus();
}
this.openList();
this.setState({ hasFocus: true });
}

onContainerBlur = (e) => {
Expand All @@ -298,6 +299,7 @@ export class EuiComboBox extends Component {
if (this.props.onBlur) {
this.props.onBlur();
}
this.setState({ hasFocus: false });
}

onKeyDown = (e) => {
Expand Down Expand Up @@ -424,7 +426,13 @@ export class EuiComboBox extends Component {
if (this.props.onSearchChange) {
this.props.onSearchChange(searchValue);
}
this.setState({ searchValue });

this.setState(
{ searchValue },
() => {
if (searchValue && this.state.isListOpen === false) this.openList();
}
);
};

comboBoxRef = node => {
Expand Down Expand Up @@ -481,21 +489,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);
// 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 };

const stateUpdate = { matchingOptions };

if (activeOptionIndex >= matchingOptions.length) {
stateUpdate.activeOptionIndex = undefined;
}

return { matchingOptions };
return stateUpdate;
}

updateMatchingOptionsIfDifferent(newMatchingOptions) {
Expand Down Expand Up @@ -573,12 +579,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

line-height: $euiSizeL;
font-size: $euiFontSizeS;
padding: 0 $euiSizeXS;
padding: 0;
color: $euiTextColor;
vertical-align: middle;
display: inline-block;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
<AutosizeInput
role="textbox"
Expand Down

0 comments on commit 6c286ea

Please sign in to comment.