-
Notifications
You must be signed in to change notification settings - Fork 360
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
Vivre: move link pseudo states to theme.json #6150
Conversation
This is interesting and gave me a little pause. In this instance we are trading a little CSS for a lot of theme.json configuration. I don't think there's another way to do it, and I don't think it's wrong, but it's interesting to see how much less verbose CSS can be. We did seem to lose something in translation with the post title links: And the read more link: If there are other differences they have escaped me. |
8372b3b
to
0934b00
Compare
Fixed both issues. Seems like they lost in the transition. |
0934b00
to
2b787d7
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 @madhusudhand this is looking good and very cool to see the pseudo states API in action.
In addition to rebasing, I made one change so the links match the design: 2b787d7
Based on the figma, it looks like text links are underlined by default and not underlined on hover:
Let me know if the way I implemented it makes sense.
Previous comment deleted because I have no idea what I'm doing today. This looks great. 👍 |
Changes proposed in this Pull Request:
Moved css to theme.json for
:hover
,:focus
and:active
states. There are few style left in css because the style properties aren't supported in theme.json. ex:text-underline-offset
Related issue(s):
#6108