-
Notifications
You must be signed in to change notification settings - Fork 942
Description
downshift
version: 9.0.6node
version: 18.20.3npm
(oryarn
) version: 10.2.3
Reproduction repository:
Example: This is basically just the multiple selection with useSelect example from the docs (sans styling - sorry!)
https://stackblitz.com/edit/vitejs-vite-wimxfe?file=src%2FSelect.jsx
Problem description:
When deleting selected item tags using Backspace/Delete, if the selected item you're deleting is not the last item in the selectedItems
array the focus does not move to the next selected item after as expected. This happens because focus is only moved to a selectedItem
when the activeIndex
changes here
downshift/src/hooks/useMultipleSelection/index.js
Lines 62 to 74 in ee2a828
// Sets focus on active item. | |
useEffect(() => { | |
if (isInitialMount) { | |
return | |
} | |
if (activeIndex === -1 && dropdownRef.current) { | |
dropdownRef.current.focus() | |
} else if (selectedItemRefs.current[activeIndex]) { | |
selectedItemRefs.current[activeIndex].focus() | |
} | |
// eslint-disable-next-line react-hooks/exhaustive-deps | |
}, [activeIndex]) |
But when you delete an item that isn't the last selected item the
activeIndex
stays the same:downshift/src/hooks/useMultipleSelection/reducer.js
Lines 30 to 52 in ee2a828
case stateChangeTypes.SelectedItemKeyDownBackspace: | |
case stateChangeTypes.SelectedItemKeyDownDelete: { | |
if (activeIndex < 0) { | |
break | |
} | |
let newActiveIndex = activeIndex | |
if (selectedItems.length === 1) { | |
newActiveIndex = -1 | |
} else if (activeIndex === selectedItems.length - 1) { | |
newActiveIndex = selectedItems.length - 2 | |
} | |
changes = { | |
selectedItems: [ | |
...selectedItems.slice(0, activeIndex), | |
...selectedItems.slice(activeIndex + 1), | |
], | |
...{activeIndex: newActiveIndex}, | |
} | |
break |
The reason it works in the example in the docs is because that example derives the key for the selected item tags using the index of the item in the selectedItems
array so the element that the Delete/Backspace event was dispatched on isn't removed from the DOM (but just re-renders with the next selectedItem
's data) and so it continues to keep focus. If you use say the item ID for the React component key though, then the focus does not move correctly to the next element because the aforementioned effect never fires.
Suggested solution:
I think if we set some sort of internal boolean state to indicate that we need to set focus on the next render when a Delete/Backspace event happens where the activeIndex
is gonna stay the same, that should resolve this. Though obviously that will not work if the consumer has not updated the selectedItems
list by that next render - I don't know if that's actually something this library is (or even should be) concerned with. I'd be willing to submit a PR for this if I can get some folks' opinions on the suggested fix.