diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b1a4b321035..f31fb768d316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,12 @@ ## [`master`](https://github.com/elastic/eui/tree/master) +**Breaking changes** + +- `EuiFormControlLayout` no longer has `onClear`, `iconSide`, or `onIconClick` props. Instead of `onClear` it now accepts a `clear` object of the shape `{ onClick }`. Instead of the icon props, it now accepts a single `icon` prop which be either a string or an object of the shape `{ type, side, onClick }`. ([#866](https://github.com/elastic/eui/pull/866)) + **Bug fixes** +- `EuiComboBox` is no longer a focus trap and the clear button is now keyboard-accessible ([#866](https://github.com/elastic/eui/pull/866)) - `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon` now look and behave disabled when `isDisabled={true}` ([#862](https://github.com/elastic/eui/pull/862)) - `EuiGlobalToastList` no longer triggers `Uncaught TypeError: _this.callback is not a function` ([#865](https://github.com/elastic/eui/pull/865)) - `EuiGlobalToastList` checks to see if it has dismissed a toast before re-dismissing it ([#868](https://github.com/elastic/eui/pull/868)) 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 9d79a33edc78..c4dfbe24c958 100644 --- a/src/components/combo_box/__snapshots__/combo_box.test.js.snap +++ b/src/components/combo_box/__snapshots__/combo_box.test.js.snap @@ -10,6 +10,7 @@ exports[`EuiComboBox is rendered 1`] = ` > { const tabbableItems = tabbable(document); - const comboBoxIndex = tabbableItems.indexOf(this.searchInput); - // Wrap to last tabbable if tabbing backwards. - if (amount < 0) { - if (comboBoxIndex === 0) { - tabbableItems[tabbableItems.length - 1].focus(); - return; + if (document.activeElement === this.searchInput) { + const searchInputIndex = tabbableItems.indexOf(this.searchInput); + + // Wrap to last tabbable if tabbing backwards. + if (amount < 0) { + if (searchInputIndex === 0) { + tabbableItems[tabbableItems.length - 1].focus(); + return; + } } + + // Otherwise tab to the next adjacent item. + tabbableItems[searchInputIndex + amount].focus(); + return; } - // Wrap to first tabbable if tabbing forwards. - if (amount > 0) { - if (comboBoxIndex === tabbableItems.length - 1) { - tabbableItems[0].focus(); - return; + if (document.activeElement === this.clearButton) { + const clearButtonIndex = tabbableItems.indexOf(this.clearButton); + + // Wrap to first tabbable if tabbing forwards. + if (amount > 0) { + if (clearButtonIndex === tabbableItems.length - 1) { + tabbableItems[0].focus(); + return; + } } - } - tabbableItems[comboBoxIndex + amount].focus(); + // Otherwise tab to the next adjacent item. + tabbableItems[clearButtonIndex + amount].focus(); + } }; incrementActiveOptionIndex = throttle(amount => { @@ -290,14 +302,10 @@ 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(); } @@ -309,6 +317,7 @@ export class EuiComboBox extends Component { || this.optionsList === event.target || this.optionsList && this.optionsList.contains(event.target) ) { + this.onFocus(); return; } @@ -454,8 +463,14 @@ export class EuiComboBox extends Component { this.options[index] = node; }; + clearButtonRef = node => { + this.clearButton = node; + }; + componentDidMount() { this._isMounted = true; + document.addEventListener('click', this.onDocumentFocusChange); + document.addEventListener('focusin', this.onDocumentFocusChange); // TODO: This will need to be called once the actual stylesheet loads. setTimeout(() => { @@ -593,6 +608,7 @@ export class EuiComboBox extends Component { onOpenListClick={this.onOpenListClick} singleSelection={singleSelection} isDisabled={isDisabled} + clearButtonRef={this.clearButtonRef} /> {optionsList} 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 0f181f51e19a..d26fd2e1aead 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 @@ -29,6 +29,7 @@ export class EuiComboBoxInput extends Component { onOpenListClick: PropTypes.func.isRequired, singleSelection: PropTypes.bool, isDisabled: PropTypes.bool, + clearButtonRef: PropTypes.func, } constructor(props) { @@ -88,6 +89,7 @@ export class EuiComboBoxInput extends Component { onOpenListClick, singleSelection, isDisabled, + clearButtonRef, } = this.props; const pills = selectedOptions.map((option) => { @@ -147,14 +149,24 @@ export class EuiComboBoxInput extends Component { const clickProps = {}; if (!isDisabled) { - clickProps.onClear = hasSelectedOptions ? onClear : undefined; - clickProps.onIconClick = isListOpen ? undefined : onOpenListClick; + clickProps.clear = { + onClick: hasSelectedOptions ? onClear : undefined, + ref: clearButtonRef, + }; } + const icon = { + type: 'arrowDown', + side: 'right', + onClick: isListOpen && !isDisabled ? undefined : onOpenListClick, + // We want to remove this from the tab order because you can open the combo box by tabbing + // to it already. + tabIndex: '-1', + }; + return (
diff --git a/src/components/form/field_number/field_number.js b/src/components/form/field_number/field_number.js index a75471e455af..eba2f0885b97 100644 --- a/src/components/form/field_number/field_number.js +++ b/src/components/form/field_number/field_number.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import { + ICON_SIDES, EuiFormControlLayout, } from '../form_control_layout'; @@ -81,7 +82,14 @@ EuiFieldNumber.propTypes = { max: PropTypes.number, step: PropTypes.number, value: numberOrEmptyString, - icon: PropTypes.string, + icon: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.shape({ + type: PropTypes.string, + side: PropTypes.oneOf(ICON_SIDES), + onClick: PropTypes.func, + }), + ]), isInvalid: PropTypes.bool, fullWidth: PropTypes.bool, isLoading: PropTypes.bool, diff --git a/src/components/form/field_number/field_number.test.js b/src/components/form/field_number/field_number.test.js index 41b16cb32a52..15158c1d8b10 100644 --- a/src/components/form/field_number/field_number.test.js +++ b/src/components/form/field_number/field_number.test.js @@ -4,7 +4,13 @@ import { requiredProps } from '../../../test/required_props'; import { EuiFieldNumber } from './field_number'; -jest.mock('../form_control_layout', () => ({ EuiFormControlLayout: 'eui-form-control-layout' })); +jest.mock('../form_control_layout', () => { + const formControlLayout = require.requireActual('../form_control_layout') + return { + ...formControlLayout, + EuiFormControlLayout: 'eui-form-control-layout', + } +}); jest.mock('../validatable_control', () => ({ EuiValidatableControl: 'eui-validatable-control' })); describe('EuiFieldNumber', () => { diff --git a/src/components/form/field_text/field_text.js b/src/components/form/field_text/field_text.js index 7fb91cd72f91..361ebfc87e66 100644 --- a/src/components/form/field_text/field_text.js +++ b/src/components/form/field_text/field_text.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import { + ICON_SIDES, EuiFormControlLayout, } from '../form_control_layout'; @@ -61,7 +62,14 @@ EuiFieldText.propTypes = { id: PropTypes.string, placeholder: PropTypes.string, value: PropTypes.string, - icon: PropTypes.string, + icon: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.shape({ + type: PropTypes.string, + side: PropTypes.oneOf(ICON_SIDES), + onClick: PropTypes.func, + }), + ]), isInvalid: PropTypes.bool, inputRef: PropTypes.func, fullWidth: PropTypes.bool, diff --git a/src/components/form/field_text/field_text.test.js b/src/components/form/field_text/field_text.test.js index d0b7367c8518..1f04f7db0581 100644 --- a/src/components/form/field_text/field_text.test.js +++ b/src/components/form/field_text/field_text.test.js @@ -4,7 +4,13 @@ import { requiredProps } from '../../../test/required_props'; import { EuiFieldText } from './field_text'; -jest.mock('../form_control_layout', () => ({ EuiFormControlLayout: 'eui-form-control-layout' })); +jest.mock('../form_control_layout', () => { + const formControlLayout = require.requireActual('../form_control_layout') + return { + ...formControlLayout, + EuiFormControlLayout: 'eui-form-control-layout', + } +}); jest.mock('../validatable_control', () => ({ EuiValidatableControl: 'eui-validatable-control' })); describe('EuiFieldText', () => { diff --git a/src/components/form/form_control_layout/__snapshots__/form_control_layout.test.js.snap b/src/components/form/form_control_layout/__snapshots__/form_control_layout.test.js.snap index a010abb1aaa8..f6a6ac04bcb1 100644 --- a/src/components/form/form_control_layout/__snapshots__/form_control_layout.test.js.snap +++ b/src/components/form/form_control_layout/__snapshots__/form_control_layout.test.js.snap @@ -2,19 +2,52 @@ exports[`EuiFormControlLayout is rendered 1`] = `
`; +exports[`EuiFormControlLayout props clear onClick is rendered 1`] = ` +
+ +
+`; + exports[`EuiFormControlLayout props fullWidth is rendered 1`] = `
`; -exports[`EuiFormControlLayout props icon is rendered 1`] = ` +exports[`EuiFormControlLayout props icon is rendered as a string 1`] = `
@@ -40,7 +73,34 @@ exports[`EuiFormControlLayout props icon is rendered 1`] = `
`; -exports[`EuiFormControlLayout props iconSide left is rendered 1`] = ` +exports[`EuiFormControlLayout props icon is rendered as an object 1`] = ` +
+ +
+`; + +exports[`EuiFormControlLayout props icon side left is rendered 1`] = `
@@ -66,7 +126,7 @@ exports[`EuiFormControlLayout props iconSide left is rendered 1`] = `
`; -exports[`EuiFormControlLayout props iconSide right is rendered 1`] = ` +exports[`EuiFormControlLayout props icon side right is rendered 1`] = `
diff --git a/src/components/form/form_control_layout/form_control_layout.js b/src/components/form/form_control_layout/form_control_layout.js index 88fef6766dfe..6146efb30050 100644 --- a/src/components/form/form_control_layout/form_control_layout.js +++ b/src/components/form/form_control_layout/form_control_layout.js @@ -15,13 +15,12 @@ export const ICON_SIDES = Object.keys(iconSideToClassNameMap); export const EuiFormControlLayout = ({ children, icon, + clear, fullWidth, - onClear, - iconSide, isLoading, - onIconClick, compressed, - className + className, + ...rest }) => { const classes = classNames( @@ -42,11 +41,26 @@ export const EuiFormControlLayout = ({ let optionalIcon; if (icon) { + // Normalize the icon to an object if it's a string. + const iconProps = typeof icon === 'string' ? { + type: icon, + } : icon; + + const { + className: iconClassName, + type: iconType, + side: iconSide = 'left', + onClick: onIconClick, + ref: iconRef, + ...iconRest + } = iconProps + const iconClasses = classNames( 'euiFormControlLayout__icon', iconSideToClassNameMap[iconSide], + iconClassName, { - 'euiFormControlLayout__iconButton': onIconClick + 'euiFormControlLayout__iconButton': onIconClick, }, ); @@ -55,9 +69,11 @@ export const EuiFormControlLayout = ({ ) @@ -66,18 +82,28 @@ export const EuiFormControlLayout = ({