-
Notifications
You must be signed in to change notification settings - Fork 37
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
[Webcomponents] : Full text search #1122
Conversation
Affected libs:
|
📷 Screenshots are here! |
91cced1
to
6525bb0
Compare
This needed some modifications in the ES code to generate queries
caacc5e
to
b877308
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 for the fixes @cmoinier ! This mainly looks good to me.
Regarding the e2e tests, I think the webcomponents-e2e
app only displays the list as it misses the "preparation" steps. The webcomponents
app actually behaves the same way. Maybe we can solve the issue by generating a demo-e2e
app? Which command did you use to generate the webcomponents-e2e
one?
apps/webcomponents/src/app/components/gn-search-input/gn-search-input.component.ts
Outdated
Show resolved
Hide resolved
libs/feature/search/src/lib/fuzzy-search/fuzzy-search.component.html
Outdated
Show resolved
Hide resolved
libs/feature/search/src/lib/fuzzy-search/fuzzy-search.component.ts
Outdated
Show resolved
Hide resolved
Thanks for the review! I think you are right with the 'demo-e2e' app idea. I did copy/pasted the datahub-e2e app for this work, because the cypress commands were adding a general cypress app and not a dedicated webcomponents one. I think I can easily switch the config from wc app to the demo app, I will try that now :) EDIT : I did try (see commit), and it doesn't work much better, the list is still displayed... |
b877308
to
1a7bc01
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 for the adaptions @cmoinier ! I'll approve partially apart from the e2e tests that I've not gotten to work yet neither. Maybe we can push them to a separate branch for now.
9a8929f
to
4170b12
Compare
Description
This PR makes numerous fixes on the
gn-search-input
webcomponent.results-list
implementation ingn-search-input-and-results
Architectural changes
none
Quality Assurance Checklist
breaking change
labelbackport <release branch>
label