Removing one value from Search filter clears the search #55636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ishakakkad
force-pushed
the
55433-search-filter-clear-fix
branch
from
550ec50 to
cc44229
Compare
ishakakkad
commented
Jan 23, 2025
| isFullWidth | ||
| onSubmit={() => { | ||
| if (!isAutocompleteListVisible) { |
There was a problem hiding this comment.
I have used this logic instead of below logic, because in case when we select the currency or category from the suggestions and we press enter submitSearch() does not work. So now if the dropdown is not visible we assume submitSearch() is already called otherwise we call submitSearch().
onSubmit={() => {
const focusedOption = listRef.current?.getFocusedOption();
if (!focusedOption) {
submitSearch(textInputValue);
}
}}
There was a problem hiding this comment.
The behaviour you are seeing is consist with the on mouse click. Thus it's not really a bug. Please revert and use the agreed on solution.
|
@s77rt please prioritize this one when you're online since it's affecting a lot of users |
s77rt
suggested changes
Jan 23, 2025
| @@ -126,8 +126,6 @@ function SearchPageHeaderInput({queryJSON, children}: SearchPageHeaderInputProps | |||
|
|
|||
| if (updatedUserQuery || textInputValue.length > 0) { | |||
| listRef.current?.updateAndScrollToFocusedIndex(0); | |||
| } else { | |||
| listRef.current?.updateAndScrollToFocusedIndex(-1); | |||
| } | |||
There was a problem hiding this comment.
Revert. This should be:
if (updatedUserQuery) {
listRef.current?.updateAndScrollToFocusedIndex(0);
} else {
listRef.current?.updateAndScrollToFocusedIndex(-1);
}There was a problem hiding this comment.
Sorry I misunderstood your suggestion, reverted. Thank you!
Comment on lines
261
to
267
| onSearchQueryChange={(userQuery) => { | ||
| onSearchQueryChange(userQuery); | ||
| if (userQuery) { | ||
| return; | ||
| } | ||
| listRef.current?.updateAndScrollToFocusedIndex(-1); | ||
| }} |
There was a problem hiding this comment.
Sorry I misunderstood your suggestion, reverted. Thank you!
| isFullWidth | ||
| onSubmit={() => { | ||
| if (!isAutocompleteListVisible) { |
There was a problem hiding this comment.
The behaviour you are seeing is consist with the on mouse click. Thus it's not really a bug. Please revert and use the agreed on solution.
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.movdesktop.2.mov |
s77rt
approved these changes
Jan 23, 2025
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.0.89-0 🚀
|
48 tasks
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.89-8 🚀
|
47 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Fixed Issues
$ #55433
PROPOSAL: #55433 (comment)
Tests
Same as QA
Offline tests
NA
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Scenario 1:
Scenario 2:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android.mp4
Android: mWeb Chrome
Android.web.mov
iOS: Native
IOS.mp4
iOS: mWeb Safari
IOS.Web.mp4
MacOS: Chrome / Safari
Web.mov
MacOS: Desktop
Desktop.mov