fix: Highlight the first select option by default if none are selected (#3546)

Author: avinashbot

src/internal/components/options-list/utils/use-highlight-option.ts

@@ -23,6 +23,7 @@ export interface HighlightedOptionState<OptionType> {
 export interface HighlightedOptionHandlers<OptionType> {
   // Mouse handlers
   setHighlightedIndexWithMouse(index: number, moveFocus?: boolean): void;
+  highlightFirstOptionWithMouse(): void;
   // Keyboard handlers
   moveHighlightWithKeyboard(direction: -1 | 1): void;
   highlightOptionWithKeyboard(option: OptionType): void;
@@ -73,6 +74,7 @@ export function useHighlightedOption<OptionType>({
     {
       setHighlightedIndexWithMouse: (index: number, moveFocus = false) =>
         setHighlightedIndex(index, new HighlightType('mouse', moveFocus)),
+      highlightFirstOptionWithMouse: () => moveHighlightFrom(1, -1, new HighlightType('mouse', true)),
       moveHighlightWithKeyboard: (direction: -1 | 1) => moveHighlight(direction, new HighlightType('keyboard')),
       highlightOptionWithKeyboard: (option: OptionType) => highlightOption(option, new HighlightType('keyboard')),
       resetHighlightWithKeyboard: () => setHighlightedIndex(-1, new HighlightType('keyboard')),

src/multiselect/__tests__/multiselect-embedded.test.tsx

@@ -63,7 +63,10 @@ test.each([false, true])('renders options, virtualScroll=%s', virtualScroll => {
   const items = createWrapper()
     .findAllByClassName(selectableItemsStyles['selectable-item'])
     .map(w => w.getElement());
-  expect(items.map(item => item.textContent)).toEqual(['First', 'Second', 'Third', 'Fourth']);
+
+  ['First', 'Second', 'Third', 'Fourth'].forEach((expected, i) => {
+    expect(items[i]).toHaveTextContent(expected);
+  });
 });
 
 test.each([

src/multiselect/__tests__/multiselect.test.tsx

@@ -755,7 +755,10 @@ describe('Disabled item with reason', () => {
   test('has no tooltip open by default', () => {
     const { wrapper } = renderMultiselect(
       <Multiselect
-        options={[{ label: 'First', value: '1', disabled: true, disabledReason: 'disabled reason' }]}
+        options={[
+          { label: 'First', value: '1', disabled: false },
+          { label: 'Second', value: '2', disabled: true, disabledReason: 'disabled reason' },
+        ]}
         selectedOptions={[]}
       />
     );
@@ -777,14 +780,17 @@ describe('Disabled item with reason', () => {
   test('open tooltip when the item is highlighted', () => {
     const { wrapper } = renderMultiselect(
       <Multiselect
-        options={[{ label: 'First', value: '1', disabled: true, disabledReason: 'disabled reason' }]}
+        options={[
+          { label: 'First', value: '1', disabled: false },
+          { label: 'Second', value: '2', disabled: true, disabledReason: 'disabled reason' },
+        ]}
         selectedOptions={[]}
       />
     );
     wrapper.openDropdown();
-    wrapper.findTrigger().keydown(KeyCode.down);
+    wrapper.findDropdown().findOptionsContainer()!.keydown(KeyCode.down);
 
-    expect(wrapper.findDropdown().findOption(1)!.findDisabledReason()!.getElement()).toHaveTextContent(
+    expect(wrapper.findDropdown().findOption(2)!.findDisabledReason()!.getElement()).toHaveTextContent(
       'disabled reason'
     );
   });

src/multiselect/__tests__/select-all.test.tsx

@@ -154,20 +154,18 @@ describe('Multiselect with "select all" control', () => {
       wrapper.openDropdown();
       const dropdown = wrapper.findDropdown();
       const optionsContainer = dropdown.findOptionsContainer()!;
-      // When opening the dropdown no option gets highlighted if none is selected. Move one position down to highlight the "Select all" control.
-      optionsContainer.keydown(KeyCode.down);
+      // When opening the dropdown the first option ("Select all") gets highlighted if none is selected.
       optionsContainer.keydown(KeyCode.space);
       expect(dropdown.findOptionByValue('1')).toBeNull();
     });
 
-    describe.each([false, true])('virtuaScroll=%s', virtualScroll => {
+    describe.each([false, true])('virtualScroll=%s', virtualScroll => {
       test('selects all options when none are selected', () => {
         const onChange = jest.fn();
         const wrapper = renderMultiselectWithSelectAll({ onChange, virtualScroll });
         wrapper.openDropdown();
         const optionsContainer = wrapper.findDropdown().findOptionsContainer()!;
-        // When opening the dropdown no option gets highlighted if none is selected. Move one position down to highlight the "Select all" control.
-        optionsContainer.keydown(KeyCode.down);
+        // When opening the dropdown the first option ("Select all") gets highlighted if none is selected.
         optionsContainer.keydown(KeyCode.space);
         expect(onChange).toHaveBeenCalledWith(
           expect.objectContaining({ detail: { selectedOptions: optionsWithoutGroups } })
@@ -187,14 +185,14 @@ describe('Multiselect with "select all" control', () => {
       const wrapper = createWrapper(container).findMultiselect()!;
       wrapper.openDropdown();
       const selectAll = wrapper.findDropdown().findSelectAll()!;
-      expect(selectAll.getElement().textContent).toBe('Custom Select all text');
+      expect(selectAll.getElement()).toHaveTextContent('Custom Select all text');
     });
 
     test('uses i18nStrings.selectAllText from i18nStrings prop', () => {
       const wrapper = renderMultiselectWithSelectAll({ i18nStrings: { selectAllText: 'Custom Select all text' } });
       wrapper.openDropdown();
       const selectAll = wrapper.findDropdown().findSelectAll()!;
-      expect(selectAll.getElement().textContent).toBe('Custom Select all text');
+      expect(selectAll.getElement()).toHaveTextContent('Custom Select all text');
     });
 
     test('uses i18nStrings.selectAllText from i18nStrings prop when both i18n provider and i18nStrings prop are provided ', () => {
@@ -214,7 +212,7 @@ describe('Multiselect with "select all" control', () => {
       const wrapper = createWrapper(container).findMultiselect()!;
       wrapper.openDropdown();
       const selectAll = wrapper.findDropdown().findSelectAll()!;
-      expect(selectAll.getElement().textContent).toBe('Custom Select all text from i18nStrings');
+      expect(selectAll.getElement()).toHaveTextContent('Custom Select all text from i18nStrings');
     });
   });
 

src/select/__integ__/page-objects/select-page.ts

@@ -84,7 +84,7 @@ export default class SelectPageObject<
     assert.equal(
       await this.isExisting(this.wrapper.findDropdown().findHighlightedOption().toSelector()),
       isDisplayed,
-      `There should ${isDisplayed ? '' : 'not '} be a highlighted option`
+      `There should ${isDisplayed ? '' : 'not '}be a highlighted option`
     );
   }
 

src/select/__integ__/select-native-search.test.ts

@@ -73,15 +73,6 @@ describe(`Select (Native Search)`, () => {
       })
     );
 
-    test(
-      'no match - no highlight, with dropdown open',
-      setupTest(optionsType, async page => {
-        await page.clickSelect();
-        await page.keys(['a']);
-        await page.assertHighlightedOption(false);
-      })
-    );
-
     test(
       'no match - no selection, with dropdown closed',
       setupTest(optionsType, async page => {
@@ -112,15 +103,6 @@ describe(`Select (Native Search)`, () => {
         await expect(page.getTriggerLabel()).resolves.toMatch('Third thing');
       })
     );
-
-    test(
-      'no highlight when the option does not start with the search substring',
-      setupTest(optionsType, async page => {
-        await page.clickSelect();
-        await page.keys(['a']);
-        await page.assertHighlightedOption(false);
-      })
-    );
   });
 
   describe('Options - Semi-extended', () => {

src/select/__integ__/select.test.ts

@@ -280,8 +280,8 @@ test(
     await browser.url('/#/light/select/select.test.scroll-padding');
     const page = new SelectPageObject(browser, createWrapper().findSelect());
     await page.setWindowSize({ width: 800, height: 250 });
-    await page.windowScrollTo({ top: 25 });
+    await page.windowScrollTo({ top: 15 });
     await page.clickSelect();
-    await expect(page.getWindowScroll()).resolves.toEqual({ top: 25, left: 0 });
+    await expect(page.getWindowScroll()).resolves.toEqual({ top: 15, left: 0 });
   })
 );

src/select/__tests__/disabled.test.tsx

@@ -52,32 +52,41 @@ describe.each([false, true])('expandToViewport=%s', expandToViewport => {
     describe('Disabled item with reason', () => {
       test('has no tooltip open by default', () => {
         const { wrapper } = renderSelect({
-          options: [{ label: 'First', value: '1', disabled: true, disabledReason: 'disabled reason' }],
+          options: [
+            { label: 'First', value: '1' },
+            { label: 'Second', value: '2', disabled: true, disabledReason: 'disabled reason' },
+          ],
         });
         wrapper.openDropdown();
 
-        expect(wrapper.findDropdown({ expandToViewport }).findOption(1)!.findDisabledReason()).toBe(null);
+        expect(wrapper.findDropdown({ expandToViewport }).findOption(2)!.findDisabledReason()).toBe(null);
       });
 
       test('has no tooltip without disabledReason', () => {
         const { wrapper } = renderSelect({
-          options: [{ label: 'First', value: '1', disabled: true }],
+          options: [
+            { label: 'First', value: '1' },
+            { label: 'Second', value: '2', disabled: true },
+          ],
         });
         wrapper.openDropdown();
-        wrapper.findTrigger()!.keydown(KeyCode.down);
+        wrapper.findDropdown({ expandToViewport }).findOptionsContainer()!.keydown(KeyCode.down);
 
-        expect(wrapper.findDropdown({ expandToViewport }).findOption(1)!.findDisabledReason()).toBe(null);
+        expect(wrapper.findDropdown({ expandToViewport }).findOption(2)!.findDisabledReason()).toBe(null);
       });
 
       test('open tooltip when the item is highlighted', () => {
         const { wrapper } = renderSelect({
-          options: [{ label: 'First', value: '1', disabled: true, disabledReason: 'disabled reason' }],
+          options: [
+            { label: 'First', value: '1' },
+            { label: 'Second', value: '2', disabled: true, disabledReason: 'disabled reason' },
+          ],
         });
         wrapper.openDropdown();
-        wrapper.findTrigger().keydown(KeyCode.down);
+        wrapper.findDropdown({ expandToViewport }).findOptionsContainer()!.keydown(KeyCode.down);
 
         expect(
-          wrapper.findDropdown({ expandToViewport }).findOption(1)!.findDisabledReason()!.getElement()
+          wrapper.findDropdown({ expandToViewport }).findOption(2)!.findDisabledReason()!.getElement()
         ).toHaveTextContent('disabled reason');
       });
 

src/select/__tests__/use-select.test.ts

@@ -186,6 +186,24 @@ describe('useSelect', () => {
     expect(testEvent.defaultPrevented).toBe(true);
   });
 
+  test('should highlight the first option by default (keyboard:enter)', () => {
+    const hook = renderHook(useSelect, { initialProps: { ...initialProps, filteringType: 'none' } });
+
+    const { getTriggerProps } = hook.result.current;
+    const triggerProps = getTriggerProps();
+    act(() => triggerProps.onKeyDown && triggerProps.onKeyDown(createTestEvent(KeyCode.enter)));
+    expect(hook.result.current.isOpen).toBe(true);
+    expect(hook.result.current.highlightedOption).toEqual({
+      type: 'child',
+      option: {
+        label: 'Child 1',
+        value: 'child1',
+      },
+    });
+    expect(hook.result.current.highlightType.type).toBe('keyboard');
+    expect(hook.result.current.highlightType.moveFocus).toBe(true);
+  });
+
   test('should open and navigate to the first option (keyboard:down)', () => {
     const hook = renderHook(useSelect, {
       initialProps,
@@ -283,6 +301,24 @@ describe('useSelect', () => {
     expect(hook.result.current.highlightType.moveFocus).toBe(true);
   });
 
+  test('should highlight the first option by default (mouse)', () => {
+    const hook = renderHook(useSelect, { initialProps: { ...initialProps, filteringType: 'none' } });
+
+    const { getTriggerProps } = hook.result.current;
+    const triggerProps = getTriggerProps();
+    act(() => triggerProps.onMouseDown && triggerProps.onMouseDown(createCustomEvent({})));
+    expect(hook.result.current.isOpen).toBe(true);
+    expect(hook.result.current.highlightedOption).toEqual({
+      type: 'child',
+      option: {
+        label: 'Child 1',
+        value: 'child1',
+      },
+    });
+    expect(hook.result.current.highlightType.type).toBe('mouse');
+    expect(hook.result.current.highlightType.moveFocus).toBe(true);
+  });
+
   test('should open and highlight the selected option (mouse)', () => {
     const hook = renderHook(useSelect, {
       initialProps: { ...initialProps, filteringType: 'none', selectedOptions: [initialProps.options[1].option] },

src/select/utils/use-select.ts

@@ -89,6 +89,7 @@ export function useSelect({
       resetHighlightWithKeyboard,
       setHighlightedIndexWithMouse,
       highlightOptionWithKeyboard,
+      highlightFirstOptionWithMouse,
       goHomeWithKeyboard,
       goEndWithKeyboard,
     },
@@ -300,11 +301,20 @@ export function useSelect({
   useEffect(() => {
     // highlight the first selected option, when opening the Select component without filter input
     // keep the focus in the filter input when opening, so that screenreader can recognize the combobox
-    if (isOpen && !prevOpen && hasSelectedOption && !hasFilter) {
+    if (isOpen && !prevOpen && options.length > 0 && !hasFilter) {
       if (openedWithKeyboard) {
-        highlightOptionWithKeyboard(__selectedOptions[0]);
+        if (__selectedOptions[0]) {
+          highlightOptionWithKeyboard(__selectedOptions[0]);
+        } else {
+          goHomeWithKeyboard();
+        }
       } else {
-        setHighlightedIndexWithMouse(options.indexOf(__selectedOptions[0]), true);
+        if (!__selectedOptions[0] || !options.includes(__selectedOptions[0])) {
+          highlightFirstOptionWithMouse();
+        } else {
+          const highlightedIndex = options.indexOf(__selectedOptions[0]);
+          setHighlightedIndexWithMouse(highlightedIndex, true);
+        }
       }
     }
   }, [
@@ -313,6 +323,8 @@ export function useSelect({
     hasSelectedOption,
     setHighlightedIndexWithMouse,
     highlightOptionWithKeyboard,
+    highlightFirstOptionWithMouse,
+    goHomeWithKeyboard,
     openedWithKeyboard,
     options,
     prevOpen,