-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Remove svg.svg class, restore .rss-icon #24667
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
Conversation
Maybe it needs other fine tunes. I would suggest to have a framework level solution for "button svg icon margin magic", instead of adding a lot of "gt-xxx", it doesn't look good to me.
I guess that's because the icon height and line-height doesn't match? If we can give these icons proper height, no trick any more IMO? |
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.
I won't block it (and I admit that my approach is not good enough, and I missed another ".svg" class because it's in a separate file .... )
But I hope this is the last time for the "svg icon margin/align trick".
Really looking forward to a framework level clear solution.
As I said, it's likely not possible to satisfy all cases with a "global" style. The only true vertical centering is
|
Fix regression from #24476 where the
svg.svg
class misaligns SVG icons across the site and streched buttons unintentionally in vertical height.Before (button 30.3px):

After (button 30px):

vertical-align: middle is not suitable to align icons to text because
Example of
vertical-align: middle
from MDN:So I think the existing
vertical-align: text-top
is generally still the best bet: