fix(highlightedIndex): do not highlight disabled items (#1601)

downshift-js · May 17, 2024 · 4bf894b · 4bf894b
1 parent 602ee51
commit 4bf894b
Show file tree

Hide file tree

Showing 14 changed files with 531 additions and 80 deletions.
diff --git a/src/hooks/reducer.js b/src/hooks/reducer.js
@@ -1,4 +1,8 @@
-import {getHighlightedIndexOnOpen, getDefaultValue} from './utils'
+import {
+  getHighlightedIndexOnOpen,
+  getDefaultValue,
+  getDefaultHighlightedIndex,
+} from './utils'
 
 export default function downshiftCommonReducer(
   state,
@@ -46,7 +50,12 @@ export default function downshiftCommonReducer(
       break
     case stateChangeTypes.FunctionSetHighlightedIndex:
       changes = {
-        highlightedIndex: action.highlightedIndex,
+        highlightedIndex: props.isItemDisabled(
+          props.items[action.highlightedIndex],
+          action.highlightedIndex,
+        )
+          ? -1
+          : action.highlightedIndex,
       }
 
       break
@@ -58,7 +67,7 @@ export default function downshiftCommonReducer(
       break
     case stateChangeTypes.FunctionReset:
       changes = {
-        highlightedIndex: getDefaultValue(props, 'highlightedIndex'),
+        highlightedIndex: getDefaultHighlightedIndex(props),
         isOpen: getDefaultValue(props, 'isOpen'),
         selectedItem: getDefaultValue(props, 'selectedItem'),
         inputValue: getDefaultValue(props, 'inputValue'),

diff --git a/src/hooks/useCombobox/__tests__/__snapshots__/getInputProps.test.js.snap b/src/hooks/useCombobox/__tests__/__snapshots__/getInputProps.test.js.snap
@@ -0,0 +1,61 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`getInputProps event handlers on key down arrow down defaultHighlightedIndex is ignored if item is disabled 1`] = `
+[
+  [
+    Americium,
+    2,
+  ],
+  [
+    Americium,
+    2,
+  ],
+  [
+    Neptunium,
+    0,
+  ],
+]
+`;
+
+exports[`getInputProps event handlers on key down arrow down initialHighlightedIndex is ignored if item is disabled 1`] = `
+[
+  [
+    Americium,
+    2,
+  ],
+  [
+    Neptunium,
+    0,
+  ],
+]
+`;
+
+exports[`getInputProps event handlers on key down arrow up defaultHighlightedIndex is ignored if item is disabled 1`] = `
+[
+  [
+    Americium,
+    2,
+  ],
+  [
+    Americium,
+    2,
+  ],
+  [
+    Oganesson,
+    25,
+  ],
+]
+`;
+
+exports[`getInputProps event handlers on key down arrow up initialHighlightedIndex is ignored if item is disabled 1`] = `
+[
+  [
+    Americium,
+    2,
+  ],
+  [
+    Oganesson,
+    25,
+  ],
+]
+`;
diff --git a/src/hooks/useCombobox/__tests__/getInputProps.test.js b/src/hooks/useCombobox/__tests__/getInputProps.test.js
@@ -436,47 +436,64 @@ describe('getInputProps', () => {
   })
 
   describe('event handlers', () => {
-    test('on change should open the menu and keep the input value', async () => {
-      renderCombobox()
+    describe('on change', () => {
+      test('should open the menu and keep the input value', async () => {
+        renderCombobox()
 
-      await changeInputValue('california')
+        await changeInputValue('california')
 
-      expect(getItems()).toHaveLength(items.length)
-      expect(getInput()).toHaveValue('california')
-    })
+        expect(getItems()).toHaveLength(items.length)
+        expect(getInput()).toHaveValue('california')
+      })
 
-    test('on change should remove the highlightedIndex', async () => {
-      renderCombobox({initialHighlightedIndex: 2})
+      test('should remove the highlightedIndex', async () => {
+        renderCombobox({initialHighlightedIndex: 2})
 
-      await changeInputValue('california')
+        await changeInputValue('california')
 
-      expect(getInput()).toHaveAttribute('aria-activedescendant', '')
-    })
+        expect(getInput()).toHaveAttribute('aria-activedescendant', '')
+      })
 
-    test('on change should reset to defaultHighlightedIndex', async () => {
-      const defaultHighlightedIndex = 2
-      renderCombobox({defaultHighlightedIndex})
+      test('should reset to defaultHighlightedIndex', async () => {
+        const defaultHighlightedIndex = 2
+        renderCombobox({defaultHighlightedIndex})
 
-      await changeInputValue('a')
+        await changeInputValue('a')
 
-      expect(getInput()).toHaveAttribute(
-        'aria-activedescendant',
-        defaultIds.getItemId(defaultHighlightedIndex),
-      )
+        expect(getInput()).toHaveAttribute(
+          'aria-activedescendant',
+          defaultIds.getItemId(defaultHighlightedIndex),
+        )
 
-      await keyDownOnInput('{ArrowDown}')
+        await keyDownOnInput('{ArrowDown}')
 
-      expect(getInput()).toHaveAttribute(
-        'aria-activedescendant',
-        defaultIds.getItemId(defaultHighlightedIndex + 1),
-      )
+        expect(getInput()).toHaveAttribute(
+          'aria-activedescendant',
+          defaultIds.getItemId(defaultHighlightedIndex + 1),
+        )
 
-      await changeInputValue('a')
+        await changeInputValue('a')
 
-      expect(getInput()).toHaveAttribute(
-        'aria-activedescendant',
-        defaultIds.getItemId(defaultHighlightedIndex),
-      )
+        expect(getInput()).toHaveAttribute(
+          'aria-activedescendant',
+          defaultIds.getItemId(defaultHighlightedIndex),
+        )
+      })
+
+      test('should not reset to defaultHighlightedIndex if disabled', async () => {
+        const defaultHighlightedIndex = 2
+        renderCombobox({
+          defaultHighlightedIndex,
+          isItemDisabled(_item, index) {
+            return index === defaultHighlightedIndex
+          },
+        })
+
+        await keyDownOnInput('{ArrowDown}')
+        await changeInputValue('a')
+
+        expect(getInput()).toHaveAttribute('aria-activedescendant', '')
+      })
     })
 
     describe('on key down', () => {
@@ -579,11 +596,14 @@ describe('getInputProps', () => {
 
         test('initialHighlightedIndex is ignored if item is disabled', async () => {
           const initialHighlightedIndex = 2
+          const isItemDisabled = jest
+            .fn()
+            .mockImplementation(
+              item => items.indexOf(item) === initialHighlightedIndex,
+            )
           renderCombobox({
             initialHighlightedIndex,
-            isItemDisabled(item) {
-              return items.indexOf(item) === initialHighlightedIndex
-            },
+            isItemDisabled,
           })
 
           await keyDownOnInput('{ArrowUp}')
@@ -592,6 +612,7 @@ describe('getInputProps', () => {
             'aria-activedescendant',
             defaultIds.getItemId(items.length - 1),
           )
+          expect(isItemDisabled.mock.calls.slice(0, 2)).toMatchSnapshot()
         })
 
         test('initialHighlightedIndex is ignored and defaultHighlightedIndex is chosen if enabled', async () => {
@@ -615,11 +636,14 @@ describe('getInputProps', () => {
 
         test('defaultHighlightedIndex is ignored if item is disabled', async () => {
           const defaultHighlightedIndex = 2
+          const isItemDisabled = jest
+            .fn()
+            .mockImplementation(
+              item => items.indexOf(item) === defaultHighlightedIndex,
+            )
           renderCombobox({
             defaultHighlightedIndex,
-            isItemDisabled(item) {
-              return items.indexOf(item) === defaultHighlightedIndex
-            },
+            isItemDisabled,
           })
 
           await keyDownOnInput('{ArrowUp}')
@@ -628,6 +652,7 @@ describe('getInputProps', () => {
             'aria-activedescendant',
             defaultIds.getItemId(items.length - 1),
           )
+          expect(isItemDisabled.mock.calls.slice(0, 3)).toMatchSnapshot()
         })
 
         test('both defaultHighlightedIndex and initialHighlightedIndex are ignored if items are disabled', async () => {
@@ -901,11 +926,14 @@ describe('getInputProps', () => {
 
         test('initialHighlightedIndex is ignored if item is disabled', async () => {
           const initialHighlightedIndex = 2
+          const isItemDisabled = jest
+            .fn()
+            .mockImplementation(
+              item => items.indexOf(item) === initialHighlightedIndex,
+            )
           renderCombobox({
             initialHighlightedIndex,
-            isItemDisabled(item) {
-              return items.indexOf(item) === initialHighlightedIndex
-            },
+            isItemDisabled,
           })
 
           await keyDownOnInput('{ArrowDown}')
@@ -914,6 +942,7 @@ describe('getInputProps', () => {
             'aria-activedescendant',
             defaultIds.getItemId(0),
           )
+          expect(isItemDisabled.mock.calls.slice(0, 2)).toMatchSnapshot()
         })
 
         test('initialHighlightedIndex is ignored and defaultHighlightedIndex is chosen if enabled', async () => {
@@ -937,11 +966,14 @@ describe('getInputProps', () => {
 
         test('defaultHighlightedIndex is ignored if item is disabled', async () => {
           const defaultHighlightedIndex = 2
+          const isItemDisabled = jest
+            .fn()
+            .mockImplementation(
+              item => items.indexOf(item) === defaultHighlightedIndex,
+            )
           renderCombobox({
             defaultHighlightedIndex,
-            isItemDisabled(item) {
-              return items.indexOf(item) === defaultHighlightedIndex
-            },
+            isItemDisabled,
           })
 
           await keyDownOnInput('{ArrowDown}')
@@ -950,6 +982,7 @@ describe('getInputProps', () => {
             'aria-activedescendant',
             defaultIds.getItemId(0),
           )
+          expect(isItemDisabled.mock.calls.slice(0, 3)).toMatchSnapshot()
         })
 
         test('both defaultHighlightedIndex and initialHighlightedIndex are ignored if items are disabled', async () => {
@@ -1859,7 +1892,7 @@ describe('getInputProps', () => {
         renderCombobox({
           initialIsOpen: true,
           initialHighlightedIndex,
-          environment: undefined
+          environment: undefined,
         })
 
         await tab()
@@ -1967,19 +2000,26 @@ describe('getInputProps', () => {
         )
       })
 
-
       test('initialHighlightedIndex is ignored if item is disabled', async () => {
         const initialHighlightedIndex = 2
+        const isItemDisabled = jest
+          .fn()
+          .mockImplementation(
+            item => items.indexOf(item) === initialHighlightedIndex,
+          )
         renderCombobox({
           initialHighlightedIndex,
-          isItemDisabled(item) {
-            return items.indexOf(item) === initialHighlightedIndex
-          },
+          isItemDisabled,
         })
 
         await clickOnInput()
 
         expect(getInput()).toHaveAttribute('aria-activedescendant', '')
+        expect(isItemDisabled).toHaveBeenNthCalledWith(
+          1,
+          items[initialHighlightedIndex],
+          initialHighlightedIndex,
+        )
       })
 
       test('initialHighlightedIndex is ignored and defaultHighlightedIndex is chosen if enabled', async () => {
@@ -2003,16 +2043,24 @@ describe('getInputProps', () => {
 
       test('defaultHighlightedIndex is ignored if item is disabled', async () => {
         const defaultHighlightedIndex = 2
+        const isItemDisabled = jest
+          .fn()
+          .mockImplementation(
+            item => items.indexOf(item) === defaultHighlightedIndex,
+          )
         renderCombobox({
           defaultHighlightedIndex,
-          isItemDisabled(item) {
-            return items.indexOf(item) === defaultHighlightedIndex
-          },
+          isItemDisabled,
         })
 
         await clickOnInput()
 
         expect(getInput()).toHaveAttribute('aria-activedescendant', '')
+        expect(isItemDisabled).toHaveBeenNthCalledWith(
+          1,
+          items[defaultHighlightedIndex],
+          defaultHighlightedIndex,
+        )
       })
 
       test('both defaultHighlightedIndex and initialHighlightedIndex are ignored if items are disabled', async () => {
@@ -2032,7 +2080,6 @@ describe('getInputProps', () => {
 
         expect(getInput()).toHaveAttribute('aria-activedescendant', '')
       })
-
     })
   })
 

diff --git a/src/hooks/useCombobox/__tests__/getItemProps.test.js b/src/hooks/useCombobox/__tests__/getItemProps.test.js
@@ -346,6 +346,25 @@ describe('getItemProps', () => {
           defaultIds.getItemId(defaultHighlightedIndex),
         )
       })
+
+      test('it selects the item and resets to user defined defaults, but considers disabled status for an item', async () => {
+        const index = 1
+        const defaultHighlightedIndex = 2
+        renderCombobox({
+          defaultIsOpen: true,
+          defaultHighlightedIndex,
+          isItemDisabled(_item, idx) {
+            return idx === defaultHighlightedIndex
+          },
+        })
+        const input = getInput()
+
+        await clickOnItemAtIndex(index)
+
+        expect(input).toHaveValue(items[index])
+        expect(getItems()).toHaveLength(items.length)
+        expect(input).toHaveAttribute('aria-activedescendant', '')
+      })
     })
   })