fix(ListBox): a11y - move filter tag as sibling of menu trigger, esc …

…key improvements (#13173) * fix(MultiSelect): a11y - move filter tag as sibling of menu trigger * fix(Multiselect): remove tag from tab order * fix(FilterableMultiselect): remove clear, tag from tab order * fix(ComboBox): retain input value when menu is open and esc is pressed * fix(FilterableMultiSelect): a few more UX tweaks * style(MultiSelect): update styles to fix invalid issues when focused * style(FluidMultiselect): adjust focus styles when invalid * chore(log): remove console log --------- Co-authored-by: Andrea N. Cardona <[email protected]>
carbon-design-system · Mar 3, 2023 · e891d19 · e891d19
1 parent 5aff418
commit e891d19
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 70 deletions.
diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx
@@ -480,6 +480,14 @@ const ComboBox = React.forwardRef((props: ComboBoxProps, ref) => {
             if (match(event, keys.Enter) && !inputValue) {
               toggleMenu();
             }
+
+            if (match(event, keys.Escape) && inputValue) {
+              if (event.target === textInput.current && isOpen) {
+                toggleMenu();
+                event.preventDownshiftDefault = true;
+                event.persist();
+              }
+            }
           },
         });
 

diff --git a/packages/react/src/components/ListBox/ListBoxSelection.tsx b/packages/react/src/components/ListBox/ListBoxSelection.tsx
@@ -9,7 +9,6 @@ import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Close } from '@carbon/icons-react';
-import { match, keys } from '../../internal/keyboard';
 import { usePrefix } from '../../internal/usePrefix';
 import { KeyboardEvent, MouseEvent } from 'react';
 
@@ -84,20 +83,6 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
       onClearSelection(event);
     }
   };
-  const handleOnKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
-    event.stopPropagation();
-    if (disabled || readOnly) {
-      return;
-    }
-
-    // When a user hits ENTER, we'll clear the selection
-    if (match(event.code, keys.Enter)) {
-      clearSelection(event);
-      if (onClearSelection) {
-        onClearSelection(event);
-      }
-    }
-  };
   const description = selectionCount ? t('clear.all') : t('clear.selection');
   const tagClasses = cx(
     `${prefix}--tag`,
@@ -107,17 +92,18 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
       [`${prefix}--tag--disabled`]: disabled,
     }
   );
+
+  /* eslint-disable jsx-a11y/click-events-have-key-events */
   return selectionCount ? (
     <div className={tagClasses}>
       <span className={`${prefix}--tag__label`} title={`${selectionCount}`}>
         {selectionCount}
       </span>
       <div
         role="button"
-        tabIndex={disabled ? -1 : 0}
+        tabIndex={-1}
         className={`${prefix}--tag__close-icon`}
         onClick={handleOnClick}
-        onKeyDown={handleOnKeyDown}
         aria-label={t('clear.all')}
         title={description}
         aria-disabled={readOnly ? true : undefined}>
@@ -128,9 +114,8 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
     <div
       role="button"
       className={className}
-      tabIndex={disabled ? -1 : 0}
+      tabIndex={-1}
       onClick={handleOnClick}
-      onKeyDown={handleOnKeyDown}
       aria-label={description}
       title={description}>
       {selectionCount}

diff --git a/packages/react/src/components/ListBox/next/ListBoxSelection.js b/packages/react/src/components/ListBox/next/ListBoxSelection.js
@@ -9,7 +9,6 @@ import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Close } from '@carbon/icons-react';
-import { match, keys } from '../../../internal/keyboard';
 import { usePrefix } from '../../../internal/usePrefix';
 
 /**
@@ -51,21 +50,6 @@ function ListBoxSelection({
     }
   }
 
-  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 (
       <div className={tagClasses}>
@@ -77,8 +61,7 @@ function ListBoxSelection({
           className={`${prefix}--tag__close-icon`}
           disabled={disabled}
           onClick={onClick}
-          onKeyDown={onKeyDown}
-          tabIndex={disabled ? -1 : 0}
+          tabIndex={-1}
           title={description}
           type="button">
           <Close />
@@ -94,8 +77,7 @@ function ListBoxSelection({
       className={className}
       disabled={disabled}
       onClick={onClick}
-      onKeyDown={onKeyDown}
-      tabIndex={disabled ? -1 : 0}
+      tabIndex={-1}
       title={description}
       type="button">
       <Close />

diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js
@@ -87,6 +87,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
 
   const wrapperClasses = cx(
     `${prefix}--multi-select__wrapper`,
+    `${prefix}--multi-select--filterable__wrapper`,
     `${prefix}--list-box__wrapper`,
     [enabled ? containerClassName : null],
     {
@@ -243,6 +244,8 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
               [enabled ? null : containerClassName],
               {
                 [`${prefix}--multi-select--invalid`]: invalid,
+                [`${prefix}--multi-select--invalid--focused`]:
+                  invalid && inputFocused,
                 [`${prefix}--multi-select--open`]: isOpen,
                 [`${prefix}--multi-select--inline`]: inline,
                 [`${prefix}--multi-select--selected`]: selectedItem.length > 0,
@@ -299,6 +302,20 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
                   event.stopPropagation();
                 }
 
+                if (!disabled) {
+                  if (match(event, keys.Delete) || match(event, keys.Escape)) {
+                    if (isOpen) {
+                      handleOnMenuChange(true);
+                      clearInputValue();
+                      event.stopPropagation();
+                    } else if (!isOpen) {
+                      clearInputValue();
+                      clearSelection();
+                      event.stopPropagation();
+                    }
+                  }
+                }
+
                 if (match(event, keys.Tab)) {
                   handleOnMenuChange(false);
                 }
@@ -308,6 +325,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
               },
               onBlur: () => {
                 setInputFocused(false);
+                setInputValue('');
               },
             });
 

diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx
@@ -213,7 +213,7 @@ export interface MultiSelectProps<ItemType>
 
   /**
    * `onMenuChange` is a utility for this controlled component to communicate to a
-   * consuming component that the menu was opend(`true`)/closed(`false`).
+   * consuming component that the menu was open(`true`)/closed(`false`).
    */
   onMenuChange?(open: boolean): void;
 
