Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

fix(Selection): prevent selection from reselecting when deselecting #1293

Closed

Conversation

scottdickerson
Copy link
Contributor

Prevent selection from reselecting when deselecting

Closes IBM/carbon-components-react#1291

{{short description}}

Changelog

New

  • {{new thing}}

Changed

  • Fix the selection criteria to actually check the id when determining reselection, this prevents the need to pass the exact in-memory item to the initialSelectedItems list.

Removed

  • {{removed thing}}

Prevent selection from reselecting when deselecting
@scottdickerson
Copy link
Contributor Author

If you remove the Selection change and go to the MultiSelect storybook for multiple items with initial Values selected, you can see the bug when you deselect the existing selection (see bug #1291). Then putting back in the Selection change will fix it.

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM basically, but .findIndex() is not supported by IE11 or in our polyfills list. Given updating polyfills list causes a breaking change, do you want to try an approach without .findIndex()? Thanks!

Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the strategy here, but when it comes to API for initialSelectedItems I think it'd be nice if we supported an array of ids (strings) or an array of indices (numbers) that we would check for in the handleOnItemChange. In addition, could we add a test for this additional functionality for Selection?

@scottdickerson
Copy link
Contributor Author

yes I can do that, thanks for the suggestions. @joshblack and @asudoh.

@scottdickerson
Copy link
Contributor Author

this has been superceded by this track: #1314

asudoh added a commit to asudoh/carbon-components-react that referenced this pull request Apr 18, 2019
emyarod pushed a commit that referenced this pull request Apr 25, 2019
@vpicone vpicone mentioned this pull request Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants