-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] Use reasonable unitless line-height for Box #19260
Conversation
The official docs currently appear like so: <img width="847" alt="Screen Shot 2020-01-16 at 14 46 03" src="https://user-images.githubusercontent.com/364725/72497140-13a00180-386f-11ea-9927-edc7d016a268.png"> However, using the lineHeight property does not set the unit to "px" in the generated css rule. <img width="922" alt="Screen Shot 2020-01-16 at 14 50 18" src="https://user-images.githubusercontent.com/364725/72497315-87daa500-386f-11ea-8506-38a70da70eb8.png"> Assuming this is the correct action for the property, the docs should reflect the final state rather than the misleading "10px" label which currently exists.
No bundle size changes comparing bb3a8aa...b872a1a |
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.
Well spotted. I encountered the same issue recently because I was used to JSS converting unitless to pixel values. line-height
is special because unitless and pixel values are possible values for the css porperty. Unitless is relative to the font-size while pixel values are absolute. See https://developer.mozilla.org/en-US/docs/Web/CSS/line-height#Values
That being said we probably want to use 10px
. 10x the font-size seems unreasonable.
@oliviertassinari Was the intentation actually to set the line-height to 10 times the font size? Even Material typography doesn't go that big. 10px seems more like it was intended. |
@eps1lon Right, thanks. 10x sounds too much for this demo. I think that using something like 2x will be better. (unitless line-height are recommended for the inheritance behavior). |
@minikomi Much appreciated, thanks. |
The official docs currently appear like so:
However, using the lineHeight property does not set the unit to "px" in the generated css rule.
Assuming this is the correct action for the property, the docs should reflect the final state rather than the misleading "10px" label which currently exists.