thehyve / react-select-checked

A React select component based on JedWatson/React-Select with checkmarks on selected options.
GNU General Public License v3.0
9 stars 5 forks source link

Refactor data flow to minimise duplicate state #9

Closed fedde-s closed 6 years ago

fedde-s commented 6 years ago

Using on the value property directly allows the component to no longer keep an isSelected flag updated in option objects, which simplifies the code substantially. Once this has been factored out, it can also benefit from the toggling functionality built into the underlying react-select component instead of a custom onChange handler. None of this should affect the functionality in any way, but it does require an update to the current release of react-select.

fedde-s commented 6 years ago

@rnugraha, could you review whether you agree with this refactoring, and especially whether the dependency version upgrade is correct?

fedde-s commented 6 years ago

The npm command seems to want me to update package-lock.json as well now that the dependencies have changed. should I include that change in the commit?

rnugraha commented 6 years ago

Please commit package-lock.json as well

fedde-s commented 6 years ago

I updated package-lock.json, and rebased the patches to resolve the merge conflicts.

pieterlukasse commented 6 years ago

@rnugraha is this ready to be merged?

rnugraha commented 6 years ago

Will close this since this is no longer necessary

fedde-s commented 6 years ago

I think it is as necessary as it ever was, to make the component easier to improve on and potentially more accessible.

fedde-s commented 6 years ago

@rnugraha: done.