diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ce27e6e10..ca3093a4436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ - `EuiSearchBar` no longer has an `onParse` callback, and now passes an object to `onChange` with the shape `{ query, queryText, error }` ([#863](https://github.com/elastic/eui/pull/863)) - `EuiInMemoryTable`'s `search.onChange` callback now passes an object with `{ query, queryText, error }` instead of only the query ([#863](https://github.com/elastic/eui/pull/863)) +- `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, the clear button is now keyboard-accessible, and the virtualized list no longer interferes with the tab order ([#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-docs/src/components/guide_page/guide_page.js b/src-docs/src/components/guide_page/guide_page.js index 61ca6221d54..fb1b3192ec8 100644 --- a/src-docs/src/components/guide_page/guide_page.js +++ b/src-docs/src/components/guide_page/guide_page.js @@ -10,7 +10,7 @@ import { EuiSpacer, EuiFlexGroup, EuiFlexItem, - EuiButton + EuiButton, } from '../../../../src/components'; export const GuidePage = ({ children, title, intro, componentLinkTo }) => { @@ -38,6 +38,9 @@ export const GuidePage = ({ children, title, intro, componentLinkTo }) => { {children} + + {/* Give some space between the bottom of long content and the bottom of the screen */} + ); }; diff --git a/src-docs/src/views/form_controls/field_password.js b/src-docs/src/views/form_controls/field_password.js index e523958e337..b5b76292171 100644 --- a/src-docs/src/views/form_controls/field_password.js +++ b/src-docs/src/views/form_controls/field_password.js @@ -72,6 +72,16 @@ export default class extends Component { onChange={this.onChange} compressed /> + + + + ); } diff --git a/src-docs/src/views/form_controls/form_control_layout.js b/src-docs/src/views/form_controls/form_control_layout.js new file mode 100644 index 00000000000..d63f24ae602 --- /dev/null +++ b/src-docs/src/views/form_controls/form_control_layout.js @@ -0,0 +1,110 @@ +import React, { + Fragment, +} from 'react'; + +import { + EuiFormControlLayout, + EuiSpacer, +} from '../../../../src/components'; + +export default () => ( + + + + + + + + + + + + + + + {} }} + > + + + + + + {} }} + > + + + + + + + + + + + + + + + + + + {} }} + icon="search" + > + + + + + + {} }} + icon={{ type: 'arrowDown', side: 'right' }} + > + + + + + + {} }} + icon="search" + > + + + + + + {} }} + icon={{ type: 'arrowDown', side: 'right' }} + > + + + + + + {} }} + icon="search" + > + + + +); diff --git a/src-docs/src/views/form_controls/form_controls_example.js b/src-docs/src/views/form_controls/form_controls_example.js index 4914112048e..1fa851b9d68 100644 --- a/src-docs/src/views/form_controls/form_controls_example.js +++ b/src-docs/src/views/form_controls/form_controls_example.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { Fragment } from 'react'; import { renderToHtml } from '../../services'; @@ -14,6 +14,7 @@ import { EuiFieldSearch, EuiFieldText, EuiFilePicker, + EuiFormControlLayout, EuiLink, EuiRadio, EuiRange, @@ -74,6 +75,10 @@ import Switch from './switch'; const switchSource = require('!!raw-loader!./switch'); const switchHtml = renderToHtml(Switch); +import FormControlLayout from './form_control_layout'; +const formControlLayoutSource = require('!!raw-loader!./form_control_layout'); +const formControlLayoutHtml = renderToHtml(FormControlLayout); + export const FormControlsExample = { title: 'Form controls', sections: [{ @@ -255,6 +260,34 @@ export const FormControlsExample = { EuiSwitch, }, demo: , + }, { + title: 'Form control layout', + source: [{ + type: GuideSectionTypes.JS, + code: formControlLayoutSource, + }, { + type: GuideSectionTypes.HTML, + code: formControlLayoutHtml, + }], + text: ( + +

+ EuiFormControlLayout is generally used internally to consistently style + form controls, but it’s published in case you want to create your own form control + which matches those of EUI. The examples below demonstrate its various states. +

+ +

+ Note that the padding on the input itself doesn’t take into account the presence + of the various icons supported by EuiFormControlLayout. Any input component + provided to EuiFormControlLayout is responsible for its own padding. +

+ + ), + props: { + EuiFormControlLayout, + }, + demo: , }], }; 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 5285fbbb094..0d50dc5a6ad 100644 --- a/src/components/combo_box/__snapshots__/combo_box.test.js.snap +++ b/src/components/combo_box/__snapshots__/combo_box.test.js.snap @@ -16,13 +16,14 @@ exports[`EuiComboBox is rendered 1`] = ` onChange={[Function]} onClear={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={Array []} singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="" /> @@ -44,9 +45,9 @@ exports[`props isClearable is false with selectedOptions 1`] = ` isListOpen={false} onChange={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={ @@ -60,6 +61,7 @@ exports[`props isClearable is false with selectedOptions 1`] = ` ] } singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="Mimas, Iapetus" /> @@ -81,13 +83,14 @@ exports[`props isClearable is false without selectedOptions 1`] = ` isListOpen={false} onChange={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={Array []} singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="" /> @@ -110,9 +113,9 @@ exports[`props isDisabled 1`] = ` isListOpen={false} onChange={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={ @@ -123,6 +126,7 @@ exports[`props isDisabled 1`] = ` ] } singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="Mimas" /> @@ -145,13 +149,14 @@ exports[`props options 1`] = ` onChange={[Function]} onClear={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={Array []} singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="" /> @@ -174,9 +179,9 @@ exports[`props selectedOptions 1`] = ` onChange={[Function]} onClear={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={ @@ -190,6 +195,7 @@ exports[`props selectedOptions 1`] = ` ] } singleSelection={false} + toggleButtonRef={[Function]} updatePosition={[Function]} value="Mimas, Iapetus" /> @@ -212,9 +218,9 @@ exports[`props singleSelection 1`] = ` onChange={[Function]} onClear={[Function]} onClick={[Function]} - onClose={[Function]} + onCloseListClick={[Function]} onFocus={[Function]} - onOpen={[Function]} + onOpenListClick={[Function]} onRemoveOption={[Function]} searchValue="" selectedOptions={ @@ -225,6 +231,7 @@ exports[`props singleSelection 1`] = ` ] } singleSelection={true} + toggleButtonRef={[Function]} updatePosition={[Function]} value="Mimas" /> diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 7d4813b5189..608588716af 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -137,26 +137,46 @@ export class EuiComboBox extends Component { }; tabAway = amount => { + if (![-1, 1].includes(amount)) { + throw new Error(`tabAway expects amount to be -1 or 1, but received ${amount}`); + } + 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 === -1) { + if (searchInputIndex === 0) { + tabbableItems[tabbableItems.length - 1].focus(); + return true; + } } + + // Otherwise tab to the next adjacent item. + tabbableItems[searchInputIndex + amount].focus(); + return true; } - // Wrap to first tabbable if tabbing forwards. - if (amount > 0) { - if (comboBoxIndex === tabbableItems.length - 1) { - tabbableItems[0].focus(); - return; + 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; } - tabbableItems[comboBoxIndex + amount].focus(); + // Tab natively. + return false; }; incrementActiveOptionIndex = throttle(amount => { @@ -350,12 +370,10 @@ export class EuiComboBox extends Component { break; case TAB: - e.preventDefault(); - e.stopPropagation(); - if (e.shiftKey) { - this.tabAway(-1); - } else { - this.tabAway(1); + const amount = e.shiftKey ? -1 : 1; + if (this.tabAway(amount)) { + e.preventDefault(); + e.stopPropagation(); } break; } @@ -417,6 +435,14 @@ export class EuiComboBox extends Component { } }; + onOpenListClick = () => { + this.searchInput.focus(); + }; + + onCloseListClick = () => { + this.closeList(); + }; + onSearchChange = (searchValue) => { if (this.props.onSearchChange) { this.props.onSearchChange(searchValue); @@ -450,6 +476,10 @@ export class EuiComboBox extends Component { this.options[index] = node; }; + toggleButtonRef = node => { + this.toggleButton = node; + }; + componentDidMount() { this._isMounted = true; @@ -586,10 +616,11 @@ export class EuiComboBox extends Component { onClear={isClearable && !isDisabled ? this.clearSelectedOptions : undefined} hasSelectedOptions={selectedOptions.length > 0} isListOpen={isListOpen} - onOpen={this.openList} - onClose={this.closeList} + onOpenListClick={this.onOpenListClick} + onCloseListClick={this.onCloseListClick} singleSelection={singleSelection} isDisabled={isDisabled} + toggleButtonRef={this.toggleButtonRef} /> {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 33ff228776e..85c7a377e60 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 @@ -26,10 +26,11 @@ export class EuiComboBoxInput extends Component { onClear: PropTypes.func, hasSelectedOptions: PropTypes.bool.isRequired, isListOpen: PropTypes.bool.isRequired, - onOpen: PropTypes.func.isRequired, - onClose: PropTypes.func.isRequired, + onOpenListClick: PropTypes.func.isRequired, + onCloseListClick: PropTypes.func.isRequired, singleSelection: PropTypes.bool, isDisabled: PropTypes.bool, + toggleButtonRef: PropTypes.func, } constructor(props) { @@ -86,10 +87,11 @@ export class EuiComboBoxInput extends Component { onClear, hasSelectedOptions, isListOpen, - onOpen, - onClose, + onOpenListClick, + onCloseListClick, singleSelection, isDisabled, + toggleButtonRef, } = this.props; const pills = selectedOptions.map((option) => { @@ -149,14 +151,21 @@ export class EuiComboBoxInput extends Component { const clickProps = {}; if (!isDisabled) { - clickProps.onClear = hasSelectedOptions ? onClear : undefined; - clickProps.onIconClick = isListOpen ? onClose : onOpen; + clickProps.clear = { + onClick: hasSelectedOptions ? onClear : undefined, + }; } + const icon = { + type: 'arrowDown', + side: 'right', + onClick: isListOpen && !isDisabled ? onCloseListClick : onOpenListClick, + ref: toggleButtonRef, + }; + return (
diff --git a/src/components/form/field_number/field_number.test.js b/src/components/form/field_number/field_number.test.js index 41b16cb32a5..15158c1d8b1 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.test.js b/src/components/form/field_text/field_text.test.js index d0b7367c851..1f04f7db058 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 a010abb1aaa..42d6a76a15b 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,93 +2,173 @@ 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`] = `
- + + + + + +
`; -exports[`EuiFormControlLayout props iconSide left is rendered 1`] = ` +exports[`EuiFormControlLayout props icon is rendered as an object 1`] = `
- + + + + + +
`; -exports[`EuiFormControlLayout props iconSide right is rendered 1`] = ` +exports[`EuiFormControlLayout props icon side left is rendered 1`] = `
- + + + + + +
+ +`; + +exports[`EuiFormControlLayout props icon side right is rendered 1`] = ` +
+
+ +
`; @@ -97,7 +177,11 @@ exports[`EuiFormControlLayout props isLoading is rendered 1`] = ` class="euiFormControlLayout" >
+ class="euiFormControlLayout__icons euiFormControlLayout__icons--right" + > +
+
`; diff --git a/src/components/form/form_control_layout/_form_control_layout.scss b/src/components/form/form_control_layout/_form_control_layout.scss index 7e197063cea..b6671054f8c 100644 --- a/src/components/form/form_control_layout/_form_control_layout.scss +++ b/src/components/form/form_control_layout/_form_control_layout.scss @@ -13,80 +13,49 @@ } .euiFormControlLayout__icon { - position: absolute; - top: $euiFormControlPadding; - left: $euiFormControlPadding; pointer-events: none; - - @at-root { - .euiFormControlLayout--compressed#{&} { - top: $euiFormControlPadding--compressed; - } - } } - .euiFormControlLayout__iconButton { + .euiFormControlLayout__icon--button { pointer-events: all; + height: $euiSize; + width: $euiSize; @include size($euiSize); .euiIcon { vertical-align: baseline; } - @at-root { - .euiFormControlLayout--compressed#{&} { - top: $euiFormControlPadding--compressed - 1px; - } + svg { + margin-top: -$euiSizeXS; + } + + &:focus { + background: $euiColorPrimary; + border-radius: 100%; + color: $euiColorGhost; } } - .euiFormControlLayout__clear { + .euiFormControlLayout__icons { position: absolute; - top: $euiFormControlPadding; - right: $euiFormControlPadding; - - @include euiFormControlLayout__clear('.euiFormControlLayout__clearIcon'); - - @at-root { - .euiFormControlLayout--compressed#{&} { - top: $euiFormControlPadding--compressed + 2px; - } - } + top: 0; + bottom: 0; + left: $euiFormControlPadding; + display: flex; + align-items: center; - // Loading spinner needs adjustment if clear also exists - ~ .euiFormControlLayout__loading { - right: $euiFormControlPadding*3; + > * + * { + margin-left: $euiFormControlPadding; } } - .euiFormControlLayout__icon--right { + .euiFormControlLayout__icons--right { left: auto; right: $euiFormControlPadding; - - // Loading spinner needs adjustment if icon also exists like selects. - ~ .euiFormControlLayout__loading { - right: $euiFormControlPadding*3; - } - - // Same with clear buttons - ~ .euiFormControlLayout__clear { - right: $euiFormControlPadding*3; - - ~ .euiFormControlLayout__loading { - right: $euiFormControlPadding*5; - } - } } - .euiFormControlLayout__loading { - position: absolute; - top: $euiFormControlPadding; - right: $euiFormControlPadding; - - @at-root { - .euiFormControlLayout--compressed#{&} { - top: $euiFormControlPadding--compressed + 1px; - } - } + .euiFormControlLayout__clear { + @include euiFormControlLayout__clear('.euiFormControlLayout__clearIcon'); } } diff --git a/src/components/form/form_control_layout/_mixins.scss b/src/components/form/form_control_layout/_mixins.scss index 05865c366fd..30828beea91 100644 --- a/src/components/form/form_control_layout/_mixins.scss +++ b/src/components/form/form_control_layout/_mixins.scss @@ -5,6 +5,11 @@ background-color: transparentize($euiColorMediumShade, .5); border-radius: $clearSize; line-height: $clearSize; + transition: background-color $euiAnimSpeedExtraFast $euiAnimSlightResistance; + + &:focus, &:hover { + background-color: $euiColorPrimary; + } #{$iconClass} { @include size($euiSizeS); 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 88fef6766df..839ccc66e01 100644 --- a/src/components/form/form_control_layout/form_control_layout.js +++ b/src/components/form/form_control_layout/form_control_layout.js @@ -1,116 +1,185 @@ -import React from 'react'; +import React, { Fragment, Component } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { EuiIcon } from '../../icon'; import { EuiLoadingSpinner } from '../../loading'; -const iconSideToClassNameMap = { - left: '', - right: 'euiFormControlLayout__icon--right', -}; +export const ICON_SIDES = ['left', 'right']; -export const ICON_SIDES = Object.keys(iconSideToClassNameMap); - -export const EuiFormControlLayout = ({ - children, - icon, - fullWidth, - onClear, - iconSide, - isLoading, - onIconClick, - compressed, - className -}) => { - - const classes = classNames( - 'euiFormControlLayout', - { - 'euiFormControlLayout--fullWidth': fullWidth, - 'euiFormControlLayout--compressed': compressed, - }, - className - ); - - let optionalLoader; - if (isLoading) { - optionalLoader = ( - - ); - } +export class EuiFormControlLayout extends Component { + render() { + const { + children, + icon, // eslint-disable-line no-unused-vars + clear, // eslint-disable-line no-unused-vars + fullWidth, + isLoading, // eslint-disable-line no-unused-vars + compressed, + className, + ...rest + } = this.props; - let optionalIcon; - if (icon) { - const iconClasses = classNames( - 'euiFormControlLayout__icon', - iconSideToClassNameMap[iconSide], + const classes = classNames( + 'euiFormControlLayout', { - 'euiFormControlLayout__iconButton': onIconClick + 'euiFormControlLayout--fullWidth': fullWidth, + 'euiFormControlLayout--compressed': compressed, }, + className ); - if (onIconClick) { - optionalIcon = ( + return ( +
+ {children} + {this.renderIcons()} +
+ ); + } + + renderIcons() { + const { + isLoading, + icon, + clear, + } = this.props; + + let optionalLoader; + if (isLoading) { + optionalLoader = ( + + ); + } + + let optionalIconSide; + 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 + + optionalIconSide = iconSide + + const iconClasses = classNames( + 'euiFormControlLayout__icon', + iconClassName, + { + 'euiFormControlLayout__icon--button': onIconClick, + }, + ); + + if (onIconClick) { + optionalIcon = ( + + ) + } else { + optionalIcon = ( +