-
Notifications
You must be signed in to change notification settings - Fork 3
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 query bug #287
Search bar query bug #287
Conversation
|
[diff-counting] Significant lines: 2. |
Visit the preview URL for this PR (updated for commit 1ec721e): https://cu-apts-staging--pr287-jh2228-search-bar-qu-e22hnen7.web.app (expires Fri, 12 May 2023 18:49:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 096ac87b789b31770a01964fe0aaa92d563b9353 |
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.
Hey Jessica! Great job on this UI change this fixes the bug where the search bar was outlined. Looking at the code changes, the setQuery('') for both the events where the down arrow is pressed and enter is pressed is important to match the intuition of the user. All in all, great job on this PR and it is good to have it in the product!
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.
Great work Jessica! This is an important fix and bugs are always important to find and squash. Looking forward to seeing more!
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.
The fixing looks great. However, the bug of search bar not reset after switching between different pages still stay, please update this PR with all bugs related to this issue fixed before merge!
This PR looks great. However, apart from Sophia's review that the bug remains when switching pages, I also noticed that the search bar is cleared after the down arrow key is pressed, which may not be something the user wants. Also because the search query doesn't actually go through when the down key is pressed so it just clears it without searching anything. Still, good progress though! |
Fixed in #302 |
Fixed search bar query bug
This pull request is the first step towards fixing the search bar query bux
Test Plan
1) After searching "main" from the homepage
2) Search results for bar - search bar is cleared
3) Search new from the home screen - search bar is cleared
4) Switching to FAQ - search bar is cleared - before fix search query was saved in search bar
Notes
Breaking Changes