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

Allow keyboard navigation of single select combobox #1619

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

**Bug fixes**

- Fixes keyboard navigation of `EuiComboBox` items in single selection mode ([#1619](https://github.com/elastic/eui/pull/1619))
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
- `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))
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
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 (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 @@ -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