Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiSelectable] searchProps.onChange returns stale state #4262

Closed
smhutch opened this issue Nov 12, 2020 · 4 comments
Closed

[EuiSelectable] searchProps.onChange returns stale state #4262

smhutch opened this issue Nov 12, 2020 · 4 comments
Labels

Comments

@smhutch
Copy link
Contributor

smhutch commented Nov 12, 2020

This issue only applies to the latest version (30.3). It is mostly a follow-up to #4132, which has been resolved 🎉 , but introduced a small issue as part of the main fix.

Issue

The new issue I have observed is that the options arg passed to onChange and searchProps.onChange differ. While onChange receives the latest state, searchProps.onChange receives the old state (meaning the checked value for the item that was clicked on is out of date).

I created a sandbox to show the issue. The issue in this sandbox can be seen by checking the console, and comparing the values shown for the two callbacks.

Secondary request

A secondary request, is that it would be nice to pass the visible options as an array (instead of an object), so that it matches the "top level" onChange. That's not a blocking issue for me though, and appreciate it might be a hassle to change.

@smhutch
Copy link
Contributor Author

smhutch commented Nov 12, 2020

I would be happy to help with a contribution here, if someone could confirm my understanding of the issue. Cheers!

@thompsongl
Copy link
Contributor

I would be happy to help with a contribution here

We'd be happy to have you contribute! I think you are correct on both points: searchProps.onChange should use the updated state value, and the first parameter should be an array (a little confused why TypeScript didn't catch this).

@smhutch
Copy link
Contributor Author

smhutch commented Nov 20, 2020

Opened a PR to fix the issue: #4292, and another to try to avoid prop conflicts moving forward within this component: #4291. LMK what you think. Thanks!

@thompsongl
Copy link
Contributor

Fixed in #4292

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

No branches or pull requests

2 participants