-
Notifications
You must be signed in to change notification settings - Fork 115
fix!: search bar not returning results #1087
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!: search bar not returning results #1087
Conversation
| @SearchBar:toggle="isSearchOpen = $event" | ||
| @SearchBar:result="result = $event" | ||
| :is-search-open="isSearchOpen" | ||
| @set-is-open="isSearchOpen = $event" |
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 wasn't a toggle per se, it always set the emitted value
to be a toggle, it'd have to be "isSearchOpen = !isSearchOpen"
| }, | ||
| directives: { clickOutside }, | ||
| props: { | ||
| isSearchOpen: { |
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.
remove isSearchOpen ref from SearchBar itself as it was duplicated.
There's was isSearchOpen in AppHeader.vue that's necessary for both SearchBar and SearchResults, so there's no point keeping a duplicate of the same ref inside SearchBar - they could desync somehow etc.
So 2 way binding is used here now
| }, | ||
| }, | ||
| setup({ itemsPerPage, minTermLen }, { emit }) { | ||
| setup(props, { emit }) { |
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.
don't destructure props because of reactivity loss
| const closeSearch = (event: MouseEvent) => { | ||
| if (document) { | ||
| const searchResultsEl = document.querySelectorAll('.search'); | ||
| const searchResultsEl = document.querySelector('.search'); |
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.
No point searching for all instances of .search if you only check the first element in the array below
This will be removed later since communicating thorugh document. isn't the best
6ffff3c to
5e22ace
Compare
useFacet is meant to be used with url sorting params in the product category page, not with simple product searches. useFacet implicitly has some category search logic while the search bar doesn't care about products
| const productList : Products = await getProductList({ | ||
| pageSize: props.itemsPerPage, | ||
| search: term.value, | ||
| }) as unknown as Products; |
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.
Have to override type because getProductList returns some "ProductListQUERY" type instead of something that contains ProductInterface
5e22ace to
367efb9
Compare
the wishlist store initially downloads only product counts. the actual products are loaded later on demand, because that's a slow request. the problem is if you search the wishlist while the actual wishlist products aren't loaded yet, every search result will appear as if it's not in the wishlist (heart icon instead of heart_fill)
367efb9 to
b863587
Compare
useFacet is meant to be used with url sorting params in the product category
page, not with simple product searches. useFacet implicitly has some category
search logic while the search bar doesn't care about categories
follow-up to #1075