fix(combobox): update to match ARIA 1.2 criteria (#7777)

* 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 <[email protected]> * fix(combobox): update aria-selected & aria-current * fix(combobox): update ariaLabel Co-authored-by: Andrea N. Cardona <[email protected]> Co-authored-by: Carolyn MacLeod <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
carbon-design-system · Mar 4, 2021 · dd2a0bc · dd2a0bc
1 parent 4a3955e
commit dd2a0bc
Show file tree

Hide file tree

Showing 6 changed files with 409 additions and 117 deletions.
diff --git 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
@@ -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
@@ -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,
-        }) => (
-          <div className={wrapperClasses}>
-            {titleText && (
-              <label className={titleClasses} {...getLabelProps()}>
-                {titleText}
-              </label>
-            )}
-            <ListBox
-              className={className}
-              disabled={disabled}
-              invalid={invalid}
-              aria-label={ariaLabel}
-              invalidText={invalidText}
-              isOpen={isOpen}
-              light={light}
-              size={size}
-              warn={warn}
-              warnText={warnText}>
-              <ListBox.Field
-                {...getToggleButtonProps({
-                  disabled,
-                  onClick: this.onToggleClick(isOpen),
-                })}>
-                <input
-                  disabled={disabled}
-                  className={inputClasses}
-                  type="text"
-                  tabIndex="0"
-                  aria-autocomplete="list"
-                  ref={this.textInput}
-                  {...rest}
-                  {...getInputProps({
-                    disabled,
-                    placeholder,
-                    onKeyDown: (event) => {
-                      if (match(event, keys.Space)) {
-                        event.stopPropagation();
-                      }
-
-                      if (match(event, keys.Enter)) {
-                        toggleMenu();
-                      }
-                    },
-                  })}
-                />
-                {invalid && (
-                  <WarningFilled16
-                    className={`${prefix}--list-box__invalid-icon`}
-                  />
-                )}
-                {showWarning && (
-                  <WarningAltFilled16
-                    className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`}
+        }) => {
+          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 <input> 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 <label for> provides this aria information.
+            'aria-labelledby': null,
+            disabled,
+            placeholder,
+            onClick() {
+              toggleMenu();
+            },
+            onKeyDown: (event) => {
+              if (match(event, keys.Space)) {
+                event.stopPropagation();
+              }
+
+              if (match(event, keys.Enter)) {
+                toggleMenu();
+              }
+            },
+          });
+
+          return (
+            <div className={wrapperClasses}>
+              {titleText && (
+                <label className={titleClasses} {...labelProps}>
+                  {titleText}
+                </label>
+              )}
+              <ListBox
+                className={className}
+                disabled={disabled}
+                invalid={invalid}
+                invalidText={invalidText}
+                isOpen={isOpen}
+                light={light}
+                size={size}
+                warn={warn}
+                warnText={warnText}>
+                <div className={`${prefix}--list-box__field`}>
+                  <input
+                    role="combobox"
+                    disabled={disabled}
+                    className={inputClasses}
+                    type="text"
+                    tabIndex="0"
+                    aria-autocomplete="list"
+                    aria-expanded={rootProps['aria-expanded']}
+                    aria-haspopup="listbox"
+                    aria-controls={inputProps['aria-controls']}
+                    {...inputProps}
+                    {...rest}
+                    ref={mergeRefs(this.textInput, rootProps.ref)}
                   />
-                )}
-                {inputValue && (
-                  <ListBox.Selection
-                    clearSelection={clearSelection}
+                  {invalid && (
+                    <WarningFilled16
+                      className={`${prefix}--list-box__invalid-icon`}
+                    />
+                  )}
+                  {showWarning && (
+                    <WarningAltFilled16
+                      className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`}
+                    />
+                  )}
+                  {inputValue && (
+                    <ListBoxSelection
+                      clearSelection={clearSelection}
+                      translateWithId={translateWithId}
+                      disabled={disabled}
+                      onClearSelection={this.handleSelectionClear}
+                    />
+                  )}
+                  <ListBoxTrigger
+                    {...buttonProps}
+                    isOpen={isOpen}
                     translateWithId={translateWithId}
-                    disabled={disabled}
-                    onClearSelection={this.handleSelectionClear}
                   />
-                )}
-                <ListBox.MenuIcon
-                  isOpen={isOpen}
-                  translateWithId={translateWithId}
-                />
-              </ListBox.Field>
-              {isOpen && (
+                </div>
                 <ListBox.Menu {...getMenuProps({ 'aria-label': ariaLabel })}>
-                  {this.filterItems(items, itemToString, inputValue).map(
-                    (item, index) => {
-                      const itemProps = getItemProps({ item, index });
-                      return (
-                        <ListBox.MenuItem
-                          key={itemProps.id}
-                          isActive={selectedItem === item}
-                          tabIndex="-1"
-                          isHighlighted={
-                            highlightedIndex === index ||
-                            (selectedItem && selectedItem.id === item.id) ||
-                            false
-                          }
-                          title={itemToElement ? item.text : itemToString(item)}
-                          {...itemProps}>
-                          {itemToElement ? (
-                            <ItemToElement key={itemProps.id} {...item} />
-                          ) : (
-                            itemToString(item)
-                          )}
-                          {selectedItem === item && (
-                            <Checkmark16
-                              className={`${prefix}--list-box__menu-item__selected-icon`}
-                            />
-                          )}
-                        </ListBox.MenuItem>
-                      );
-                    }
-                  )}
+                  {isOpen
+                    ? this.filterItems(items, itemToString, inputValue).map(
+                        (item, index) => {
+                          const itemProps = getItemProps({
+                            item,
+                            index,
+                            ['aria-current']:
+                              selectedItem === item ? true : null,
+                            ['aria-selected']:
+                              highlightedIndex === index ? true : null,
+                          });
+                          return (
+                            <ListBox.MenuItem
+                              key={itemProps.id}
+                              isActive={selectedItem === item}
+                              tabIndex="-1"
+                              isHighlighted={
+                                highlightedIndex === index ||
+                                (selectedItem && selectedItem.id === item.id) ||
+                                false
+                              }
+                              title={
+                                itemToElement ? item.text : itemToString(item)
+                              }
+                              {...itemProps}>
+                              {itemToElement ? (
+                                <ItemToElement key={itemProps.id} {...item} />
+                              ) : (
+                                itemToString(item)
+                              )}
+                              {selectedItem === item && (
+                                <Checkmark16
+                                  className={`${prefix}--list-box__menu-item__selected-icon`}
+                                />
+                              )}
+                            </ListBox.MenuItem>
+                          );
+                        }
+                      )
+                    : null}
                 </ListBox.Menu>
+              </ListBox>
+              {helperText && !invalid && !warn && (
+                <div id={comboBoxHelperId} className={helperClasses}>
+                  {helperText}
+                </div>
               )}
-            </ListBox>
-            {helperText && !invalid && !warn && (
-              <div id={comboBoxHelperId} className={helperClasses}>
-                {helperText}
-              </div>
-            )}
-          </div>
-        )}
+            </div>
+          );
+        }}
       </Downshift>
     );
   }