From a583281aefaaf33cb1a6c393967a060c61911e43 Mon Sep 17 00:00:00 2001 From: Silviu Alexandru Avram Date: Sun, 3 Mar 2024 12:09:54 +0200 Subject: [PATCH] feat(hooks): itemToKey prop (#1573) * change for useCombobox reducer * useCombo docs * finish useCombo tests * types * docs * finish changes * correct docs --- src/hooks/useCombobox/README.md | 48 +++++ src/hooks/useCombobox/__tests__/props.test.js | 174 ++++++++++++++++++ src/hooks/useCombobox/utils.js | 34 +++- src/hooks/useMultipleSelection/README.md | 30 +++ src/hooks/useMultipleSelection/index.js | 5 +- src/hooks/useMultipleSelection/reducer.js | 4 +- src/hooks/useMultipleSelection/utils.js | 1 + src/hooks/useSelect/README.md | 30 +++ src/hooks/useSelect/reducer.js | 5 +- src/hooks/utils.js | 25 ++- typings/index.d.ts | 3 + 11 files changed, 336 insertions(+), 23 deletions(-) diff --git a/src/hooks/useCombobox/README.md b/src/hooks/useCombobox/README.md index 246a7138..d60a8697 100644 --- a/src/hooks/useCombobox/README.md +++ b/src/hooks/useCombobox/README.md @@ -81,6 +81,7 @@ and update if necessary. - [defaultIsOpen](#defaultisopen) - [defaultHighlightedIndex](#defaulthighlightedindex) - [defaultInputValue](#defaultinputvalue) + - [itemToKey](#itemtokey) - [selectedItemChanged](#selecteditemchanged) - [getA11yStatusMessage](#geta11ystatusmessage) - [getA11ySelectionMessage](#geta11yselectionmessage) @@ -393,8 +394,55 @@ reset or when an item is selected. Pass a string that sets the content of the input when downshift is reset or when an item is selected. +### itemToKey + +> `function(item: any)` | defaults to: `item => item` + +Used to determine the uniqueness of an item when searching for the item or +comparing the item with another. Returns the item itself, by default, so the +comparing/searching is done internally via referential equality. + +If using items as objects and their reference will change during use, you can +use the function to generate a unique key for each item, such as an `id` prop. + +```js +function itemToKey(item) { + return item.id +} +``` + +> This deprecates the "selectedItemChanged" prop. If you are using the prop +> already, make sure you change to "itemToKey" as the former will be removed in +> the next Breaking Change update. A migration example: + +```js +// initial items. +const items = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] +// the same items but with different references, for any reason. +const newItems = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] + +// previously, if you probably had something like this. +function selectedItemChanged(item1, item2) { + return item1.id === item2.id +} + +// moving forward, switch to this one. +function itemToKey(item) { + return item.id + // and we will do the comparison like: const isChanged = itemToKey(prevSelectedItem) !== itemToKey(nextSelectedItem) +} +``` + ### selectedItemChanged +> DEPRECATED. Please use "itemToKey". + > `function(prevItem: any, item: any)` | defaults to: > `(prevItem, item) => (prevItem !== item)` diff --git a/src/hooks/useCombobox/__tests__/props.test.js b/src/hooks/useCombobox/__tests__/props.test.js index 03cd3cfb..895e942a 100644 --- a/src/hooks/useCombobox/__tests__/props.test.js +++ b/src/hooks/useCombobox/__tests__/props.test.js @@ -165,6 +165,9 @@ describe('props', () => { }) test('props update of selectedItem will not update inputValue state if selectedItemChanged returns false', () => { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}) const initialSelectedItem = {id: 1, value: 'hmm'} const selectedItem = {id: 1, value: 'wow'} function itemToString(item) { @@ -197,6 +200,116 @@ describe('props', () => { initialSelectedItem, selectedItem, ) + expect(consoleWarnSpy).toHaveBeenCalledTimes(1) + expect(consoleWarnSpy).toHaveBeenCalledWith( + `The "selectedItemChanged" is deprecated. Please use "itemToKey instead". https://github.com/downshift-js/downshift/blob/master/src/hooks/useCombobox/README.md#selecteditemchanged`, + ) + consoleWarnSpy.mockRestore() + }) + }) + + describe('itemToKey', () => { + test('props update of selectedItem will update inputValue state with default itemToKey referential equality check', () => { + const initialSelectedItem = {id: 3, value: 'init'} + const selectedItem = {id: 1, value: 'wow'} + const newSelectedItem = {id: 1, value: 'not wow'} + function itemToString(item) { + return item.value + } + const stateReducer = jest + .fn() + .mockImplementation((_state, {changes}) => changes) + + const {rerender} = renderCombobox({ + stateReducer, + itemToString, + selectedItem: initialSelectedItem, + }) + + expect(stateReducer).not.toHaveBeenCalled() // won't get called on first render + + rerender({ + stateReducer, + itemToString, + selectedItem, + }) + + expect(stateReducer).toHaveBeenCalledTimes(1) + expect(stateReducer).toHaveBeenCalledWith( + { + inputValue: itemToString(initialSelectedItem), + selectedItem, + highlightedIndex: -1, + isOpen: false, + }, + expect.objectContaining({ + type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem, + changes: { + inputValue: itemToString(selectedItem), + selectedItem, + highlightedIndex: -1, + isOpen: false, + }, + }), + ) + + stateReducer.mockClear() + rerender({ + stateReducer, + selectedItem: newSelectedItem, + itemToString, + }) + + expect(stateReducer).toHaveBeenCalledTimes(1) + expect(stateReducer).toHaveBeenCalledWith( + { + inputValue: itemToString(selectedItem), + selectedItem: newSelectedItem, + highlightedIndex: -1, + isOpen: false, + }, + expect.objectContaining({ + changes: { + inputValue: itemToString(newSelectedItem), + selectedItem: newSelectedItem, + highlightedIndex: -1, + isOpen: false, + }, + type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem, + }), + ) + expect(getInput()).toHaveValue(itemToString(newSelectedItem)) + }) + + test('props update of selectedItem will not update inputValue state if itemToKey returns equal values', () => { + const initialSelectedItem = {id: 1, value: 'hmm'} + const selectedItem = {id: 1, value: 'wow'} + function itemToString(item) { + return item.value + } + const itemToKey = jest.fn().mockImplementation(item => item.id) + const stateReducer = jest + .fn() + .mockImplementation((_state, {changes}) => changes) + + const {rerender} = renderCombobox({ + itemToKey, + stateReducer, + selectedItem: initialSelectedItem, + itemToString, + }) + + rerender({ + itemToKey, + stateReducer, + selectedItem, + itemToString, + }) + + expect(getInput()).toHaveValue(itemToString(initialSelectedItem)) + expect(itemToKey).toHaveBeenCalledTimes(2) + expect(itemToKey).toHaveBeenNthCalledWith(1, selectedItem) + expect(itemToKey).toHaveBeenNthCalledWith(2, initialSelectedItem) }) }) @@ -601,6 +714,67 @@ describe('props', () => { expect(input).toHaveValue(selectedItem) }) + test('selectedItem change updates the input value', async () => { + const selectedItem = items[2] + const newSelectedItem = items[4] + const nullSelectedItem = null + const lastSelectedItem = items[1] + const stateReducer = jest.fn().mockImplementation((s, a) => a.changes) + + const {rerender} = renderCombobox({ + selectedItem, + stateReducer, + }) + const input = getInput() + + expect(input).toHaveValue(selectedItem) + expect(stateReducer).not.toHaveBeenCalled() // don't call on first render. + + rerender({ + selectedItem: newSelectedItem, + stateReducer, + }) + + expect(stateReducer).toHaveBeenCalledTimes(1) + expect(stateReducer).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem, + }), + ) + expect(input).toHaveValue(newSelectedItem) + + stateReducer.mockClear() + rerender({ + selectedItem: nullSelectedItem, + stateReducer, + }) + + expect(stateReducer).toHaveBeenCalledTimes(1) + expect(stateReducer).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem, + }), + ) + expect(input).toHaveValue('') + + stateReducer.mockClear() + rerender({ + selectedItem: lastSelectedItem, + stateReducer, + }) + + expect(stateReducer).toHaveBeenCalledTimes(1) + expect(stateReducer).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem, + }), + ) + expect(input).toHaveValue(lastSelectedItem) + }) + describe('stateReducer', () => { beforeEach(() => jest.useFakeTimers()) afterEach(() => { diff --git a/src/hooks/useCombobox/utils.js b/src/hooks/useCombobox/utils.js index b8aa9ba6..49c7ba56 100644 --- a/src/hooks/useCombobox/utils.js +++ b/src/hooks/useCombobox/utils.js @@ -83,16 +83,31 @@ export function useControlledReducer( } if ( - !isInitialMount && // on first mount we already have the proper inputValue for a initial selected item. - props.selectedItemChanged( - previousSelectedItemRef.current, - props.selectedItem, - ) + !isInitialMount // on first mount we already have the proper inputValue for a initial selected item. ) { - dispatch({ - type: ControlledPropUpdatedSelectedItem, - inputValue: props.itemToString(props.selectedItem), - }) + let shouldCallDispatch + + if (props.selectedItemChanged === undefined) { + shouldCallDispatch = + props.itemToKey(props.selectedItem) !== + props.itemToKey(previousSelectedItemRef.current) + } else { + console.warn( + `The "selectedItemChanged" is deprecated. Please use "itemToKey instead". https://github.com/downshift-js/downshift/blob/master/src/hooks/useCombobox/README.md#selecteditemchanged`, + ) + + shouldCallDispatch = props.selectedItemChanged( + previousSelectedItemRef.current, + props.selectedItem, + ) + } + + if (shouldCallDispatch) { + dispatch({ + type: ControlledPropUpdatedSelectedItem, + inputValue: props.itemToString(props.selectedItem), + }) + } } previousSelectedItemRef.current = @@ -116,7 +131,6 @@ if (process.env.NODE_ENV !== 'production') { export const defaultProps = { ...defaultPropsCommon, - selectedItemChanged: (prevItem, item) => prevItem !== item, getA11yStatusMessage, isItemDisabled() { return false diff --git a/src/hooks/useMultipleSelection/README.md b/src/hooks/useMultipleSelection/README.md index 784207b5..9d16dc9e 100644 --- a/src/hooks/useMultipleSelection/README.md +++ b/src/hooks/useMultipleSelection/README.md @@ -44,6 +44,7 @@ such as when an item has been removed from selection. - [initialActiveIndex](#initialactiveindex) - [defaultSelectedItems](#defaultselecteditems) - [defaultActiveIndex](#defaultactiveindex) + - [itemToKey](#itemtokey) - [getA11yRemovalMessage](#geta11yremovalmessage) - [onActiveIndexChange](#onactiveindexchange) - [onStateChange](#onstatechange) @@ -423,6 +424,35 @@ Pass an array of items that are going to be used when downshift is reset. Pass a number that sets the index of the focused / active selected item when downshift is reset. +### itemToKey + +> `function(item: any)` | defaults to: `item => item` + +Used to determine the uniqueness of an item when searching for the item or +comparing the item with another. Returns the item itself, by default, so the +comparing/searching is done internally via referential equality. + +If using items as objects and their reference will change during use, you can +use the function to generate a unique key for each item, such as an `id` prop. + +```js +// initial items. +const selectedItems = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] +// the same items but with different references, for any reason. +const newSelectedItems = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] + +function itemToKey(item) { + return item.id + // and we will do the comparison like: const isChanged = itemToKey(prevSelectedItem) !== itemToKey(nextSelectedItem) +} +``` + ### getA11yRemovalMessage > `function({/* see below */})` | default messages provided in English diff --git a/src/hooks/useMultipleSelection/index.js b/src/hooks/useMultipleSelection/index.js index 175394cb..7988f8f5 100644 --- a/src/hooks/useMultipleSelection/index.js +++ b/src/hooks/useMultipleSelection/index.js @@ -63,7 +63,10 @@ function useMultipleSelection(userProps = {}) { if (selectedItems.length < previousSelectedItemsRef.current.length) { const removedSelectedItem = previousSelectedItemsRef.current.find( - item => selectedItems.indexOf(item) < 0, + selectedItem => + selectedItems.findIndex( + item => props.itemToKey(item) === props.itemToKey(selectedItem), + ) < 0, ) setStatus( diff --git a/src/hooks/useMultipleSelection/reducer.js b/src/hooks/useMultipleSelection/reducer.js index 5ae17efd..cc8ba211 100644 --- a/src/hooks/useMultipleSelection/reducer.js +++ b/src/hooks/useMultipleSelection/reducer.js @@ -74,7 +74,9 @@ export default function downshiftMultipleSelectionReducer(state, action) { break case stateChangeTypes.FunctionRemoveSelectedItem: { let newActiveIndex = activeIndex - const selectedItemIndex = selectedItems.indexOf(selectedItem) + const selectedItemIndex = selectedItems.findIndex( + item => props.itemToKey(item) === props.itemToKey(selectedItem), + ) if (selectedItemIndex < 0) { break diff --git a/src/hooks/useMultipleSelection/utils.js b/src/hooks/useMultipleSelection/utils.js index 13affd02..72b3d7ae 100644 --- a/src/hooks/useMultipleSelection/utils.js +++ b/src/hooks/useMultipleSelection/utils.js @@ -127,6 +127,7 @@ const propTypes = { export const defaultProps = { itemToString: defaultPropsCommon.itemToString, + itemToKey: defaultPropsCommon.itemToKey, stateReducer: defaultPropsCommon.stateReducer, environment: defaultPropsCommon.environment, getA11yRemovalMessage, diff --git a/src/hooks/useSelect/README.md b/src/hooks/useSelect/README.md index 39132a4b..eaae3c5b 100644 --- a/src/hooks/useSelect/README.md +++ b/src/hooks/useSelect/README.md @@ -49,6 +49,7 @@ guide][migration-guide-v7] and update if necessary. - [defaultSelectedItem](#defaultselecteditem) - [defaultIsOpen](#defaultisopen) - [defaultHighlightedIndex](#defaulthighlightedindex) + - [itemToKey](#itemtokey) - [getA11yStatusMessage](#geta11ystatusmessage) - [getA11ySelectionMessage](#geta11yselectionmessage) - [onHighlightedIndexChange](#onhighlightedindexchange) @@ -321,6 +322,35 @@ when an item is selected. Pass a number that sets the index of the highlighted item when downshift is reset or when an item is selected. +### itemToKey + +> `function(item: any)` | defaults to: `item => item` + +Used to determine the uniqueness of an item when searching for the item or +comparing the item with another. Returns the item itself, by default, so the +comparing/searching is done internally via referential equality. + +If using items as objects and their reference will change during use, you can +use the function to generate a unique key for each item, such as an `id` prop. + +```js +// initial items. +const items = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] +// the same items but with different references, for any reason. +const newItems = [ + {id: 1, value: 'Apples'}, + {id: 2, value: 'Oranges'}, +] + +function itemToKey(item) { + return item.id + // and we will do the comparison like: const isChanged = itemToKey(prevSelectedItem) !== itemToKey(nextSelectedItem) +} +``` + ### getA11yStatusMessage > `function({/* see below */})` | default messages provided in English diff --git a/src/hooks/useSelect/reducer.js b/src/hooks/useSelect/reducer.js index a49afde6..c45e43aa 100644 --- a/src/hooks/useSelect/reducer.js +++ b/src/hooks/useSelect/reducer.js @@ -28,7 +28,10 @@ export default function downshiftSelectReducer(state, action) { const inputValue = `${state.inputValue}${lowercasedKey}` const prevHighlightedIndex = !state.isOpen && state.selectedItem - ? props.items.indexOf(state.selectedItem) + ? props.items.findIndex( + item => + props.itemToKey(item) === props.itemToKey(state.selectedItem), + ) : state.highlightedIndex const highlightedIndex = getItemIndexByCharacterKey({ keysSoFar: inputValue, diff --git a/src/hooks/utils.js b/src/hooks/utils.js index d620d5f2..5e4fd198 100644 --- a/src/hooks/utils.js +++ b/src/hooks/utils.js @@ -72,10 +72,10 @@ function stateReducer(s, a) { * @returns {string} The a11y message. */ function getA11ySelectionMessage(selectionParameters) { - const {selectedItem, itemToString: itemToStringLocal} = selectionParameters + const {selectedItem, itemToString} = selectionParameters return selectedItem - ? `${itemToStringLocal(selectedItem)} has been selected.` + ? `${itemToString(selectedItem)} has been selected.` : '' } @@ -158,10 +158,6 @@ function getItemAndIndex(itemProp, indexProp, items, errorMessage) { return [item, index] } -function itemToString(item) { - return item ? String(item) : '' -} - function isAcceptedCharacterKey(key) { return /^\S{1}$/.test(key) } @@ -261,7 +257,12 @@ function useControlledReducer( } const defaultProps = { - itemToString, + itemToString(item) { + return item ? String(item) : '' + }, + itemToKey(item) { + return item + }, stateReducer, getA11ySelectionMessage, scrollIntoView, @@ -313,7 +314,9 @@ function getInitialState(props) { return { highlightedIndex: highlightedIndex < 0 && selectedItem && isOpen - ? props.items.indexOf(selectedItem) + ? props.items.findIndex( + item => props.itemToKey(item) === props.itemToKey(selectedItem), + ) : highlightedIndex, isOpen, selectedItem, @@ -322,7 +325,8 @@ function getInitialState(props) { } function getHighlightedIndexOnOpen(props, state, offset) { - const {items, initialHighlightedIndex, defaultHighlightedIndex} = props + const {items, initialHighlightedIndex, defaultHighlightedIndex, itemToKey} = + props const {selectedItem, highlightedIndex} = state if (items.length === 0) { @@ -340,7 +344,7 @@ function getHighlightedIndexOnOpen(props, state, offset) { return defaultHighlightedIndex } if (selectedItem) { - return items.indexOf(selectedItem) + return items.findIndex(item => itemToKey(selectedItem) === itemToKey(item)) } if (offset === 0) { return -1 @@ -649,6 +653,7 @@ const commonPropTypes = { Node: PropTypes.func.isRequired, }), itemToString: PropTypes.func, + itemToKey: PropTypes.func, stateReducer: PropTypes.func, } diff --git a/typings/index.d.ts b/typings/index.d.ts index 9836617e..4fe90542 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -341,6 +341,7 @@ export interface UseSelectProps { items: Item[] isItemDisabled?(item: Item, index: number): boolean itemToString?: (item: Item | null) => string + itemToKey?: (item: Item | null) => any getA11yStatusMessage?: (options: A11yStatusMessageOptions) => string getA11ySelectionMessage?: (options: A11yStatusMessageOptions) => string highlightedIndex?: number @@ -537,6 +538,7 @@ export interface UseComboboxProps { items: Item[] isItemDisabled?(item: Item, index: number): boolean itemToString?: (item: Item | null) => string + itemToKey?: (item: Item | null) => any selectedItemChanged?: (prevItem: Item, item: Item) => boolean getA11yStatusMessage?: (options: A11yStatusMessageOptions) => string getA11ySelectionMessage?: (options: A11yStatusMessageOptions) => string @@ -735,6 +737,7 @@ export interface UseMultipleSelectionProps { initialSelectedItems?: Item[] defaultSelectedItems?: Item[] itemToString?: (item: Item) => string + itemToKey?: (item: Item | null) => any getA11yRemovalMessage?: (options: A11yRemovalMessage) => string stateReducer?: ( state: UseMultipleSelectionState,