-
Notifications
You must be signed in to change notification settings - Fork 86
refactor!: replace abbr SVG with <div> element #9330
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
base: main
Are you sure you want to change the base?
Conversation
ee4c445
to
ae6bc64
Compare
98edddc
to
d1e0e1b
Compare
d1e0e1b
to
b1ec14e
Compare
|
} | ||
|
||
:host([theme~='xlarge']) [part='abbr'] { | ||
font-size: 2.5em; |
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.
note: These values were originally calculated with SVG scaling in mind, so they are no longer valid. However, I was able to achieve a relatively close result using font-size: round(up, var(--_avatar-size) * 0.4, 3px)
@jouni let me know what you think.
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’s a bit complex at first glance, but who am I to criticize anyone’s CSS of that, when I’m often the one coming up with very elaborate constructs 😆
I didn't remember to test it locally, but am I right to assume that the font size adjusts with 3px increments, i.e., it’s always divisible by 3?
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’s a bit complex at first glance, but who am I to criticize anyone’s CSS of that, when I’m often the one coming up with very elaborate constructs
Yeah, it might not be immediately clear...
var(--_avatar-size) * 0.4
could be replaced with 0.4sqh
assuming we make avatar a CSS container, but it seemed to me a good idea to avoid CSS containers when values are already available.
I didn't remember to test it locally, but am I right to assume that the font size adjusts with 3px increments, i.e., it’s always divisible by 3?
Yes, that's right.
I don't like that now the abbreviation isn't exactly centered, when we rely on the line-height to center it. But that depends on the font, how the characters are centered in the line box. I’d rather make the avatar host a flex container and use Was there some issue using |
Description
Replaces the avatar's abbr SVG with a
<div>
element, following up on #9316, which made the same change for the icon SVG.This also fixes the issue of abbr rendering with the wrong color in the new base styles:
Warning
This change may be breaking, as
font-size
on[part=abbr]
now behaves differently due to removal of SVG scaling.Type of change