-
Notifications
You must be signed in to change notification settings - Fork 19
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
Search bar: add ❌ button to clear the search terms; emit actions upon user input. #214
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by a few things in this PR.
- What is the purpose of this PR? there's no description, and the title is lengthy but not quite detailed enough.
- As a follow-up to the above, this doesn't seem to actually implement the search functionality. So it'd be pretty confusing for users to see a new search bar that they can interact with but doesn't actually do anything.... unless there is another PR that is designed as a pairing to this?
- The search timer idea is unusual, to say the least. The action of searching should happen automatically upon typing (i.e., when the typed text changes), which you can determine by checking for actions on the actual
TextInput
widget.
That's my problem I didn't describe it clearly, I meant an enhancement to the search bar component. Add Action and clicked events and close icon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok nice, now I understand what the scope of this PR is. It looks good, just one small nit about unnecessary changes to the rooms_sidebar.rs
file.
41ba290
to
d16a7d0
Compare
Partially addresses #123.