From 4bf894ba355f8c281bf4cea98fc32d01fbc3f8d7 Mon Sep 17 00:00:00 2001 From: Silviu Alexandru Avram Date: Fri, 17 May 2024 10:03:28 +0300 Subject: [PATCH] fix(highlightedIndex): do not highlight disabled items (#1601) --- src/hooks/reducer.js | 15 +- .../__snapshots__/getInputProps.test.js.snap | 61 ++++++++ .../__tests__/getInputProps.test.js | 147 ++++++++++++------ .../__tests__/getItemProps.test.js | 19 +++ src/hooks/useCombobox/__tests__/props.test.js | 42 +++++ .../useCombobox/__tests__/returnProps.test.js | 39 +++++ src/hooks/useCombobox/reducer.js | 6 +- .../getToggleButtonProps.test.js.snap | 61 ++++++++ .../useSelect/__tests__/getItemProps.test.js | 19 +++ .../__tests__/getToggleButtonProps.test.js | 68 +++++--- src/hooks/useSelect/__tests__/props.test.js | 42 +++++ .../useSelect/__tests__/returnProps.test.js | 36 +++++ src/hooks/useSelect/reducer.js | 3 +- src/hooks/utils.js | 53 ++++++- 14 files changed, 531 insertions(+), 80 deletions(-) create mode 100644 src/hooks/useCombobox/__tests__/__snapshots__/getInputProps.test.js.snap create mode 100644 src/hooks/useSelect/__tests__/__snapshots__/getToggleButtonProps.test.js.snap diff --git a/src/hooks/reducer.js b/src/hooks/reducer.js index b404910f..62117c01 100644 --- 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 new file mode 100644 index 00000000..605082fd --- /dev/null +++ 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 index bbf09f5a..7f6ad371 100644 --- 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 index 190f4004..8085e4d7 100644 --- 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', '') + }) }) }) diff --git a/src/hooks/useCombobox/__tests__/props.test.js b/src/hooks/useCombobox/__tests__/props.test.js index a93a95ae..76ea494f 100644 --- a/src/hooks/useCombobox/__tests__/props.test.js +++ b/src/hooks/useCombobox/__tests__/props.test.js @@ -1447,4 +1447,46 @@ describe('props', () => { `downshift: A component has changed the controlled prop "inputValue" to be uncontrolled. This prop should not switch from controlled to uncontrolled (or vice versa). Decide between using a controlled or uncontrolled Downshift element for the lifetime of the component. More info: https://github.com/downshift-js/downshift#control-props`, ) }) + + test('initialHighlightedIndex is ignored if item is disabled and menu is intially open', () => { + const initialHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === initialHighlightedIndex, + ) + renderCombobox({ + initialHighlightedIndex, + isItemDisabled, + initialIsOpen: true, + }) + + expect(getInput()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[initialHighlightedIndex], + initialHighlightedIndex, + ) + }) + + test('defaultHighlightedIndex is ignored if item is disabled and menu is intially open', () => { + const defaultHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === defaultHighlightedIndex, + ) + renderCombobox({ + defaultHighlightedIndex, + isItemDisabled, + initialIsOpen: true, + }) + + expect(getInput()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[defaultHighlightedIndex], + defaultHighlightedIndex, + ) + }) }) diff --git a/src/hooks/useCombobox/__tests__/returnProps.test.js b/src/hooks/useCombobox/__tests__/returnProps.test.js index 6bbb8b73..908d9ed5 100644 --- a/src/hooks/useCombobox/__tests__/returnProps.test.js +++ b/src/hooks/useCombobox/__tests__/returnProps.test.js @@ -90,6 +90,22 @@ describe('returnProps', () => { expect(result.current.highlightedIndex).toBe(2) }) + test('setHighlightedIndex does not set highlightedIndex if item is disabled', () => { + const highlightedIndex = 2 + const {result} = renderUseCombobox({ + initialIsOpen: true, + isItemDisabled(_item, index) { + return index === highlightedIndex + }, + }) + + act(() => { + result.current.setHighlightedIndex(highlightedIndex) + }) + + expect(result.current.highlightedIndex).toBe(-1) + }) + test('setInputValue sets inputValue', () => { const {result} = renderUseCombobox({}) @@ -163,6 +179,29 @@ describe('returnProps', () => { expect(result.current.isOpen).toBe(props.defaultIsOpen) expect(result.current.inputValue).toBe(props.defaultInputValue) }) + + test('reset does not set the defaultHighlightedIndex if item is disabled', () => { + const props = { + defaultIsOpen: false, + defaultHighlightedIndex: 3, + defaultSelectedItem: items[2], + isItemDisabled(_item, index) { + return index === 3 + }, + } + const {result} = renderUseCombobox(props) + + act(() => { + result.current.openMenu() + result.current.selectItem(items[4]) + result.current.setHighlightedIndex(1) + result.current.reset() + }) + + expect(result.current.selectedItem).toBe(props.defaultSelectedItem) + expect(result.current.highlightedIndex).toBe(-1) + expect(result.current.isOpen).toBe(props.defaultIsOpen) + }) }) describe('state and props', () => { diff --git a/src/hooks/useCombobox/reducer.js b/src/hooks/useCombobox/reducer.js index c8647e47..065df497 100644 --- a/src/hooks/useCombobox/reducer.js +++ b/src/hooks/useCombobox/reducer.js @@ -2,6 +2,7 @@ import { getHighlightedIndexOnOpen, getDefaultValue, getChangesOnSelection, + getDefaultHighlightedIndex, } from '../utils' import {getHighlightedIndex, getNonDisabledIndex} from '../../utils' import commonReducer from '../reducer' @@ -16,10 +17,11 @@ export default function downshiftUseComboboxReducer(state, action) { case stateChangeTypes.ItemClick: changes = { isOpen: getDefaultValue(props, 'isOpen'), - highlightedIndex: getDefaultValue(props, 'highlightedIndex'), + highlightedIndex: getDefaultHighlightedIndex(props), selectedItem: props.items[action.index], inputValue: props.itemToString(props.items[action.index]), } + break case stateChangeTypes.InputKeyDownArrowDown: if (state.isOpen) { @@ -135,7 +137,7 @@ export default function downshiftUseComboboxReducer(state, action) { case stateChangeTypes.InputChange: changes = { isOpen: true, - highlightedIndex: getDefaultValue(props, 'highlightedIndex'), + highlightedIndex: getDefaultHighlightedIndex(props), inputValue: action.inputValue, } break diff --git a/src/hooks/useSelect/__tests__/__snapshots__/getToggleButtonProps.test.js.snap b/src/hooks/useSelect/__tests__/__snapshots__/getToggleButtonProps.test.js.snap new file mode 100644 index 00000000..ea566b35 --- /dev/null +++ b/src/hooks/useSelect/__tests__/__snapshots__/getToggleButtonProps.test.js.snap @@ -0,0 +1,61 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`getToggleButtonProps event handlers on keydown arrow down defaultHighlightedIndex is ignored if item is disabled 1`] = ` +[ + [ + Americium, + 2, + ], + [ + Americium, + 2, + ], + [ + Neptunium, + 0, + ], +] +`; + +exports[`getToggleButtonProps event handlers on keydown arrow down initialHighlightedIndex is ignored if item is disabled 1`] = ` +[ + [ + Americium, + 2, + ], + [ + Neptunium, + 0, + ], +] +`; + +exports[`getToggleButtonProps event handlers on keydown arrow up defaultHighlightedIndex is ignored if item is disabled 1`] = ` +[ + [ + Americium, + 2, + ], + [ + Americium, + 2, + ], + [ + Oganesson, + 25, + ], +] +`; + +exports[`getToggleButtonProps event handlers on keydown arrow up initialHighlightedIndex is ignored if item is disabled 1`] = ` +[ + [ + Americium, + 2, + ], + [ + Oganesson, + 25, + ], +] +`; diff --git a/src/hooks/useSelect/__tests__/getItemProps.test.js b/src/hooks/useSelect/__tests__/getItemProps.test.js index 4643f256..1f4a0e51 100644 --- a/src/hooks/useSelect/__tests__/getItemProps.test.js +++ b/src/hooks/useSelect/__tests__/getItemProps.test.js @@ -374,6 +374,25 @@ describe('getItemProps', () => { defaultIds.getItemId(2), ) }) + + 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 + renderSelect({ + defaultIsOpen: true, + defaultHighlightedIndex, + isItemDisabled(_item, idx) { + return idx === defaultHighlightedIndex + }, + }) + const toggleButton = getToggleButton() + + await clickOnItemAtIndex(index) + + expect(toggleButton).toHaveTextContent(items[index]) + expect(getItems()).toHaveLength(items.length) + expect(toggleButton).toHaveAttribute('aria-activedescendant', '') + }) }) }) diff --git a/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js b/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js index 32ff4bff..ee240d15 100644 --- a/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js +++ b/src/hooks/useSelect/__tests__/getToggleButtonProps.test.js @@ -388,16 +388,24 @@ describe('getToggleButtonProps', () => { test('initialHighlightedIndex is ignored if item is disabled', async () => { const initialHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === initialHighlightedIndex, + ) renderSelect({ initialHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === initialHighlightedIndex - }, + isItemDisabled, }) await clickOnToggleButton() expect(getToggleButton()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[initialHighlightedIndex], + initialHighlightedIndex, + ) }) test('initialHighlightedIndex is ignored and defaultHighlightedIndex is chosen if enabled', async () => { @@ -421,16 +429,24 @@ describe('getToggleButtonProps', () => { test('defaultHighlightedIndex is ignored if item is disabled', async () => { const defaultHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === defaultHighlightedIndex, + ) renderSelect({ defaultHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === defaultHighlightedIndex - }, + isItemDisabled, }) await clickOnToggleButton() expect(getToggleButton()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[defaultHighlightedIndex], + defaultHighlightedIndex, + ) }) test('both defaultHighlightedIndex and initialHighlightedIndex are ignored if items are disabled', async () => { @@ -848,11 +864,14 @@ describe('getToggleButtonProps', () => { test('initialHighlightedIndex is ignored if item is disabled', async () => { const initialHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === initialHighlightedIndex, + ) renderSelect({ initialHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === initialHighlightedIndex - }, + isItemDisabled, }) await keyDownOnToggleButton('{ArrowUp}') @@ -861,6 +880,7 @@ describe('getToggleButtonProps', () => { '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 () => { @@ -884,11 +904,14 @@ describe('getToggleButtonProps', () => { test('defaultHighlightedIndex is ignored if item is disabled', async () => { const defaultHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === defaultHighlightedIndex, + ) renderSelect({ defaultHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === defaultHighlightedIndex - }, + isItemDisabled, }) await keyDownOnToggleButton('{ArrowUp}') @@ -897,6 +920,7 @@ describe('getToggleButtonProps', () => { '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 () => { @@ -1180,11 +1204,14 @@ describe('getToggleButtonProps', () => { test('initialHighlightedIndex is ignored if item is disabled', async () => { const initialHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === initialHighlightedIndex, + ) renderSelect({ initialHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === initialHighlightedIndex - }, + isItemDisabled, }) await keyDownOnToggleButton('{ArrowDown}') @@ -1193,6 +1220,7 @@ describe('getToggleButtonProps', () => { 'aria-activedescendant', defaultIds.getItemId(0), ) + expect(isItemDisabled.mock.calls.slice(0, 2)).toMatchSnapshot() }) test('initialHighlightedIndex is ignored and defaultHighlightedIndex is chosen if enabled', async () => { @@ -1216,11 +1244,14 @@ describe('getToggleButtonProps', () => { test('defaultHighlightedIndex is ignored if item is disabled', async () => { const defaultHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === defaultHighlightedIndex, + ) renderSelect({ defaultHighlightedIndex, - isItemDisabled(item) { - return items.indexOf(item) === defaultHighlightedIndex - }, + isItemDisabled, }) await keyDownOnToggleButton('{ArrowDown}') @@ -1229,6 +1260,7 @@ describe('getToggleButtonProps', () => { '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 () => { diff --git a/src/hooks/useSelect/__tests__/props.test.js b/src/hooks/useSelect/__tests__/props.test.js index a6626a5d..439791f8 100644 --- a/src/hooks/useSelect/__tests__/props.test.js +++ b/src/hooks/useSelect/__tests__/props.test.js @@ -863,4 +863,46 @@ describe('props', () => { `downshift: A component has changed the controlled prop "highlightedIndex" to be uncontrolled. This prop should not switch from controlled to uncontrolled (or vice versa). Decide between using a controlled or uncontrolled Downshift element for the lifetime of the component. More info: https://github.com/downshift-js/downshift#control-props`, ) }) + + test('initialHighlightedIndex is ignored if item is disabled and menu is intially open', () => { + const initialHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === initialHighlightedIndex, + ) + renderSelect({ + initialHighlightedIndex, + isItemDisabled, + initialIsOpen: true, + }) + + expect(getToggleButton()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[initialHighlightedIndex], + initialHighlightedIndex, + ) + }) + + test('defaultHighlightedIndex is ignored if item is disabled and menu is intially open', () => { + const defaultHighlightedIndex = 2 + const isItemDisabled = jest + .fn() + .mockImplementation( + item => items.indexOf(item) === defaultHighlightedIndex, + ) + renderSelect({ + defaultHighlightedIndex, + isItemDisabled, + initialIsOpen: true, + }) + + expect(getToggleButton()).toHaveAttribute('aria-activedescendant', '') + expect(isItemDisabled).toHaveBeenNthCalledWith( + 1, + items[defaultHighlightedIndex], + defaultHighlightedIndex, + ) + }) }) diff --git a/src/hooks/useSelect/__tests__/returnProps.test.js b/src/hooks/useSelect/__tests__/returnProps.test.js index 4d49266f..ae467c76 100644 --- a/src/hooks/useSelect/__tests__/returnProps.test.js +++ b/src/hooks/useSelect/__tests__/returnProps.test.js @@ -92,6 +92,19 @@ describe('returnProps', () => { expect(result.current.highlightedIndex).toBe(2) }) + test('setHighlightedIndex does not set highlightedIndex if item is disabled', () => { + const highlightedIndex = 2 + const {result} = renderUseSelect({initialIsOpen: true, isItemDisabled(_item, index) { + return index === highlightedIndex + }}) + + act(() => { + result.current.setHighlightedIndex(highlightedIndex) + }) + + expect(result.current.highlightedIndex).toBe(-1) + }) + test('selectItem sets selectedItem', () => { const {result} = renderUseSelect() @@ -139,6 +152,29 @@ describe('returnProps', () => { ) expect(result.current.isOpen).toBe(props.defaultIsOpen) }) + + test('reset does not set the defaultHighlightedIndex if item is disabled', () => { + const props = { + defaultIsOpen: false, + defaultHighlightedIndex: 3, + defaultSelectedItem: items[2], + isItemDisabled(_item, index) { + return index === 3 + }, + } + const {result} = renderUseSelect(props) + + act(() => { + result.current.openMenu() + result.current.selectItem(items[4]) + result.current.setHighlightedIndex(1) + result.current.reset() + }) + + expect(result.current.selectedItem).toBe(props.defaultSelectedItem) + expect(result.current.highlightedIndex).toBe(-1) + expect(result.current.isOpen).toBe(props.defaultIsOpen) + }) }) describe('state and props', () => { diff --git a/src/hooks/useSelect/reducer.js b/src/hooks/useSelect/reducer.js index c45e43aa..380818bd 100644 --- a/src/hooks/useSelect/reducer.js +++ b/src/hooks/useSelect/reducer.js @@ -3,6 +3,7 @@ import { getHighlightedIndexOnOpen, getDefaultValue, getChangesOnSelection, + getDefaultHighlightedIndex, } from '../utils' import commonReducer from '../reducer' import {getItemIndexByCharacterKey} from './utils' @@ -17,7 +18,7 @@ export default function downshiftSelectReducer(state, action) { case stateChangeTypes.ItemClick: changes = { isOpen: getDefaultValue(props, 'isOpen'), - highlightedIndex: getDefaultValue(props, 'highlightedIndex'), + highlightedIndex: getDefaultHighlightedIndex(props), selectedItem: props.items[action.index], } diff --git a/src/hooks/utils.js b/src/hooks/utils.js index 08c9e50e..af6ba5b0 100644 --- a/src/hooks/utils.js +++ b/src/hooks/utils.js @@ -293,7 +293,7 @@ function getInitialValue( function getInitialState(props) { const selectedItem = getInitialValue(props, 'selectedItem') const isOpen = getInitialValue(props, 'isOpen') - const highlightedIndex = getInitialValue(props, 'highlightedIndex') + const highlightedIndex = getInitialHighlightedIndex(props) const inputValue = getInitialValue(props, 'inputValue') return { @@ -327,14 +327,14 @@ function getHighlightedIndexOnOpen(props, state, offset) { if ( initialHighlightedIndex !== undefined && highlightedIndex === initialHighlightedIndex && - !isItemDisabled(items[initialHighlightedIndex]) + !isItemDisabled(items[initialHighlightedIndex], initialHighlightedIndex) ) { return initialHighlightedIndex } if ( defaultHighlightedIndex !== undefined && - !isItemDisabled(items[defaultHighlightedIndex]) + !isItemDisabled(items[defaultHighlightedIndex], defaultHighlightedIndex) ) { return defaultHighlightedIndex } @@ -343,14 +343,17 @@ function getHighlightedIndexOnOpen(props, state, offset) { return items.findIndex(item => itemToKey(selectedItem) === itemToKey(item)) } - if (offset < 0 && !isItemDisabled(items[items.length - 1])) { + if ( + offset < 0 && + !isItemDisabled(items[items.length - 1], items.length - 1) + ) { return items.length - 1 } - if (offset > 0 && !isItemDisabled(items[0])) { + if (offset > 0 && !isItemDisabled(items[0], 0)) { return 0 } - + return -1 } /** @@ -643,6 +646,43 @@ function useIsInitialMount() { return isInitialMountRef.current } +/** + * Returns the new highlightedIndex based on the defaultHighlightedIndex prop, if it's not disabled. + * + * @param {Object} props Props from useCombobox or useSelect. + * @returns {number} The highlighted index. + */ +function getDefaultHighlightedIndex(props) { + const highlightedIndex = getDefaultValue(props, 'highlightedIndex') + if ( + highlightedIndex > -1 && + props.isItemDisabled(props.items[highlightedIndex], highlightedIndex) + ) { + return -1 + } + + return highlightedIndex +} + +/** + * Returns the new highlightedIndex based on the initialHighlightedIndex prop, if not disabled. + * + * @param {Object} props Props from useCombobox or useSelect. + * @returns {number} The highlighted index. + */ +function getInitialHighlightedIndex(props) { + const highlightedIndex = getInitialValue(props, 'highlightedIndex') + + if ( + highlightedIndex > -1 && + props.isItemDisabled(props.items[highlightedIndex], highlightedIndex) + ) { + return -1 + } + + return highlightedIndex +} + // Shared between all exports. const commonPropTypes = { environment: PropTypes.shape({ @@ -710,4 +750,5 @@ export { commonPropTypes, useIsInitialMount, useA11yMessageStatus, + getDefaultHighlightedIndex, }