fix(useMultipleSelect): removeSelectedItem adding duplicate items to selected items

[drewbrend]

fixes #1316

What:

Fixing the handling of undefined/null being provided to useMultipleSelection's removeSelectedItem function.

Why:

Currently when undefined/null is provided one of the items is duplicated in selectedItems

How:

Only modifying the selectedItems and activeIndex if selectedItemIndex is greater than or equal to zero

Checklist:

• Documentation N/A
• Tests
• TypeScript Types N/A
• Flow Types N/A
• Ready to be merged
  

[lveillard]

Hello! Not sure but i think this PR could fix this also:
#1355

Will try it once is good.
Anything we can do to speed up the merge?
Thanks!

[drewbrend]

Hello! Not sure but i think this PR could fix this also: #1355

Will try it once is good. Anything we can do to speed up the merge? Thanks!

I don't think it will, downshift relies on the items being reference equal so you probably need to fix how you're handling/re-creating your items

[lveillard]

so we have to drop all the good immutability practices? Why not being dependent on the content of the array instead of it's reference? Replacing/deleting object properties may not be the best option...

Also the example 5 is broken even just by adding some initial choices, no need to manipulate the object with .map() to cause the error of multiplying selected items.

And also it only happens with the inside of the popper, removing badges works as expected, so maybe is more an issue with the example than downshift itself? 🤔

I personally was unable to understand where it comes from

[silviuaavram]

Hi! Will take a look at this and will merge if it's good to go. Thank you!

[lveillard]

Hello @silviuaavram would be lovely to get some news about this, I think this is creating some weird bugs like this one: #1355

[drewbrend]

@lveillard this will prevent you from getting duplicated values but won't fix your issue, you need to provide an item referentially equal to one of the provided items

[silviuaavram]

Finally fixed the CI pipeline. Let's see what this PR.

[silviuaavram]

Great changes and tests, congrats!

I think we should rebase this with latest master, since it has been a while, do that small change and merge it.

Thank you again for your patience @drewbrend !

[silviuaavram]

@all-contributors please add @drewbrend for code

[allcontributors]

@silviuaavram

I've put up a pull request to add @drewbrend! 🎉

[github-actions]

🎉 This PR is included in version 6.1.8 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