Skip to content
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

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Conversation

minikomi
Copy link
Contributor

The official docs currently appear like so:

Screen Shot 2020-01-16 at 14 46 03

However, using the lineHeight property does not set the unit to "px" in the generated css rule.

Screen Shot 2020-01-16 at 14 50 18

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.

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.
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 16, 2020

No bundle size changes comparing bb3a8aa...b872a1a

Generated by 🚫 dangerJS against b872a1a

Copy link
Member

@eps1lon eps1lon left a 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.

@eps1lon eps1lon added the docs Improvements or additions to the documentation label Jan 16, 2020
@eps1lon
Copy link
Member

eps1lon commented Jan 16, 2020

@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.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 16, 2020

@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).

@eps1lon eps1lon changed the title [docs] Line height property does not set px units [docs] Use reasonable unitless line-height for Box Jan 16, 2020
@eps1lon eps1lon merged commit c9d19a8 into mui:master Jan 16, 2020
@eps1lon
Copy link
Member

eps1lon commented Jan 16, 2020

@minikomi Much appreciated, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants