From 3d7eeb9bdc94795ea697cd9a0f0337621b509ca0 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 26 Jan 2021 15:09:10 -0600 Subject: [PATCH 1/6] fix(combobox): remove ARIA from containing uninteractive elements --- .../react/src/components/ComboBox/ComboBox.js | 221 ++++++++++-------- 1 file changed, 125 insertions(+), 96 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js index 4f1baf5bc654..55eb03486daf 100644 --- a/packages/react/src/components/ComboBox/ComboBox.js +++ b/packages/react/src/components/ComboBox/ComboBox.js @@ -350,6 +350,30 @@ export default class ComboBox extends React.Component { [`${prefix}--text-input--empty`]: !this.state.inputValue, }); + // return ( + // + // {({ getRootProps, getInputProps }) => { + // const rootProps = getRootProps({}, { suppressRefError: true }); + // const inputProps = getInputProps({}); + // console.log(''); + // console.log(rootProps); + // console.log(inputProps); + + // return ( + //
+ // + //
+ //
+ // + // + //
+ //
+ //
+ // ); + // }} + //
+ // ); + // needs to be Capitalized for react to render it correctly const ItemToElement = itemToElement; return ( @@ -368,6 +392,7 @@ export default class ComboBox extends React.Component { getInputProps, getItemProps, getLabelProps, + getRootProps, isOpen, inputValue, selectedItem, @@ -375,108 +400,112 @@ export default class ComboBox extends React.Component { clearSelection, toggleMenu, getMenuProps, - }) => ( -
- {titleText && ( - - )} - - - { + return ( +
+ {titleText && ( + + )} + + { - if (match(event, keys.Space)) { - event.stopPropagation(); - } - - if (match(event, keys.Enter)) { - toggleMenu(); - } - }, - })} - /> - {invalid && ( - + { + if (match(event, keys.Space)) { + event.stopPropagation(); + } + + if (match(event, keys.Enter)) { + toggleMenu(); + } + }, + })} /> - )} - {inputValue && ( - + )} + {inputValue && ( + + )} + + + {isOpen && ( + + {this.filterItems(items, itemToString, inputValue).map( + (item, index) => { + const itemProps = getItemProps({ item, index }); + return ( + + {itemToElement ? ( + + ) : ( + itemToString(item) + )} + {selectedItem === item && ( + + )} + + ); + } + )} + )} - - - {isOpen && ( - - {this.filterItems(items, itemToString, inputValue).map( - (item, index) => { - const itemProps = getItemProps({ item, index }); - return ( - - {itemToElement ? ( - - ) : ( - itemToString(item) - )} - {selectedItem === item && ( - - )} - - ); - } - )} - + + {helperText && !invalid && ( +
+ {helperText} +
)} - - {helperText && !invalid && ( -
- {helperText} -
- )} -
- )} +
+ ); + }} ); } From 2546013cb2232eede971ddbaefbf11c9785bfad3 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 26 Jan 2021 15:58:38 -0600 Subject: [PATCH 2/6] fix(combobox): update additional ARIA roles and attributes --- .../src/components/list-box/_list-box.scss | 10 ++++- .../react/src/components/ComboBox/ComboBox.js | 37 ++++--------------- .../src/components/ListBox/ListBoxField.js | 5 +-- .../src/components/ListBox/ListBoxMenuIcon.js | 18 +++++---- .../components/ListBox/ListBoxSelection.js | 8 ++-- 5 files changed, 31 insertions(+), 47 deletions(-) diff --git a/packages/components/src/components/list-box/_list-box.scss b/packages/components/src/components/list-box/_list-box.scss index 56ae3cd629fe..c0f1837b2dac 100644 --- a/packages/components/src/components/list-box/_list-box.scss +++ b/packages/components/src/components/list-box/_list-box.scss @@ -388,9 +388,15 @@ $list-box-menu-width: rem(300px); // Menu status inside of a `list-box__field` .#{$prefix}--list-box__menu-icon { + @include button-reset($width: false); + position: absolute; top: 0; - right: $carbon--spacing-05; + + // TODO: convert to spacing tokens + right: 8px; + padding: 0 8px; + display: flex; align-items: center; height: 100%; @@ -416,6 +422,8 @@ $list-box-menu-width: rem(300px); // Selection indicator for a `list-box__field` .#{$prefix}--list-box__selection { + @include button-reset($width: false); + position: absolute; top: 50%; /* to preserve .5rem space between icons according to spec top/transform used to center the combobox clear selection icon in IE11 */ diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js index 55eb03486daf..1003a8a0632a 100644 --- a/packages/react/src/components/ComboBox/ComboBox.js +++ b/packages/react/src/components/ComboBox/ComboBox.js @@ -350,30 +350,6 @@ export default class ComboBox extends React.Component { [`${prefix}--text-input--empty`]: !this.state.inputValue, }); - // return ( - // - // {({ getRootProps, getInputProps }) => { - // const rootProps = getRootProps({}, { suppressRefError: true }); - // const inputProps = getInputProps({}); - // console.log(''); - // console.log(rootProps); - // console.log(inputProps); - - // return ( - //
- // - //
- //
- // - // - //
- //
- //
- // ); - // }} - //
- // ); - // needs to be Capitalized for react to render it correctly const ItemToElement = itemToElement; return ( @@ -401,6 +377,11 @@ export default class ComboBox extends React.Component { toggleMenu, getMenuProps, }) => { + const buttonProps = getToggleButtonProps({ + disabled, + onClick: this.onToggleClick(isOpen), + }); + return (
{titleText && ( @@ -416,11 +397,7 @@ export default class ComboBox extends React.Component { isOpen={isOpen} light={light} size={size}> - + {isOpen && ( diff --git a/packages/react/src/components/ListBox/ListBoxField.js b/packages/react/src/components/ListBox/ListBoxField.js index 1f3cadd1132d..b72aaaa88b50 100644 --- a/packages/react/src/components/ListBox/ListBoxField.js +++ b/packages/react/src/components/ListBox/ListBoxField.js @@ -21,10 +21,7 @@ export const translationIds = {}; */ function ListBoxField({ children, disabled, tabIndex, ...rest }) { return ( -
+
{children}
); diff --git a/packages/react/src/components/ListBox/ListBoxMenuIcon.js b/packages/react/src/components/ListBox/ListBoxMenuIcon.js index aa25c8b5f3fc..40b3e36f8830 100644 --- a/packages/react/src/components/ListBox/ListBoxMenuIcon.js +++ b/packages/react/src/components/ListBox/ListBoxMenuIcon.js @@ -19,25 +19,27 @@ export const translationIds = { }; const defaultTranslations = { - [translationIds['close.menu']]: 'Close menu', - [translationIds['open.menu']]: 'Open menu', + [translationIds['close.menu']]: 'Close', + [translationIds['open.menu']]: 'Open', }; /** * `ListBoxMenuIcon` is used to orient the icon up or down depending on the * state of the menu for a given `ListBox` */ -const ListBoxMenuIcon = ({ isOpen, translateWithId: t }) => { +const ListBoxMenuIcon = ({ isOpen, translateWithId: t, ...rest }) => { const className = cx(`${prefix}--list-box__menu-icon`, { [`${prefix}--list-box__menu-icon--open`]: isOpen, }); const description = isOpen ? t('close.menu') : t('open.menu'); return ( -
- - {description} - -
+ ); }; diff --git a/packages/react/src/components/ListBox/ListBoxSelection.js b/packages/react/src/components/ListBox/ListBoxSelection.js index e61643b46972..87c0c54da97f 100644 --- a/packages/react/src/components/ListBox/ListBoxSelection.js +++ b/packages/react/src/components/ListBox/ListBoxSelection.js @@ -56,17 +56,15 @@ const ListBoxSelection = ({ }; const description = selectionCount ? t('clear.all') : t('clear.selection'); return ( -
{selectionCount} -
+ ); }; From 6c086983ed9268884585879ed1eeee0a64bd3f23 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 3 Feb 2021 14:11:01 -0600 Subject: [PATCH 3/6] fix(combobox): correctly move focus to input and handle menu changes --- .../react/src/components/ComboBox/ComboBox.js | 69 +++++++++++++------ .../src/components/ListBox/ListBoxField.js | 8 ++- .../components/ListBox/ListBoxSelection.js | 5 +- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js index 1003a8a0632a..6257a07f5274 100644 --- a/packages/react/src/components/ComboBox/ComboBox.js +++ b/packages/react/src/components/ComboBox/ComboBox.js @@ -15,6 +15,7 @@ import ListBox, { PropTypes as ListBoxPropTypes } from '../ListBox'; import { match, keys } from '../../internal/keyboard'; import setupGetInstanceId from '../../tools/setupGetInstanceId'; import { mapDownshiftProps } from '../../tools/createPropAdapter'; +import mergeRefs from '../../tools/mergeRefs'; const { prefix } = settings; @@ -303,6 +304,10 @@ export default class ComboBox extends React.Component { event.preventDownshiftDefault = true; event.persist(); } + + if (this.textInput.current) { + this.textInput.current.focus(); + } }; render() { @@ -369,23 +374,58 @@ export default class ComboBox extends React.Component { getItemProps, getLabelProps, getRootProps, + getMenuProps, isOpen, inputValue, selectedItem, highlightedIndex, clearSelection, toggleMenu, - getMenuProps, }) => { + const rootProps = getRootProps( + {}, + { + suppressRefError: true, + } + ); + const labelProps = getLabelProps(); const buttonProps = getToggleButtonProps({ disabled, onClick: this.onToggleClick(isOpen), + // When we moved the "root node" of Downshift to the for + // WCAG 2.1 compliance, we unfortunately hit this branch for the + // "mouseup" event that downshift listens to: + // https://github.com/downshift-js/downshift/blob/v5.2.1/src/downshift.js#L1051-L1065 + // + // As a result, it will reset the state of the component and so we + // stop the event from propagating to prevent this. This allows the + // toggleMenu behavior for the toggleButton to correctly open and + // close the menu. + onMouseUp(event) { + event.stopPropagation(); + }, + }); + const inputProps = getInputProps({ + disabled, + placeholder, + onClick() { + toggleMenu(); + }, + onKeyDown: (event) => { + if (match(event, keys.Space)) { + event.stopPropagation(); + } + + if (match(event, keys.Enter)) { + toggleMenu(); + } + }, }); return (
{titleText && ( -