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

Programatically changing the state to select all values does not set the select all checkbox #92

Closed
ghost opened this issue Jan 8, 2019 · 6 comments · Fixed by #93
Closed
Labels

Comments

@ghost
Copy link

ghost commented Jan 8, 2019

Version

4.1.0

Here's what went wrong:

Seems like there is a synchronization issue where if the state is changed to select all values, the 'Select all' button (if enabled) is not checked.

See here to reproduce the bug:
https://codesandbox.io/s/9351z3moqr

  1. Click on the Select button at the top, which will select Item 1 and Item 2. Notice the Select all button is not checked but it should be.
  2. Click on the Select all button. Due to the internal states being out of sync, the select all is now effectively selecting all the values rather than unchecking all the values (effectively re-syncing the state).
  3. Click on the Select all button again. This will now correctly uncheck all the values.
@Aidurber Aidurber added the bug label Jan 8, 2019
@Aidurber
Copy link
Owner

Aidurber commented Jan 8, 2019

@wlamnz Thanks for the report. I'll take a look later.

Aidurber added a commit that referenced this issue Jan 8, 2019
With objects as options the equality check was goosed. Now use the object value so we dont have to
do a deep equality check

fix #92
Aidurber added a commit that referenced this issue Jan 8, 2019
With objects as options the equality check was goosed. Now use the object value so we dont have to
do a deep equality check

fix #92
@Aidurber
Copy link
Owner

Aidurber commented Jan 8, 2019

🎉 This issue has been resolved in version 4.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost
Copy link
Author

ghost commented Jan 8, 2019

@Aidurber Thanks for your quick effort. However, I just tested the issue on 4.1.1 and I still seem to have the same problem. The Select all checkbox is still not being set once all the values in the dropdown are set.

See below:
image

https://codesandbox.io/s/5w2yq84xql (4.1.1 version)

Clicking the Select button at the top to programmatically set the Item 1 and Item 2 value still does not set the Select all checkbox.

Note: this version is definitely better in a sense that toggling any of the Item values (i.e. Item 1 or Item 2 from the example) will set the Select all, which the previous version did not.

@Aidurber
Copy link
Owner

Aidurber commented Jan 8, 2019

😕 I had it working correctly and even wrote tests around it. Back to the drawing board!

@Aidurber Aidurber reopened this Jan 8, 2019
@Aidurber
Copy link
Owner

Aidurber commented Jan 9, 2019

Hi @wlamnz, I figured it out. The codesandbox you sent was using React and React-DOM 16.0.0.

Picky has a dependency on React > 16.3.0 since it's using the UNSAFE_* lifecycle methods. See this for reference: https://reactjs.org/blog/2018/03/29/react-v-16-3.html

If you're using 16.0.0 in your app you might want to consider bumping to 16.3.0 or higher. There are no breaking changes.

@ghost
Copy link
Author

ghost commented Jan 10, 2019

Ah, that makes a lot sense. Thanks for that!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant