-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fix weird search behaviour #361
Conversation
adda1d4
to
f63239d
Compare
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.
It looks good to me. I left some small comments. Let me know.
this.hideRecentSearches = false; | ||
debounce(func, CLEAR_RESULTS_DELAY)(); |
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.
This could confuse users a little bit, since we will show the recent search when we have less than MIN_CHARACTERS_QUERY
. I think we should show recent search when the input field is completely empty, instead (e.g. query.length === 0
)
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.
Hm, I've tried this, but felt the behaviour implemented here felt a bit more natural. Have you tried both options? In any case let me know if you want me to change it, no problem at all.
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 just tested it locally as-is and I think it's fine for now. We can change this behavior in the future if we feel it doesn't work well.
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.
Looks good to me 💯
Fixes #358
Fixes #357