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

Unitless Line-height Issues #81

Closed
yulolimum opened this issue Jul 1, 2014 · 6 comments
Closed

Unitless Line-height Issues #81

yulolimum opened this issue Jul 1, 2014 · 6 comments

Comments

@yulolimum
Copy link

Good day!

It's been a long day, so please excuse me if I'm spacing out on this one.

Use this snippet as reference:

$base-font-size: 16px;
$base-line-height: $base-font-size * 1.75;
$unitless-line-height: $base-line-height / ($base-line-height * 0 + 1);
$header-line-height: 1;
$base-border-radius: em(3);

There are a couple issues that I'm seeing with this line: $unitless-line-height: $base-line-height / ($base-line-height * 0 + 1);

Wouldn't ($base-line-height * 0 + 1), just equal 1? You're basically, just stripping $base-line-height of its units and setting that, which introduces the next issue:

According to the Mozilla line-height summary, the "number value" is:

The used value is this unitless multiplied by the element's font size. The computed value is the same as the specified . In most cases this is the preferred way to set line-height with no unexpected results in case of inheritance.

So, if we calculate $unitless-line-height, we would get 28. If 28 is used as a line-height value without units, the resulting line-height would be roughly 448px. This is wrong.

I think this is what you guys were looking for:

$unitless-line-height: $base-line-height / $base-font-size;

This will result in 1.75 and strip the units while at it - which is what you want to use for unitless line-height.

@kylefiedler
Copy link
Contributor

@yulianglukhenko check out the strip units function in bourbon: https://github.com/thoughtbot/bourbon/blob/master/app/assets/stylesheets/functions/_strip-units.scss and more importantly this thread in stack overflow: http://stackoverflow.com/questions/12328259/how-do-you-strip-the-unit-from-any-number-in-sass
The most important line there is:

You need to divide by 1 of the same unit.

So ($base-line-height * 0 + 1) is 1 and what ever the unit is (in your example it'd be pixels). Then $base-line-height / 1px would give you the unitless value of $base-line-height

$unitless-line-height: $base-line-height / $base-font-size; would give you 28px / 16px the result would be 1.75px which is not what we want the line-height to be.

@yulolimum
Copy link
Author

I've tested this numerous times and it works for me; however, you can strip the units however is best for you. That's not the real issue here.

The real issue is the fact that the math is wrong. This: $unitless-line-height: $base-line-height / ($base-line-height * 0 + 1); would be this: 28px / 1 would be 28. A value of 28 for the line-height is 448px if the font size is 16px. Please fix this as it's not usable the way it's set up now.

@kylefiedler
Copy link
Contributor

@yulianglukhenko Except the $base-font-size in Bitters isn't 16px it's 1em so the math comes out to 1.75 for Bitters out of the box. Is your issue that you think that if you change the value for $base-font-size that you shouldn't have to change the values for $base-line-height and $unitless-line-height?

@yulolimum
Copy link
Author

Damn.. that's my issue. Completely forgot that base-font-size is 1em out of the box. Thanks for the explanation and sorry for the hassle.

@kylefiedler
Copy link
Contributor

@yulianglukhenko No problem. You've pointed out an issue with some of those variables which is a good thing. There is a long discussion on $unitless-line-height over here: #80 (diff)

It is probably going to change.

Is there a reason that you aren't using ems? Or event the em() function in Bourbon?

@yulolimum
Copy link
Author

I usually do for simple websites. However, the web app I'm currently working on was designed by a print designer and has a lot of weirdly positioned elements that need to have strict font-sizes. Since this will be a responsive app, the decision was made to go with px instead of em.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants