-
Notifications
You must be signed in to change notification settings - Fork 32
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: Do not change route path when clearing fuzzy search #804
Conversation
Affected libs:
|
… empty to prevents undesired route changes
3a96d50
to
8392121
Compare
to same version as package.json
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.
Thanks for all the added tests! I did a suggestion to make the code more straightforward, what do you think?
@@ -76,6 +79,8 @@ export class FuzzySearchComponent implements OnInit { | |||
} | |||
|
|||
handleInputCleared() { | |||
this.searchService.updateFilters({ any: '' }) | |||
if (this.currentSearchFilters.any) { |
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.
Instead of storing an internal state here you can do something like so:
async handleInputCleared() {
const currentSearchFilters = await firstValueFrom(this.searchFacade.searchFilters$);
if (!currentSearchFilters.any) { return; }
this.searchService.updateFilters({ any: '' })
}
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.
Nice idea, also works as expected, thanks! I just inverted the if
to save another line.
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.
Thanks @tkohr for changing the behavior for clearing the search input! It looks good to me 👍
Thanks for the reviews! Unfortunately, the e2e test I tried to improve still does not pass systematically and needed a re-run. |
Description
Currently, the router navigates to the search when the fuzzy search entry is cleared (with the input's cross) on a different tab/page than the search. This PR intends to maintain the same path when clearing the search. The
fuzzy-search.component
now only callsupdateFilters
ifsearchFilters.any
is not empty or undefined.Also updates
@geospatial-sdk/geocoding
for the npm package and attempts to fix a flaky e2e test.Quality Assurance Checklist
breaking change
labelbackport <release branch>
labelThis work is sponsored by MEL.