downshift-js / downshift

🏎 A set of primitives to build simple, flexible, WAI-ARIA compliant React autocomplete, combobox or select dropdown components.
http://downshift-js.com/
MIT License
12.09k stars 929 forks source link

previousSelectedItemsRef.current is not updated #1535

Open boris-brtan opened 1 year ago

boris-brtan commented 1 year ago

https://github.com/downshift-js/downshift/blob/90d6d7208c61132c01868473c6dfac3a05e92076/src/hooks/useMultipleSelection/index.js#L81

in some special cases for multiple selection when we use setSelectedItems instead of addSelectedItem, it can happen that previousSelectedItemsRef.current is not updated because setSelectedItems sets the same amount of items but different.

Example

1, 2, 3, 4
set 2, 3
set 1, 4
but in previousSelectedItemsRef.current are still 2, 3 set 1, 2, 3 in previousSelectedItemsRef.current are 1,2,3 because length changed

boris-brtan commented 1 year ago

is this something that can be fixed or do you need Pull Request to be created?

silviuaavram commented 1 year ago

It is a corner case, but still valid. On the other hand, the alternative would be to search for potentially removed items on every render. Any better suggestions?

Furthermore, this hook logic is to announce to screen readers when an item is removed. It needs a completely different logic in order to handle the case with setSelectedItems.

I am still thinking about a better way with the whole a11y screen reader announcements, since the current behaviour is not great. I will update once we figure out something.

boris-brtan commented 10 months ago

tried something, it is not perfect however better than nothing.