DSD-1924: Heading component mobile line height #1729
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
bigfishdesign13
added
the
Needs Review
src/theme/components/heading.ts
Outdated
| @@ -172,7 +172,7 @@ export const headings = { | |||
| }, | |||
| fontWeight: "heading.heading8", | |||
| letterSpacing: "0", | |||
| lineHeight: "1.50", | |||
| lineHeight: { base: "1.50", md: "1.50" }, // This value is intensionally the same for mobile and desktop. | |||
There was a problem hiding this comment.
You should be able to just set this to lineHeight: "1.50" and it should be set for both mobile and the medium breakpoint.
As an aside, is 1.50 not the same as 1.5?
There was a problem hiding this comment.
Right. I will simplify and then update my comment to explain why this one is not setup to be responsive like the others.
There was a problem hiding this comment.
Also, can you share documentation about the difference of 1.50 and 1.5 for line-height?
There was a problem hiding this comment.
I don't know if it's true that those two values 1.50 and 1.5 are not the same so I was asking, but I can look for resources.
There was a problem hiding this comment.
Sorry, I read that as a statement and not as a question. Yes, 1.50 and 1.5 are numerically the same. However, the designer in me opted to use the same format as those that do require two decimal places. Symmetry. Consistency. All that.
bigfishdesign13
added
Ship It
Fixes JIRA ticket DSD-1924
This PR does the following:
Headingcomponent to addline-heightstyles for mobile.How has this been tested?
Accessibility concerns or updates
Accessibility Checklist
aria-liveis used, a screenreader was used to verify the text is read.refs, focus management was reviewed.Open Questions
Checklist:
Front End Review: