-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add autocomplete in search bar #165
Conversation
a9636c3
to
a004023
Compare
4e89c98
to
2a9dd68
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.
Thanks @Kout95
Requested changes are mostly semantic :-)
I will test it as soon as you have amended it.
frontend/src/utils/taxonomies.ts
Outdated
export const getTaxonomyName = (taxonomy: string): string => { | ||
return `${taxonomy}`.replace('s_tags', '').replace('ies_tags', 'y'); | ||
return `${taxonomy}`.replace('ies_tags', 'y').replace('s_tags', ''); | ||
}; |
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 should not be deduced this way, it's brittle and very specific to current configuration.
We have to open a ticket to make it better. I think the best would be for the developer to specify suggestion source in the component, or we make a specific API point for the facet suggestion that seeks for right completion in the configuration.
* Handle the input event | ||
* It will update the query and call the getTaxonomiesTerms method to show suggestions |
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.
Good comment 🙂
Co-authored-by: Alex Garel <[email protected]>
# Conflicts: # frontend/README.md # frontend/src/search-a-licious.ts
return document.querySelectorAll( | ||
`searchalicious-facets[search-name=${this.name}]` | ||
); |
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 might not work with default name, but I will fix this with a commit before testing.
@Kout95 FYI I added a small commit with a very small refactor. |
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 ! Thank you @Kout95
What
Screenshots
Part of