diff --git a/CHANGELOG.md b/CHANGELOG.md
index 353f93b2487..9fad37f6263 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,12 @@
## [`master`](https://github.com/elastic/eui/tree/master)
- Added `apmTrace` icon to `EuiIcon` set ([#1263](https://github.com/elastic/eui/pull/1263))
+- Added `compressed` prop to `EuiComboBox` ([#1258](https://github.com/elastic/eui/pull/1258))
-No public interface changes since `4.6.1`.
+**Bug fixes**
+
+- `EuiComboBox` no longer throws a _Maximum update depth exceeded_ error when used in popovers/modals ([#1258](https://github.com/elastic/eui/pull/1258))
+- `Escape` key now closes `EuiComboBox` options list ([#1258](https://github.com/elastic/eui/pull/1258))
## [`4.6.1`](https://github.com/elastic/eui/tree/v4.6.1)
diff --git a/src-docs/src/views/combo_box/combo_box_example.js b/src-docs/src/views/combo_box/combo_box_example.js
index 9ae64d90d9d..0a94b03ec21 100644
--- a/src-docs/src/views/combo_box/combo_box_example.js
+++ b/src-docs/src/views/combo_box/combo_box_example.js
@@ -11,6 +11,7 @@ import {
} from '../../components';
import {
+ EuiLink,
EuiCallOut,
EuiCode,
EuiComboBox,
@@ -129,7 +130,7 @@ export const ComboBoxExample = {
}],
text: (
- EuiComboBoxList uses react-virtualized{' '}
+ EuiComboBoxList uses react-virtualized{' '}
to only render visible options to be super fast no matter how many options there are.
@@ -287,16 +325,22 @@ exports[`props options list is rendered 1`] = `
exports[`props selectedOptions are rendered 1`] = `
{
- if (![-1, 1].includes(amount)) {
- throw new Error(`tabAway expects amount to be -1 or 1, but received ${amount}`);
- }
-
- const tabbableItems = tabbable(document);
-
- if (document.activeElement === this.searchInput) {
- const searchInputIndex = tabbableItems.indexOf(this.searchInput);
-
- // Wrap to last tabbable if tabbing backwards.
- if (amount === -1) {
- if (searchInputIndex === 0) {
- tabbableItems[tabbableItems.length - 1].focus();
- return true;
- }
- }
-
- // Otherwise tab to the next adjacent item.
- tabbableItems[searchInputIndex + amount].focus();
- return true;
- }
-
- if (document.activeElement === this.toggleButton) {
- const toggleButtonIndex = tabbableItems.indexOf(this.toggleButton);
-
- // Wrap to first tabbable if tabbing forwards.
- if (amount === 1) {
- if (toggleButtonIndex === tabbableItems.length - 1) {
- tabbableItems[0].focus();
- return true;
- }
- }
-
- // Otherwise tab to the next adjacent item.
- tabbableItems[toggleButtonIndex + amount].focus();
- return true;
- }
-
- // Tab natively.
- return false;
- };
-
- incrementActiveOptionIndex = throttle(amount => {
- // If there are no options available, reset the focus.
+ incrementActiveOptionIndex = amount => {
+ // If there are no options available, do nothing.
if (!this.state.matchingOptions.length) {
- this.clearActiveOption();
return;
}
this.setState(({ activeOptionIndex, matchingOptions }) => {
let nextActiveOptionIndex;
- if (!this.hasActiveOption()) {
+ if (activeOptionIndex == null) {
// If this is the beginning of the user's keyboard navigation of the menu, then we'll focus
// either the first or last item.
nextActiveOptionIndex = amount < 0 ? matchingOptions.length - 1 : 0;
@@ -215,10 +173,10 @@ export class EuiComboBox extends Component {
return { activeOptionIndex: nextActiveOptionIndex };
});
- }, 200);
+ };
hasActiveOption = () => {
- return this.state.activeOptionIndex !== undefined;
+ return this.state.activeOptionIndex != null;
};
clearActiveOption = () => {
@@ -227,27 +185,11 @@ export class EuiComboBox extends Component {
});
};
- focusActiveOption = () => {
- // If an item is focused, focus it.
- if (this.hasActiveOption() && this.options[this.state.activeOptionIndex]) {
- this.options[this.state.activeOptionIndex].focus();
- }
- };
-
- focusSearchInput = () => {
- this.clearActiveOption();
- this.searchInput.focus();
- };
-
clearSearchValue = () => {
this.onSearchChange('');
};
removeLastOption = () => {
- if (this.hasActiveOption()) {
- return;
- }
-
if (!this.props.selectedOptions.length) {
return;
}
@@ -263,7 +205,7 @@ export class EuiComboBox extends Component {
addCustomOption = () => {
if (this.doesSearchMatchOnlyOption()) {
- this.options[0].click();
+ this.onAddOption(this.state.matchingOptions[0]);
return;
}
@@ -277,16 +219,14 @@ export class EuiComboBox extends Component {
}
// Add new custom pill if this is custom input, even if it partially matches an option..
- if (!this.hasActiveOption() || this.doesSearchMatchOnlyOption()) {
- const isOptionCreated = this.props.onCreateOption(this.state.searchValue, flattenOptionGroups(this.props.options));
+ const isOptionCreated = this.props.onCreateOption(this.state.searchValue, flattenOptionGroups(this.props.options));
- // Expect the consumer to be explicit in rejecting a custom option.
- if (isOptionCreated === false) {
- return;
- }
-
- this.clearSearchValue();
+ // Expect the consumer to be explicit in rejecting a custom option.
+ if (isOptionCreated === false) {
+ return;
}
+
+ this.clearSearchValue();
};
doesSearchMatchOnlyOption = () => {
@@ -307,63 +247,67 @@ export class EuiComboBox extends Component {
};
onFocus = () => {
- document.addEventListener('click', this.onDocumentFocusChange);
- document.addEventListener('focusin', this.onDocumentFocusChange);
this.openList();
}
- onBlur = () => {
- document.removeEventListener('click', this.onDocumentFocusChange);
- document.removeEventListener('focusin', this.onDocumentFocusChange);
- this.closeList();
- }
-
- onDocumentFocusChange = event => {
- // Close the list if the combo box has lost focus.
- if (
- this.comboBox === event.target
- || this.comboBox.contains(event.target)
- || this.optionsList === event.target
- || this.optionsList && this.optionsList.contains(event.target)
- ) {
- return;
+ onBlur = (e) => {
+ // close the options list, unless the use clicked on an option
+ const focusedInOptionsList = this.optionsList && this.optionsList.contains(e.relatedTarget);
+ const focusedInInput = this.comboBox && this.comboBox.contains(e.relatedTarget);
+ if (!focusedInOptionsList && !focusedInInput) {
+ this.closeList();
}
- // Wait for the DOM to update.
- requestAnimationFrame(() => {
- if (document.activeElement === this.searchInput) {
- return;
- }
+ if (this.props.onBlur) {
+ this.props.onBlur(e);
+ }
+ }
- this.onBlur();
- });
- };
+ onComboBoxLoseFocus = e => {
+ if (!this.comboBox || !this.comboBox.contains(e.relatedTarget)) {
+ this.closeList();
+ }
+ }
onKeyDown = (e) => {
switch (e.keyCode) {
case comboBoxKeyCodes.UP:
e.preventDefault();
- this.incrementActiveOptionIndex(-1);
+ e.stopPropagation();
+ if (this.state.isListOpen) {
+ this.incrementActiveOptionIndex(-1);
+ } else {
+ this.openList();
+ }
break;
case comboBoxKeyCodes.DOWN:
e.preventDefault();
- this.incrementActiveOptionIndex(1);
+ e.stopPropagation();
+ if (this.state.isListOpen) {
+ this.incrementActiveOptionIndex(1);
+ } else {
+ this.openList();
+ }
break;
case BACKSPACE:
+ e.stopPropagation();
this.removeLastOption();
break;
case ESCAPE:
- // Move focus from options list to input.
- if (this.hasActiveOption()) {
- this.focusSearchInput();
- }
+ e.stopPropagation();
+ this.closeList();
break;
case comboBoxKeyCodes.ENTER:
- this.addCustomOption();
+ e.stopPropagation();
+ if (this.hasActiveOption()) {
+ this.onAddOption(this.state.matchingOptions[this.state.activeOptionIndex]);
+ } else {
+ this.addCustomOption();
+ }
break;
case TAB:
@@ -371,15 +315,13 @@ export class EuiComboBox extends Component {
if (this.hasActiveOption()) {
e.preventDefault();
e.stopPropagation();
- break;
}
+ break;
- const amount = e.shiftKey ? -1 : 1;
- if (this.tabAway(amount)) {
- e.preventDefault();
- e.stopPropagation();
+ default:
+ if (this.props.onKeyDown) {
+ this.props.onKeyDown(e);
}
- break;
}
};
@@ -392,6 +334,10 @@ export class EuiComboBox extends Component {
}
onAddOption = (addedOption) => {
+ if (addedOption.disabled) {
+ return;
+ }
+
const { onChange, selectedOptions, singleSelection } = this.props;
onChange(singleSelection ? [addedOption] : selectedOptions.concat(addedOption));
@@ -399,16 +345,19 @@ export class EuiComboBox extends Component {
if (singleSelection) {
this.closeList();
+ this.searchInput.blur();
return;
}
- this.focusSearchInput();
+ this.clearActiveOption();
+ this.searchInput.focus();
};
onRemoveOption = (removedOption) => {
const { onChange, selectedOptions } = this.props;
onChange(selectedOptions.filter(option => option !== removedOption));
- this.focusSearchInput();
+
+ this.clearActiveOption();
};
clearSelectedOptions = () => {
@@ -425,23 +374,6 @@ export class EuiComboBox extends Component {
this.clearActiveOption();
};
- onComboBoxFocus = (e) => {
- // If the user has tabbed to the combo box, open it.
- if (e.target === this.searchInput) {
- this.searchInput.focus();
- return;
- }
-
- // If a user clicks on an option without selecting it, then it will take focus
- // and we need to update the index.
- const optionIndex = this.options.indexOf(e.target);
- if (optionIndex !== -1) {
- this.setState({
- activeOptionIndex: optionIndex,
- });
- }
- };
-
onOpenListClick = () => {
this.searchInput.focus();
};
@@ -531,7 +463,7 @@ export class EuiComboBox extends Component {
if (!newMatchingOptions.length) {
// Prevent endless setState -> componentWillUpdate -> setState loop.
- if (this.state.hasActiveOption) {
+ if (this.hasActiveOption()) {
this.clearActiveOption();
}
}
@@ -546,15 +478,10 @@ export class EuiComboBox extends Component {
// isn't called after a state change, and we track `searchValue` in state
// instead we need to react to a change in searchValue here
this.updateMatchingOptionsIfDifferent(getMatchingOptions(options, selectedOptions, searchValue, this.props.async));
-
- this.focusActiveOption();
}
componentWillUnmount() {
- this.incrementActiveOptionIndex.cancel();
this._isMounted = false;
- document.removeEventListener('click', this.onDocumentFocusChange);
- document.removeEventListener('focusin', this.onDocumentFocusChange);
}
render() {
@@ -577,6 +504,7 @@ export class EuiComboBox extends Component {
rowHeight,
isClearable,
fullWidth,
+ compressed,
'data-test-subj': dataTestSubj,
...rest
} = this.props;
@@ -588,6 +516,7 @@ export class EuiComboBox extends Component {
'euiComboBox-isInvalid': isInvalid,
'euiComboBox-isDisabled': isDisabled,
'euiComboBox--fullWidth': fullWidth,
+ 'euiComboBox--compressed': compressed,
});
const value = selectedOptions.map(selectedOption => selectedOption.label).join(', ');
@@ -596,6 +525,7 @@ export class EuiComboBox extends Component {
if (!noSuggestions && isListOpen) {
const optionsListDataTestSubj = dataTestSubj ? `${dataTestSubj}-optionsList` : undefined;
+
optionsList = (
);
@@ -627,12 +558,15 @@ export class EuiComboBox extends Component {
return (
{optionsList}
diff --git a/src/components/combo_box/combo_box.test.js b/src/components/combo_box/combo_box.test.js
index cb8d5f4fe72..da2a636adf5 100644
--- a/src/components/combo_box/combo_box.test.js
+++ b/src/components/combo_box/combo_box.test.js
@@ -6,27 +6,6 @@ import { comboBoxKeyCodes } from '../../services';
import { EuiComboBox } from './combo_box';
-let hasComboBoxLostFocus = false;
-
-// This module requires a browser environment, so we'll fake what it returns.
-jest.mock('tabbable', () => () => {
- const proxy = new Proxy([], {
- get: (target, prop) => {
- // If the consumer is trying to access an element in the array, e.g. [-2], pretend there's
- // a focusable element there. When it's focused, we're going to assume this means that
- // the combo box has lost focus.
- if (prop.match(/-?\d+/)) {
- return {
- focus: () => { hasComboBoxLostFocus = true; },
- };
- } else {
- return target[prop];
- }
- },
- });
- return proxy;
-});
-
jest.mock(
'../portal',
() => ({
@@ -34,9 +13,10 @@ jest.mock(
})
);
-beforeEach(() => {
- hasComboBoxLostFocus = false;
-});
+// Mock the htmlIdGenerator to generate predictable ids for snapshot tests
+jest.mock('../../services/accessibility/html_id_generator', () => ({
+ htmlIdGenerator: () => { return (suffix) => `htmlid_${suffix}`; },
+}));
const options = [{
label: 'Titan',
@@ -157,11 +137,14 @@ describe('props', () => {
describe('behavior', () => {
describe('tabbing', () => {
test(`off the search input closes the options list if the user isn't navigating the options`, () => {
+ const onKeyDownWrapper = jest.fn();
const component = mount(
-
+
+
+
);
const searchInput = findTestSubject(component, 'comboBoxSearchInput');
@@ -173,16 +156,19 @@ describe('behavior', () => {
// Tab backwards to take focus off the combo box.
searchInput.simulate('keyDown', { keyCode: comboBoxKeyCodes.TAB, shiftKey: true });
- // Losing focus will close the options list.
- expect(hasComboBoxLostFocus).toBe(true);
+ // If the TAB keydown propagated to the wrapper, then a browser DOM would shift the focus
+ expect(onKeyDownWrapper.mock.calls.length).toBe(1);
});
test('off the search input does nothing if the user is navigating the options', () => {
+ const onKeyDownWrapper = jest.fn();
const component = mount(
-
+