feat(typography): New mixin to set exact baseline height of text elements. #3083
Conversation
abhiomkar
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3083 +/- ##
=======================================
Coverage 98.13% 98.13%
=======================================
Files 101 101
Lines 4444 4444
Branches 585 585
=======================================
Hits 4361 4361
Misses 83 83Continue to review full report at Codecov.
|
packages/mdc-typography/_mixins.scss
Outdated
| vertical-align: 0; | ||
| } | ||
|
|
||
| @if $trailing { |
There was a problem hiding this comment.
I would just make this another mixin. You could have mdc-typography-top-to-baseline($distance) and mdc-typography-baseline-to-bottom($distance)
Make sense?
And once you do that I don't think you will need this offset logic...That logic is a little weird so I want to see what it looks like as two mixins before I review more.
There was a problem hiding this comment.
Okay I looked at #3085 and I think i understand (somewhat) why this mixin's logic is more complicated than I expected it to be.
I think that mdc-list has a strange HTML structure for supporting two line lists. I think if we change that HTML markup (so that it is not mixing text elements with span elements), then we can more simply implement the baseline alignment fix.
|
|
||
| @mixin mdc-typography-baseline-top($distance) { | ||
| margin-top: 0; | ||
| line-height: normal; |
There was a problem hiding this comment.
I think setting line-height to normal will become unnecessary once we remove line-height from typography scale.
But for now I think it is okay.
There was a problem hiding this comment.
I would keep this for now. Secondary text in list and hint text on text field still has line-heights.
|
🤖 Beep boop! Screenshot test reportCommit 2bcc4b8 vs. No diffs! 💯🎉 |
Feat required for #2783 & #3038