@@ -315,7 +315,8 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
   const { current: multiSelectInstanceId } = useRef(getInstanceId());
   const [highlightedIndex, setHighlightedIndex] = useState();
   const [isFocused, setIsFocused] = useState(false);
-  const [isOpen, setIsOpen] = useState(open);
+  const [inputFocused, setInputFocused] = useState(false);
+  const [isOpen, setIsOpen] = useState(open || false);
   const [prevOpenProp, setPrevOpenProp] = useState(open);
   const [topItems, setTopItems] = useState([]);
   const {
@@ -347,7 +348,14 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
     items,
   });
 
-  const toggleButtonProps = getToggleButtonProps();
+  const toggleButtonProps = getToggleButtonProps({
+    onFocus: () => {
+      setInputFocused(true);
+    },
+    onBlur: () => {
+      setInputFocused(false);
+    },
+  });
   const mergedRef = mergeRefs(toggleButtonProps.ref, ref);
 
   const selectedItems = selectedItem as ItemType[];
@@ -406,6 +414,7 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
     [enabled ? null : containerClassName],
     {
       [`${prefix}--multi-select--invalid`]: invalid,
+      [`${prefix}--multi-select--invalid--focused`]: invalid && inputFocused,
       [`${prefix}--multi-select--warning`]: showWarning,
       [`${prefix}--multi-select--inline`]: inline,
       [`${prefix}--multi-select--selected`]:
@@ -464,12 +473,21 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
   }
 
   const onKeyDown = (e) => {
-    if (match(e, keys.Delete) && !disabled) {
-      clearSelection();
-      e.stopPropagation();
+    if (!disabled) {
+      if (match(e, keys.Delete) || match(e, keys.Escape)) {
+        clearSelection();
+        e.stopPropagation();
+      }
     }
   };
 
+  const multiSelectFieldWrapperClasses = cx(
+    `${prefix}--list-box__field--wrapper`,
+    {
+      [`${prefix}--list-box__field--wrapper--input-focused`]: inputFocused,
+    }
+  );
+
   const handleFocus = (evt: React.FocusEvent<HTMLDivElement>) => {
     evt.target.classList.contains(`${prefix}--tag__close-icon`)
       ? setIsFocused(false)
@@ -529,15 +547,7 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
             className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`}
           />
         )}
-        <button
-          type="button"
-          className={`${prefix}--list-box__field`}
-          disabled={disabled}
-          aria-disabled={disabled || readOnly}
-          {...toggleButtonProps}
-          ref={mergedRef}
-          onKeyDown={onKeyDown}
-          {...readOnlyEventHandlers}>
+        <div className={multiSelectFieldWrapperClasses}>
           {selectedItems.length > 0 && (
             <ListBox.Selection
               readOnly={readOnly}
@@ -548,14 +558,24 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
               disabled={disabled}
             />
           )}
-          <span id={fieldLabelId} className={`${prefix}--list-box__label`}>
-            {label}
-          </span>
-          <ListBox.MenuIcon
-            isOpen={!!isOpen}
-            translateWithId={translateWithId}
-          />
-        </button>
+          <button
+            type="button"
+            className={`${prefix}--list-box__field`}
+            disabled={disabled}
+            aria-disabled={disabled || readOnly}
+            {...toggleButtonProps}
+            ref={mergedRef}
+            onKeyDown={onKeyDown}
+            {...readOnlyEventHandlers}>
+            <span id={fieldLabelId} className={`${prefix}--list-box__label`}>
+              {label}
+            </span>
+            <ListBox.MenuIcon
+              isOpen={isOpen}
+              translateWithId={translateWithId}
+            />
+          </button>
+        </div>
         <ListBox.Menu aria-multiselectable="true" {...getMenuProps()}>
           {isOpen &&
             // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
@@ -737,7 +757,7 @@ MultiSelect.propTypes = {
 
   /**
    * `onMenuChange` is a utility for this controlled component to communicate to a
-   * consuming component that the menu was opend(`true`)/closed(`false`).
+   * consuming component that the menu was open(`true`)/closed(`false`).
    */
   onMenuChange: PropTypes.func,
 

diff --git a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss
@@ -19,6 +19,16 @@
 @use '../fluid-combo-box';
 
 @mixin fluid-multiselect {
+  .#{$prefix}--multi-select__wrapper.#{$prefix}--list-box__wrapper--fluid--focus:not(.#{$prefix}--multi-select--filterable__wrapper)
+    .#{$prefix}--list-box__field--wrapper--input-focused {
+    outline: none;
+  }
+
+  .#{$prefix}--list-box__wrapper--fluid
+    .#{$prefix}--tag.#{$prefix}--tag--filter {
+    margin-top: 1.25rem;
+  }
+
   // Filterable
   .#{$prefix}--list-box__wrapper--fluid
     .#{$prefix}--multi-select--filterable--input-focused {

diff --git a/packages/styles/scss/components/form/_form.scss b/packages/styles/scss/components/form/_form.scss
@@ -125,7 +125,7 @@ $input-label-weight: 400 !default;
     > .#{$prefix}--text-area--invalid:not(:focus),
   .#{$prefix}--select-input__wrapper[data-invalid]
     .#{$prefix}--select-input:not(:focus),
-  .#{$prefix}--list-box[data-invalid]:not(:focus),
+  .#{$prefix}--list-box[data-invalid]:not(.#{$prefix}--multi-select--invalid--focused),
   .#{$prefix}--combo-box[data-invalid]:not(.#{$prefix}--multi-select--selected)
     .#{$prefix}--text-input:not(:focus) {
     @include focus-outline('invalid');

diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss
@@ -17,12 +17,19 @@
 /// @access public
 /// @group multi-select
 @mixin multiselect {
-  .#{$prefix}--multi-select .#{$prefix}--tag {
-    min-width: auto;
-    margin: 0 $spacing-03 0 0;
+  .#{$prefix}--multi-select .#{$prefix}--list-box__field--wrapper {
+    display: inline-flex;
+    width: 100%;
+    height: calc(100% + 1px);
+    align-items: center;
+  }
+
+  .#{$prefix}--multi-select .#{$prefix}--list-box__field:focus {
+    @include focus-outline('reset');
   }
 
-  .#{$prefix}--multi-select--filterable .#{$prefix}--tag {
+  .#{$prefix}--multi-select .#{$prefix}--tag {
+    min-width: auto;
     margin: 0 $spacing-03 0 $spacing-05;
   }
 
@@ -75,12 +82,16 @@
     outline: none;
   }
 
-  .#{$prefix}--multi-select--filterable--input-focused {
+  .#{$prefix}--multi-select--filterable--input-focused,
+  .#{$prefix}--multi-select
+    .#{$prefix}--list-box__field--wrapper--input-focused {
     @include focus-outline('outline');
   }
 
   .#{$prefix}--multi-select--filterable.#{$prefix}--multi-select--selected
-    .#{$prefix}--text-input {
+    .#{$prefix}--text-input,
+  .#{$prefix}--multi-select.#{$prefix}--multi-select--selected
+    .#{$prefix}--list-box__field {
     padding-left: 0;
   }