-
Notifications
You must be signed in to change notification settings - Fork 0
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 page] Add search input and filters #22
Conversation
search-filters are just placeholders for now
that uses gn-ui components for now
to implement dropdown button template corresponding to mockups
only use mel-primary-button tailwind component here instead of angular component
also adds mel-title-line to styles for common red line over titles and makes checked checkboxes display in primary color
… mel prefix in class name
to prevent ghost displaying in search-results if no results found
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 this big work! 👍
A few comments and behavior issues I noticed :
-
In some cases, if i type only a part of a word that I know will match (eg : "carto" or "conven") in the home page, it goes to the search page alright, but returns no results (when it should). When I do that with some other words (eg : 'alpen'), it returns the right results.
-
Shouldn't filter's numbers be updated with the search? for instance this is misleading for the user IMO (5 results but multiple themes have a count in the filters). I don't know if it's something that's done in gn-ui actually.
-
@Angi-Kinas if you test, FYI, you'll need to run
npm install
first to upgrade gn-ui deps.
apps/datahub/src/app/dataset/dataset-information/dataset-information.component.html
Show resolved
Hide resolved
apps/datahub/src/app/search/search-filters/filter-dropdown/filter-dropdown.component.html
Show resolved
Hide resolved
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 this works like a charm :)
I noticed only a really small detail in the UI:
- the borders of the filter buttons are not rounded in the design, but it's almost not noticeable ;-)
@@ -2,7 +2,7 @@ | |||
<gn-ui-content-ghost | |||
ghostClass="h-[212px]" | |||
class="container" | |||
[showContent]="resultsReady" |
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 cleaning this up! I don't know why I implemented it with overrides...
Thanks for the detailed reviews @cmoinier @Angi-Kinas ! I've added a commit to address the rounded buttons and the tests. Thanks also for pointing out the following two points:
I totally agree that both behaviors are not ideal, but they are identical in gn-ui. |
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 addressing the comments. I had another look and for me it looks good to go 🚀
Just a note from the gn-ui daily regarding the two behaviors mentioned above:
|
This PR adds the search form to the search page and a load more button at the bottom of the page.
The search input has not been adapted to the design yet, as the search input from #23 will be reused for this.
Todos: