Skip to content

Commit

Permalink
Fix EuiComboBox focus trap.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjcenizal committed May 23, 2018
1 parent b014a1d commit 791009f
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 35 deletions.
43 changes: 29 additions & 14 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,35 @@ export class EuiComboBox extends Component {

tabAway = 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;
const searchInputIndex = tabbableItems.indexOf(this.searchInput);
const clearButtonIndex = tabbableItems.indexOf(this.clearButton);

if (document.activeElement === 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) {
// 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 => {
Expand Down Expand Up @@ -454,6 +464,10 @@ export class EuiComboBox extends Component {
this.options[index] = node;
};

clearButtonRef = node => {
this.clearButton = node;
};

componentDidMount() {
this._isMounted = true;

Expand Down Expand Up @@ -593,6 +607,7 @@ export class EuiComboBox extends Component {
onOpenListClick={this.onOpenListClick}
singleSelection={singleSelection}
isDisabled={isDisabled}
clearButtonRef={this.clearButtonRef}
/>

{optionsList}
Expand Down
20 changes: 16 additions & 4 deletions src/components/combo_box/combo_box_input/combo_box_input.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class EuiComboBoxInput extends Component {
onOpenListClick: PropTypes.func.isRequired,
singleSelection: PropTypes.bool,
isDisabled: PropTypes.bool,
clearButtonRef: PropTypes.func,
}

constructor(props) {
Expand Down Expand Up @@ -88,6 +89,7 @@ export class EuiComboBoxInput extends Component {
onOpenListClick,
singleSelection,
isDisabled,
clearButtonRef,
} = this.props;

const pills = selectedOptions.map((option) => {
Expand Down Expand Up @@ -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 (
<EuiFormControlLayout
icon="arrowDown"
iconSide="right"
icon={icon}
{...clickProps}
>
<div
Expand Down
64 changes: 48 additions & 16 deletions src/components/form/form_control_layout/form_control_layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
},
);

Expand All @@ -55,9 +69,11 @@ export const EuiFormControlLayout = ({
<button
className={iconClasses}
onClick={onIconClick}
ref={iconRef}
{...iconRest}
>
<EuiIcon
type={icon}
type={iconType}
/>
</button>
)
Expand All @@ -66,18 +82,28 @@ export const EuiFormControlLayout = ({
<EuiIcon
aria-hidden="true"
className={iconClasses}
type={icon}
type={iconType}
{...iconRest}
/>
);
}
}

let optionalClear;
if (onClear) {
if (clear) {
const {
className: clearClassName,
onClick: onClearClick,
...clearRest
} = clear;

const clearClasses = classNames('euiFormControlLayout__clear', clearClassName);

optionalClear = (
<button
className="euiFormControlLayout__clear"
onClick={onClear}
className={clearClasses}
onClick={onClearClick}
{...clearRest}
>
<EuiIcon
className="euiFormControlLayout__clearIcon"
Expand All @@ -88,7 +114,7 @@ export const EuiFormControlLayout = ({
}

return (
<div className={classes}>
<div className={classes} {...rest}>
{children}
{optionalIcon}
{optionalClear}
Expand All @@ -99,18 +125,24 @@ export const EuiFormControlLayout = ({

EuiFormControlLayout.propTypes = {
children: PropTypes.node,
icon: PropTypes.string,
icon: PropTypes.oneOfType([
PropTypes.string,
PropTypes.shape({
type: PropTypes.string,
side: PropTypes.oneOf(ICON_SIDES),
onClick: PropTypes.func,
}),
]),
clear: PropTypes.shape({
onClick: PropTypes.func,
}),
fullWidth: PropTypes.bool,
iconSide: PropTypes.oneOf(ICON_SIDES),
isLoading: PropTypes.bool,
onClear: PropTypes.func,
onIconClick: PropTypes.func,
className: PropTypes.string,
compressed: PropTypes.bool,
};

EuiFormControlLayout.defaultProps = {
iconSide: 'left',
isLoading: false,
compressed: false,
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import React from 'react';
import { render } from 'enzyme';

import {
requiredProps,
} from '../../../test';

import {
EuiFormControlLayout,
ICON_SIDES,
Expand All @@ -11,7 +15,7 @@ jest.mock('../../', () => ({ EuiIcon: 'eui_icon', EuiLoadingSpinner: 'eui_loadin
describe('EuiFormControlLayout', () => {
test('is rendered', () => {
const component = render(
<EuiFormControlLayout>
<EuiFormControlLayout {...requiredProps}>
<input />
</EuiFormControlLayout>
);
Expand Down

0 comments on commit 791009f

Please sign in to comment.