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

MultiSelect: make possible to control the query #2319

Closed
wants to merge 1 commit into from

Conversation

alxmiron
Copy link
Contributor

Make possible to control the input query by tagInputProps.inputProps.value

@alxmiron
Copy link
Contributor Author

MultiSelect: make possible to control the query

Preview: documentation | landing | table

@alxmiron
Copy link
Contributor Author

MultiSelect: make possible to control the query

Preview: documentation | landing | table

Copy link
Contributor

@giladgray giladgray left a comment

Choose a reason for hiding this comment

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

cool easy enough 👍
any other select components that deserve this same treatment? would be great to maintain similar support across all those components

@giladgray
Copy link
Contributor

giladgray commented Mar 27, 2018

@alxmiron also, please don't amend / force push commits to PRs -- it ruins the outdated diffs.

@alxmiron
Copy link
Contributor Author

  1. Select, suggest and omnibar support inputProps.value correctly.
  2. I did not force push. I rerun CI jobs, because sometimes have problems with Circle CI tracking

@alxmiron
Copy link
Contributor Author

Apparently, this is not enough. When I change inputProps.value, internal state.query does not change. It causes inconsistency - internal state.query and input content are different

Copy link
Contributor

@giladgray giladgray left a comment

Choose a reason for hiding this comment

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

@alxmiron thanks for following up on this feature! can you get it working correctly here (and perhaps add a unit test)? or feel free to close this and we can revisit in the future.

@giladgray
Copy link
Contributor

@alxmiron any update on this PR? would you be able to get it working properly soon?

@alxmiron
Copy link
Contributor Author

This appeared to be not so easy. Sorry I cannot find time to find a solution. Now resetOnSelect prop helps me with the problem I faced when created this PR

@giladgray
Copy link
Contributor

@alxmiron yeah i know it's a tricky feature. the solution should probably involve QueryList in no small way, not just MultiSelect. i'm going to close this PR until we have bandwidth to dive deeper. thanks for the attempt!

@giladgray giladgray closed this Apr 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants