From dd2a0bc3940e58ac19d5501282933d8c8fd344a4 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 4 Mar 2021 15:29:29 -0600 Subject: [PATCH] fix(combobox): update to match ARIA 1.2 criteria (#7777) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(combobox): update to match WCAG 2.1 AA criteria *  fix(combobox): updated styling on ComboBox *  fix(combobox): updated styling on ComboBox -pt.2 * fix(comobobox): update aria-selected prop attribute * fix(comobobox): update aria-selected prop attribute * fix(combobox): remove aria-owns prop - fix lint * fix(combobox): remove aria-owns prop - remove aria-labelledby fix lint * fix(combobox): add aria commentary * fix(combobox): aria-labelledby refactor * fix(combobox): pass ariaLabel to getMenuProps * fix(combobox): update ariaLabel * fix(combobox): updated selected ui color token * Update packages/components/src/components/list-box/_list-box.scss Co-authored-by: Carolyn MacLeod * fix(combobox): update aria-selected & aria-current * fix(combobox): update ariaLabel Co-authored-by: Andrea N. Cardona Co-authored-by: Carolyn MacLeod Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../src/components/list-box/_list-box.scss | 23 +- .../src/components/ComboBox/ComboBox-test.js | 4 +- .../react/src/components/ComboBox/ComboBox.js | 262 ++++++++++-------- .../ListBox/next/ListBoxSelection.js | 162 +++++++++++ .../components/ListBox/next/ListBoxTrigger.js | 66 +++++ .../src/components/ListBox/next/index.js | 9 + 6 files changed, 409 insertions(+), 117 deletions(-) create mode 100644 packages/react/src/components/ListBox/next/ListBoxSelection.js create mode 100644 packages/react/src/components/ListBox/next/ListBoxTrigger.js create mode 100644 packages/react/src/components/ListBox/next/index.js diff --git a/packages/components/src/components/list-box/_list-box.scss b/packages/components/src/components/list-box/_list-box.scss index 007ca50836be..4553a455d555 100644 --- a/packages/components/src/components/list-box/_list-box.scss +++ b/packages/components/src/components/list-box/_list-box.scss @@ -393,18 +393,21 @@ $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; display: flex; align-items: center; - height: 100%; + justify-content: center; + width: rem(24px); + height: rem(24px); + outline: none; cursor: pointer; transition: transform $duration--fast-01 motion(standard, productive); } .#{$prefix}--list-box__menu-icon > svg { - height: 100%; fill: $icon-01; // Windows, Firefox HCM Fix @@ -416,20 +419,24 @@ $list-box-menu-width: rem(300px); } .#{$prefix}--list-box__menu-icon--open { + justify-content: center; + width: rem(24px); transform: rotate(180deg); } // 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 */ - right: rem(33px); + right: rem(36px); display: flex; align-items: center; justify-content: center; - width: rem(30px); - height: rem(30px); + width: rem(24px); + height: rem(24px); transform: translateY(-50%); cursor: pointer; transition: background-color $duration--fast-01 motion(standard, productive); @@ -751,8 +758,8 @@ $list-box-menu-width: rem(300px); .#{$prefix}--list-box__menu-item--active:hover, .#{$prefix}--list-box__menu-item--active.#{$prefix}--list-box__menu-item--highlighted { - background-color: $hover-selected-ui; - border-bottom-color: $hover-selected-ui; + background-color: $selected-ui; + border-bottom-color: $selected-ui; } .#{$prefix}--list-box__menu-item--active diff --git a/packages/react/src/components/ComboBox/ComboBox-test.js b/packages/react/src/components/ComboBox/ComboBox-test.js index ff2139c7debd..bacc66c3fb87 100644 --- a/packages/react/src/components/ComboBox/ComboBox-test.js +++ b/packages/react/src/components/ComboBox/ComboBox-test.js @@ -11,7 +11,6 @@ import { findListBoxNode, findMenuNode, findMenuItemNode, - openMenu, assertMenuOpen, assertMenuClosed, generateItems, @@ -28,6 +27,9 @@ const downshiftActions = { }; const clearInput = (wrapper) => wrapper.instance().handleOnStateChange({ inputValue: '' }, downshiftActions); +const openMenu = (wrapper) => { + wrapper.find(`[role="combobox"]`).simulate('click'); +}; describe('ComboBox', () => { let mockProps; diff --git a/packages/react/src/components/ComboBox/ComboBox.js b/packages/react/src/components/ComboBox/ComboBox.js index 490805d56424..e3ba0b2dd378 100644 --- a/packages/react/src/components/ComboBox/ComboBox.js +++ b/packages/react/src/components/ComboBox/ComboBox.js @@ -16,9 +16,11 @@ import { WarningFilled16, } from '@carbon/icons-react'; import ListBox, { PropTypes as ListBoxPropTypes } from '../ListBox'; +import { ListBoxTrigger, ListBoxSelection } from '../ListBox/next'; import { match, keys } from '../../internal/keyboard'; import setupGetInstanceId from '../../tools/setupGetInstanceId'; import { mapDownshiftProps } from '../../tools/createPropAdapter'; +import mergeRefs from '../../tools/mergeRefs'; const { prefix } = settings; @@ -242,13 +244,11 @@ export default class ComboBox extends React.Component { constructor(props) { super(props); - this.textInput = React.createRef(); - this.comboBoxInstanceId = getInstanceId(); - this.state = { inputValue: getInputValue(props, {}), }; + this.textInput = React.createRef(); } filterItems = (items, itemToString, inputValue) => @@ -317,6 +317,10 @@ export default class ComboBox extends React.Component { event.preventDownshiftDefault = true; event.persist(); } + + if (this.textInput.current) { + this.textInput.current.focus(); + } }; render() { @@ -382,126 +386,168 @@ export default class ComboBox extends React.Component { inputId={id} selectedItem={selectedItem}> {({ - getToggleButtonProps, getInputProps, getItemProps, getLabelProps, + getMenuProps, + getRootProps, + getToggleButtonProps, isOpen, inputValue, selectedItem, highlightedIndex, clearSelection, toggleMenu, - getMenuProps, - }) => ( -
- {titleText && ( - - )} - - - { - if (match(event, keys.Space)) { - event.stopPropagation(); - } - - if (match(event, keys.Enter)) { - toggleMenu(); - } - }, - })} - /> - {invalid && ( - - )} - {showWarning && ( - { + 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 + // ARIA 1.2 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({ + // Remove excess aria `aria-labelledby`. HTML
+ ); + }} ); } diff --git a/packages/react/src/components/ListBox/next/ListBoxSelection.js b/packages/react/src/components/ListBox/next/ListBoxSelection.js new file mode 100644 index 000000000000..c234da7f0952 --- /dev/null +++ b/packages/react/src/components/ListBox/next/ListBoxSelection.js @@ -0,0 +1,162 @@ +/** + * Copyright IBM Corp. 2016, 2018 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import cx from 'classnames'; +import React from 'react'; +import PropTypes from 'prop-types'; +import { Close16 } from '@carbon/icons-react'; +import { settings } from 'carbon-components'; +import { match, keys } from '../../../internal/keyboard'; + +const { prefix } = settings; + +/** + * `ListBoxSelection` is used to provide controls for clearing a selection, in + * addition to conditionally rendering a badge if the control has more than one + * selection. + */ +function ListBoxSelection({ + clearSelection, + selectionCount, + translateWithId: t, + disabled, + onClearSelection, +}) { + const className = cx(`${prefix}--list-box__selection`, { + [`${prefix}--tag--filter`]: selectionCount, + [`${prefix}--list-box__selection--multi`]: selectionCount, + }); + const description = selectionCount ? t('clear.all') : t('clear.selection'); + const tagClasses = cx( + `${prefix}--tag`, + `${prefix}--tag--filter`, + `${prefix}--tag--high-contrast`, + { + [`${prefix}--tag--disabled`]: disabled, + } + ); + + function onClick(event) { + event.stopPropagation(); + if (disabled) { + return; + } + clearSelection(event); + if (onClearSelection) { + onClearSelection(event); + } + } + + function onKeyDown(event) { + event.stopPropagation(); + if (disabled) { + return; + } + + // When a user hits ENTER, we'll clear the selection + if (match(event, keys.Enter)) { + clearSelection(event); + if (onClearSelection) { + onClearSelection(event); + } + } + } + + if (selectionCount) { + return ( +
+ + {selectionCount} + + +
+ ); + } + + return ( + + ); +} + +export const translationIds = { + 'clear.all': 'clear.all', + 'clear.selection': 'clear.selection', +}; + +const defaultTranslations = { + [translationIds['clear.all']]: 'Clear all selected items', + [translationIds['clear.selection']]: 'Clear selected item', +}; + +ListBoxSelection.propTypes = { + /** + * Specify a function to be invoked when a user interacts with the clear + * selection element. + */ + clearSelection: PropTypes.func.isRequired, + + /** + * Specify whether or not the clear selection element should be disabled + */ + disabled: PropTypes.bool, + + /** + * Specify an optional `onClearSelection` handler that is called when the underlying + * element is cleared + */ + onClearSelection: PropTypes.func, + + /** + * Specify an optional `onClick` handler that is called when the underlying + * clear selection element is clicked + */ + onClick: PropTypes.func, + + /** + * Specify an optional `onKeyDown` handler that is called when the underlying + * clear selection element fires a keydown event + */ + onKeyDown: PropTypes.func, + + /** + * Specify an optional `selectionCount` value that will be used to determine + * whether the selection should display a badge or a single clear icon. + */ + selectionCount: PropTypes.number, + + /** + * i18n hook used to provide the appropriate description for the given menu + * icon. This function takes in an id defined in `translationIds` and should + * return a string message for that given message id. + */ + translateWithId: PropTypes.func.isRequired, +}; + +ListBoxSelection.defaultProps = { + translateWithId: (id) => defaultTranslations[id], +}; + +export default ListBoxSelection; diff --git a/packages/react/src/components/ListBox/next/ListBoxTrigger.js b/packages/react/src/components/ListBox/next/ListBoxTrigger.js new file mode 100644 index 000000000000..816db162ebfd --- /dev/null +++ b/packages/react/src/components/ListBox/next/ListBoxTrigger.js @@ -0,0 +1,66 @@ +/** + * Copyright IBM Corp. 2016, 2018 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import cx from 'classnames'; +import React from 'react'; +import PropTypes from 'prop-types'; +import { ChevronDown16 } from '@carbon/icons-react'; +import { settings } from 'carbon-components'; + +const { prefix } = settings; + +export const translationIds = { + 'close.menu': 'close.menu', + 'open.menu': 'open.menu', +}; + +const defaultTranslations = { + [translationIds['close.menu']]: 'Close', + [translationIds['open.menu']]: 'Open', +}; + +/** + * `ListBoxTrigger` is used to orient the icon up or down depending on the + * state of the menu for a given `ListBox` + */ +const ListBoxTrigger = ({ 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 ( + + ); +}; + +ListBoxTrigger.propTypes = { + /** + * Specify whether the menu is currently open, which will influence the + * direction of the menu icon + */ + isOpen: PropTypes.bool.isRequired, + + /** + * i18n hook used to provide the appropriate description for the given menu + * icon. This function takes in an id defined in `translationIds` and should + * return a string message for that given message id. + */ + translateWithId: PropTypes.func.isRequired, +}; + +ListBoxTrigger.defaultProps = { + translateWithId: (id) => defaultTranslations[id], +}; + +export default ListBoxTrigger; diff --git a/packages/react/src/components/ListBox/next/index.js b/packages/react/src/components/ListBox/next/index.js new file mode 100644 index 000000000000..fca0b5be6228 --- /dev/null +++ b/packages/react/src/components/ListBox/next/index.js @@ -0,0 +1,9 @@ +/** + * Copyright IBM Corp. 2016, 2018 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +export { default as ListBoxSelection } from './ListBoxSelection'; +export { default as ListBoxTrigger } from './ListBoxTrigger';