-
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
[Dataset page] Main block #11
Conversation
4927497
to
3a11f0d
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.
This looks great already! Thanks a lot!
The most significant thing I discovered while testing, was the disabled style of the favorite button when being clicked and also on hover on all the buttons. You can check their design system for reference (https://zeroheight.com/3b9e7ec84/p/937c12-boutons/b/32e1a2).
apps/datahub/src/app/common/button-primary/button-primary.component.html
Show resolved
Hide resolved
apps/datahub/src/app/common/favorites/heart-toggle/heart-toggle.component.ts
Show resolved
Hide resolved
apps/datahub/src/app/common/favorites/heart-toggle/heart-toggle.component.ts
Show resolved
Hide resolved
and display list with gn-ui components for now
…e-star by favorite-heart allowing to reuse the connect tooltip from gn-ui
needs higher version of gn-ui
…ommon classes into mel-card
5a05cf0
to
f47db9b
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.
LGTM 👍
Thanks for the refactoring :)
Thanks for the reviews @Angi-Kinas ! |
The PR adds the section visible on the screenshot below.
Amongst others
button-primary.component
that can be reused in other parts of the app. I decided for an angular component instead of a tailwind component to centralize the indication of the icons' path.To-dos: