fix(ListBox): update focus states for listbox components (#15980)

* fix(ListBox): update focus states for listbox components * fix(ListBox): update fluid listbox components * test(Multiselect): update keybaord nav tests * fix(Multiselect): remove console.log * test(FluidMultiselect): update tests * fix(ComboBox): adjust mouse interactions * fix(Dropdown): adjust focus state on mouse leave * fix(MultiSelect): match mouse focus to keyboard, home/end interaction * fix(FilterableMultiselect): align keyboard, mouse focus interaction * style(ListBox): adjust outline offset when outline is only 1px * chore(console): remove console.log * chore(Multiselect): revert testing default state
carbon-design-system · Apr 3, 2024 · 801dd7b · 801dd7b
1 parent 53be4cb
commit 801dd7b
Show file tree

Hide file tree

Showing 13 changed files with 123 additions and 24 deletions.
diff --git a/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js b/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js
@@ -82,8 +82,7 @@ test.describe('@avt FluidMultiSelect', () => {
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
     // Navigation inside the menu
-    // move to first option
-    await page.keyboard.press('ArrowDown');
+    // Focus on first element by default
     await expect(
       page.getByRole('option', {
         name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.',
@@ -177,8 +176,7 @@ test.describe('@avt FluidMultiSelect', () => {
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
     // Navigation inside the menu
-    // move to first option
-    await page.keyboard.press('ArrowDown');
+    // Focus on first element by default
     await expect(
       page.getByRole('option', {
         name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.',

diff --git a/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js b/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js
@@ -113,8 +113,7 @@ test.describe('@avt MultiSelect', () => {
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
     // Navigation inside the menu
-    // move to first option
-    await page.keyboard.press('ArrowDown');
+    // Focus on first element by default
     await expect(
       page.getByRole('option', {
         name: 'An example option that is really long to show what should be done to handle long text',
@@ -206,8 +205,7 @@ test.describe('@avt MultiSelect', () => {
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
     // Navigation inside the menu
-    // move to first option
-    await page.keyboard.press('ArrowDown');
+    // Focus on first element by default
     await expect(
       page.getByRole('option', {
         name: 'An example option that is really long to show what should be done to handle long text',

diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx
@@ -53,9 +53,11 @@ const {
   keyDownArrowUp,
   keyDownEscape,
   clickButton,
+  clickItem,
   blurButton,
   changeInput,
   blurInput,
+  unknown,
 } = Downshift.stateChangeTypes;
 
 const defaultItemToString = <ItemType,>(item: ItemType | null) => {
@@ -450,15 +452,16 @@ const ComboBox = forwardRef(
       switch (type) {
         case keyDownArrowDown:
         case keyDownArrowUp:
-          setHighlightedIndex(changes.highlightedIndex);
+          if (changes.isOpen) {
+            updateHighlightedIndex(getHighlightedIndex(changes));
+          } else {
+            setHighlightedIndex(changes.highlightedIndex);
+          }
           break;
         case blurButton:
         case keyDownEscape:
           setHighlightedIndex(changes.highlightedIndex);
           break;
-        case clickButton:
-          setHighlightedIndex(changes.highlightedIndex);
-          break;
         case changeInput:
           updateHighlightedIndex(getHighlightedIndex(changes));
           break;
@@ -470,6 +473,11 @@ const ComboBox = forwardRef(
             }
           }
           break;
+        case clickButton:
+        case clickItem:
+        case unknown:
+          setHighlightedIndex(getHighlightedIndex(changes));
+          break;
       }
     };
 

diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx
@@ -49,6 +49,7 @@ const {
   ToggleButtonKeyDownHome,
   ToggleButtonKeyDownEnd,
   ItemMouseMove,
+  MenuMouseLeave,
 } = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & {
   ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick;
 };
@@ -303,6 +304,7 @@ const Dropdown = React.forwardRef(
           }
           return changes;
         case ItemMouseMove:
+        case MenuMouseLeave:
           return { ...changes, highlightedIndex: state.highlightedIndex };
       }
       return changes;

diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js
@@ -149,6 +149,10 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
     if (onMenuChange) {
       onMenuChange(nextIsOpen);
     }
+
+    if (!isOpen) {
+      setHighlightedIndex(0);
+    }
   }
 
   function handleOnOuterClick() {
@@ -173,6 +177,34 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
         break;
       case stateChangeTypes.keyDownEscape:
         handleOnMenuChange(false);
+        setHighlightedIndex(0);
+        break;
+      case stateChangeTypes.changeInput:
+        setHighlightedIndex(0);
+        break;
+      case stateChangeTypes.keyDownEnter:
+        if (!isOpen) {
+          setHighlightedIndex(0);
+        }
+        break;
+      case stateChangeTypes.clickItem:
+        if (isOpen) {
+          const sortedItems = sortItems(
+            filterItems(items, { itemToString, inputValue }),
+            {
+              selectedItems: {
+                top: changes.selectedItems,
+                fixed: [],
+                'top-after-reopen': topItems,
+              }[selectionFeedback],
+              itemToString,
+              compareItems,
+              locale,
+            }
+          );
+          const sortedSelectedIndex = sortedItems.indexOf(changes.selectedItem);
+          setHighlightedIndex(sortedSelectedIndex);
+        }
         break;
     }
   }

diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx
@@ -45,9 +45,11 @@ const {
   ToggleButtonKeyDownEscape,
   ToggleButtonKeyDownSpaceButton,
   ItemMouseMove,
+  MenuMouseLeave,
   ToggleButtonClick,
-  ToggleButtonKeyDownHome,
-  ToggleButtonKeyDownEnd,
+  ToggleButtonKeyDownPageDown,
+  ToggleButtonKeyDownPageUp,
+  FunctionSetHighlightedIndex,
 } = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & {
   ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick;
 };
@@ -400,6 +402,7 @@ const MultiSelect = React.forwardRef(
       getItemProps,
       selectedItem,
       highlightedIndex,
+      setHighlightedIndex,
     } = useSelect<ItemType>(selectProps);
 
     const toggleButtonProps = getToggleButtonProps({
@@ -426,6 +429,7 @@ const MultiSelect = React.forwardRef(
               match(e, keys.Enter)) &&
             !isOpen
           ) {
+            setHighlightedIndex(0);
             setItemsCleared(false);
             setIsOpenWrapper(true);
           }
@@ -522,7 +526,6 @@ const MultiSelect = React.forwardRef(
       }
 
       switch (type) {
-        case ItemClick:
         case ToggleButtonKeyDownSpaceButton:
         case ToggleButtonKeyDownEnter:
           if (changes.selectedItem === undefined) {
@@ -539,11 +542,29 @@ const MultiSelect = React.forwardRef(
           break;
         case ToggleButtonClick:
           setIsOpenWrapper(changes.isOpen || false);
-          break;
+          return { ...changes, highlightedIndex: 0 };
+        case ItemClick:
+          setHighlightedIndex(changes.selectedItem);
+          onItemChange(changes.selectedItem);
+          return { ...changes, highlightedIndex: state.highlightedIndex };
+        case MenuMouseLeave:
+          return { ...changes, highlightedIndex: state.highlightedIndex };
+        case FunctionSetHighlightedIndex:
+          if (!isOpen) {
+            return {
+              ...changes,
+              highlightedIndex: 0,
+            };
+          } else {
+            return {
+              ...changes,
+              highlightedIndex: props.items.indexOf(highlightedIndex),
+            };
+          }
         case ToggleButtonKeyDownArrowDown:
         case ToggleButtonKeyDownArrowUp:
-        case ToggleButtonKeyDownHome:
-        case ToggleButtonKeyDownEnd:
+        case ToggleButtonKeyDownPageDown:
+        case ToggleButtonKeyDownPageUp:
           if (highlightedIndex > -1) {
             const itemArray = document.querySelectorAll(
               `li.${prefix}--list-box__menu-item[role="option"]`

diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js
@@ -206,7 +206,6 @@ describe('MultiSelect', () => {
 
     expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'false');
 
-    await userEvent.keyboard('[ArrowDown]');
     await userEvent.keyboard('[Enter]');
 
     expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'true');

diff --git a/packages/styles/scss/components/combo-box/_combo-box.scss b/packages/styles/scss/components/combo-box/_combo-box.scss
@@ -8,6 +8,7 @@
 @use '../list-box';
 @use '../../config' as *;
 @use '../../theme' as *;
+@use '../../utilities/convert';
 @use '../../utilities/focus-outline' as *;
 
 /// Combo box styles
@@ -36,6 +37,12 @@
     @include focus-outline('outline');
   }
 
+  .#{$prefix}--list-box--expanded
+    .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
+    outline-offset: convert.to-rem(-1px);
+    outline-width: convert.to-rem(1px);
+  }
+
   .#{$prefix}--combo-box .#{$prefix}--list-box__field,
   .#{$prefix}--combo-box.#{$prefix}--list-box[data-invalid]
     .#{$prefix}--list-box__field,

diff --git a/packages/styles/scss/components/dropdown/_dropdown.scss b/packages/styles/scss/components/dropdown/_dropdown.scss
@@ -93,6 +93,10 @@
     border-block-end-color: $border-subtle;
   }
 
+  .#{$prefix}--dropdown--open .cds--list-box__field {
+    outline: none;
+  }
+
   .#{$prefix}--dropdown--invalid {
     @include focus-outline('invalid');
 

diff --git a/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss b/packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss
@@ -69,4 +69,11 @@
     + .#{$prefix}--list-box__invalid-icon {
     inset-inline-end: 1rem;
   }
+
+  .#{$prefix}--list-box__wrapper--fluid
+    :not(.#{$prefix}--list-box--up)
+    .#{$prefix}--combo-box
+    .#{$prefix}--list-box__menu {
+    inset-block-start: calc(100% + convert.to-rem(1px));
+  }
 }
diff --git a/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss b/packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss
@@ -114,15 +114,20 @@
     outline-offset: 0;
   }
 
+  .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
+      .#{$prefix}--list-box--expanded
+    ) {
+    outline-width: convert.to-rem(1px);
+  }
+
   .#{$prefix}--list-box__wrapper--fluid .#{$prefix}--list-box__field:focus {
     outline: none;
     outline-offset: 0;
   }
 
-  .#{$prefix}--list-box__wrapper--fluid
-    :not(.#{$prefix}--list-box--up)
+  .#{$prefix}--list-box__wrapper--fluid:not(.#{$prefix}--list-box--up)
     .#{$prefix}--list-box__menu {
-    inset-block-start: calc(100% + convert.to-rem(1px));
+    inset-block-start: calc(100%);
   }
 
   // Invalid / Warning styles
@@ -235,8 +240,7 @@
   }
 
   // direction - top
-  .#{$prefix}--list-box__wrapper--fluid
-    .#{$prefix}--list-box--up
+  .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box--up
     .#{$prefix}--list-box__menu {
     inset-block-end: $spacing-10;
   }

diff --git a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss
@@ -12,6 +12,7 @@
 @use '../../motion' as *;
 @use '../../spacing' as *;
 @use '../../theme' as *;
+@use '../../utilities/convert';
 @use '../../utilities/focus-outline' as *;
 @use '../multiselect';
 @use '../fluid-list-box';
@@ -49,4 +50,11 @@
     .#{$prefix}--list-box__field {
     align-items: baseline;
   }
+
+  .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--multi-select--filterable__wrapper:not(
+      .#{$prefix}--list-box--up
+    )
+    .#{$prefix}--list-box__menu {
+    inset-block-start: calc(100% + convert.to-rem(1px));
+  }
 }
diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss
@@ -93,6 +93,17 @@
     @include focus-outline('outline');
   }
 
+  .#{$prefix}--multi-select.#{$prefix}--list-box--expanded
+    .#{$prefix}--list-box__field--wrapper:has(
+      button[aria-activedescendant]:not([aria-activedescendant=''])
+    ),
+  .#{$prefix}--multi-select--filterable.#{$prefix}--list-box--expanded:has(
+      input[aria-activedescendant]:not([aria-activedescendant=''])
+    ) {
+    outline-offset: convert.to-rem(-1px);
+    outline-width: convert.to-rem(1px);
+  }
+
   .#{$prefix}--multi-select--filterable.#{$prefix}--multi-select--selected
     .#{$prefix}--text-input,
   .#{$prefix}--multi-select.#{$prefix}--multi-select--selected