Removed highlight from Dropdown components (#16789)

* fix: removed highlight from first item * test: fixed tests to match new behaviour * test: fixed fluid components tests * fix: fixed click on arrow * fix: fixed focus state * fix: fixed filterable multiselect focus * fix: fixed focus state for combobox * fix: fixed scss * fix: fixed multiselect focus state * fix: removed console.log * fix: fixex arrowDown to open the menu * fix: fixed tests * fix: fixed issues --------- Co-authored-by: Kritvi <[email protected]>
carbon-design-system · Aug 6, 2024 · 5998863 · 5998863
1 parent e963ee5
commit 5998863
Show file tree

Hide file tree

Showing 13 changed files with 63 additions and 23 deletions.
diff --git a/e2e/components/ComboBox/ComboBox-test.avt.e2e.js b/e2e/components/ComboBox/ComboBox-test.avt.e2e.js
@@ -84,6 +84,7 @@ test.describe('@avt ComboBox', () => {
     await expect(combobox).toBeFocused();
     await page.keyboard.press('Enter');
     await expect(menu).toBeVisible();
+    await page.keyboard.press('ArrowDown');
     // Navigation inside the menu
     // move to first option
     await expect(optionOne).toHaveClass(
@@ -122,15 +123,17 @@ test.describe('@avt ComboBox', () => {
     await page.keyboard.press('Enter');
     await page.keyboard.press('ArrowDown');
     await page.keyboard.press('Enter');
-    await expect(combobox).toHaveValue('Option 1');
+    await expect(combobox).toHaveValue(
+      'An example option that is really long to show what should be done to handle long text'
+    );
     await page.keyboard.press('Escape');
 
     // should open and select option 2
     await page.keyboard.press('Enter');
     await page.keyboard.press('ArrowDown');
     await page.keyboard.press('ArrowDown');
     await page.keyboard.press('Enter');
-    await expect(combobox).toHaveValue('Option 2');
+    await expect(combobox).toHaveValue('Option 1');
     await page.keyboard.press('Escape');
   });
 });
diff --git a/e2e/components/FluidComboBox/FluidComboBox-test.avt.e2e.js b/e2e/components/FluidComboBox/FluidComboBox-test.avt.e2e.js
@@ -84,6 +84,7 @@ test.describe('@avt FluidComboBox', () => {
     await expect(combobox).toBeFocused();
     await page.keyboard.press('Enter');
     await expect(menu).toBeVisible();
+    await page.keyboard.press('ArrowDown');
     // Navigation inside the menu
     // move to first option
     await expect(optionOne).toHaveClass(

diff --git a/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js b/e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js
@@ -81,6 +81,7 @@ test.describe('@avt FluidMultiSelect', () => {
     await expect(toggleButton).toBeFocused();
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
+    await page.keyboard.press('ArrowDown');
     // Navigation inside the menu
     // Focus on first element by default
     await expect(
@@ -175,6 +176,7 @@ test.describe('@avt FluidMultiSelect', () => {
     await expect(toggleButton).toBeFocused();
     await page.keyboard.press('Space');
     await expect(menu).toBeVisible();
+    await page.keyboard.press('ArrowDown');
     // Navigation inside the menu
     // Focus on first element by default
     await expect(

diff --git a/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js b/e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js
@@ -118,10 +118,11 @@ test.describe('@avt MultiSelect', () => {
       page.getByRole('option', {
         name: 'An example option that is really long to show what should be done to handle long text',
       })
-    ).toHaveClass(
+    ).not.toHaveClass(
       'cds--list-box__menu-item cds--list-box__menu-item--highlighted'
     );
     // select first option (should select with enter and space)
+    await page.keyboard.press('ArrowDown');
     await page.keyboard.press('Enter');
     await expect(
       page.getByRole('option', {

diff --git a/e2e/components/Toggle/Toggle-test.avt.e2e.js b/e2e/components/Toggle/Toggle-test.avt.e2e.js
@@ -63,11 +63,11 @@ test.describe('@avt Toggle', () => {
         theme: 'white',
       },
     });
+    const toggleSwitch = page.getByRole('switch');
     await page.keyboard.press('Tab');
-    await expect(page.getByRole('switch')).toBeVisible();
-    await page.keyboard.press('Space');
-    page.getByText('Off');
-    await page.keyboard.press('Space');
-    page.getByText('On');
+    await expect(toggleSwitch).toBeVisible();
+    await expect(toggleSwitch).toHaveAttribute('aria-checked', 'true');
+    await page.keyboard.press('Enter');
+    await expect(toggleSwitch).toHaveAttribute('aria-checked', 'false');
   });
 });
diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx
@@ -502,7 +502,7 @@ const ComboBox = forwardRef(
           case FunctionToggleMenu:
           case ToggleButtonClick:
             if (changes.isOpen && !changes.selectedItem) {
-              return { ...changes, highlightedIndex: 0 };
+              return { ...changes };
             }
             return changes;
 
@@ -575,7 +575,7 @@ const ComboBox = forwardRef(
 
     const inputClasses = cx(`${prefix}--text-input`, {
       [`${prefix}--text-input--empty`]: !inputValue,
-      [`${prefix}--combo-box--input--focus`]: isFocused && !isFluid,
+      [`${prefix}--combo-box--input--focus`]: isFocused,
     });
 
     // needs to be Capitalized for react to render it correctly

diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.tsx b/packages/react/src/components/MultiSelect/FilterableMultiSelect.tsx
@@ -487,9 +487,16 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
     }
   }, [controlledSelectedItems, isOpen, setTopItems]);
 
+  const validateHighlightFocus = () => {
+    if (controlledSelectedItems.length > 0) {
+      setHighlightedIndex(0);
+    }
+  };
+
   function handleMenuChange(forceIsOpen: boolean): void {
     const nextIsOpen = forceIsOpen ?? !isOpen;
     setIsOpen(nextIsOpen);
+    validateHighlightFocus();
     if (onMenuChange) {
       onMenuChange(nextIsOpen);
     }
@@ -508,8 +515,8 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
   } = useCombobox<ItemType>({
     isOpen,
     items: sortedItems,
+    // defaultHighlightedIndex: 0, // after selection, highlight the first item.
     itemToString,
-    defaultHighlightedIndex: 0, // after selection, highlight the first item.
     id,
     labelId,
     menuId,
@@ -520,7 +527,6 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
       return (item as any).disabled;
     },
   });
-
   function stateReducer(state, actionAndChanges) {
     const { type, props, changes } = actionAndChanges;
     const { highlightedIndex } = changes;
@@ -548,20 +554,30 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
         return changes;
       case FunctionToggleMenu:
       case ToggleButtonClick:
+        validateHighlightFocus();
         if (changes.isOpen && !changes.selectedItem) {
-          return { ...changes, highlightedIndex: 0 };
+          return { ...changes };
         }
-        return changes;
+
+        return { ...changes, highlightedIndex: null };
       case InputChange:
         if (onInputValueChange) {
           onInputValueChange(changes.inputValue);
         }
         setInputValue(changes.inputValue ?? '');
         setIsOpen(true);
-        return changes;
+        return { ...changes, highlightedIndex: 0 };
 
       case InputClick:
-        return { ...changes, isOpen: false };
+        validateHighlightFocus();
+        if (changes.isOpen && !changes.selectedItem) {
+          return { ...changes };
+        }
+        return {
+          ...changes,
+          isOpen: false,
+          highlightedIndex: null,
+        };
       case MenuMouseLeave:
         return { ...changes, highlightedIndex: state.highlightedIndex };
       case InputKeyDownArrowUp:

diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx
@@ -568,7 +568,11 @@ const MultiSelect = React.forwardRef(
           break;
         case ToggleButtonClick:
           setIsOpenWrapper(changes.isOpen || false);
-          return { ...changes, highlightedIndex: 0 };
+          return {
+            ...changes,
+            highlightedIndex:
+              controlledSelectedItems.length > 0 ? 0 : undefined,
+          };
         case ItemClick:
           setHighlightedIndex(changes.selectedItem);
           onItemChange(changes.selectedItem);

diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js
@@ -246,6 +246,8 @@ describe('MultiSelect', () => {
 
     expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'false');
 
+    await userEvent.keyboard('[Enter]');
+    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
@@ -37,7 +37,9 @@
     @include focus-outline('outline');
   }
 
-  .#{$prefix}--list-box--expanded
+  .#{$prefix}--combo-box.#{$prefix}--list-box--expanded:has(
+      input[aria-activedescendant]:not([aria-activedescendant=''])
+    )
     .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
     outline-offset: convert.to-rem(-1px);
     outline-width: convert.to-rem(1px);

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
@@ -37,10 +37,13 @@
     white-space: nowrap;
   }
 
-  .#{$prefix}--list-box__wrapper--fluid
-    .#{$prefix}--combo-box
-    .#{$prefix}--text-input:focus {
-    outline: none;
+  .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus
+    .#{$prefix}--combo-box.#{$prefix}--list-box--expanded:has(
+      input[aria-activedescendant]:not([aria-activedescendant=''])
+    )
+    .#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
+    outline-offset: convert.to-rem(-1px);
+    outline-width: convert.to-rem(1px);
   }
 
   .#{$prefix}--list-box__wrapper--fluid

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,6 +114,12 @@
     outline-offset: 0;
   }
 
+  .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
+      .#{$prefix}--combo-box
+    ) {
+    outline: none;
+  }
+
   .#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
       .#{$prefix}--list-box--expanded
     ) {

diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss
@@ -92,7 +92,7 @@
     .#{$prefix}--list-box__field--wrapper--input-focused:has(
       button[aria-expanded='false']
     ),
-  .#{$prefix}--multi-select.#{$prefix}--multi-select--selected
+  .#{$prefix}--multi-select
     .#{$prefix}--list-box__field--wrapper--input-focused:has(
       button[aria-expanded='true']
     ) {