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

Use unitless line-height #847

Closed
twhite313 opened this issue Sep 6, 2017 · 6 comments
Closed

Use unitless line-height #847

twhite313 opened this issue Sep 6, 2017 · 6 comments
Assignees
Milestone

Comments

@twhite313
Copy link

Currently line-height is being set to $baseline which starts out as 1.55rem. The result is that the calculated value is 24.8px (16px * 1.55rem). In some cases, font sizes are changed above that 24.8px size, resulting in text that overlaps when lines wrap.

Line-height should always be unitless. Suggest changing $baseline to 1.55 (or more appropriate number).

References:

@paulcpederson
Copy link
Member

I would tend to agree that line height should be unitless. We've used the $baseline var in other places as a way to add padding that sustains vertical rhythm, so we'd have to just go through and convert all those to rems again. Marking as bug, and assigning to next release.

@paulcpederson paulcpederson self-assigned this Sep 6, 2017
@paulcpederson paulcpederson added this to the v1 milestone Sep 6, 2017
@twhite313
Copy link
Author

Would it be easier to leave $baseline as is, and just change the line-height calls? Maybe a new variable $line-height?

@paulcpederson
Copy link
Member

That could work. Or maybe $baseline-ratio or something

@twhite313
Copy link
Author

A parallel conversation cropped up yesterday with Design -- there is a desire to move font-size-1 and up to line-height: 1.25

We should engage with them to determine all of the needs.

@HeathMeyette
Copy link

Assigning to Tim. @tim-white-esri please feel free to reassign to your team members that have additional bandwidth.

paulcpederson added a commit to paulcpederson/calcite-web that referenced this issue Mar 29, 2018
macandcheese pushed a commit that referenced this issue Mar 30, 2018
* fix pre and post on large screen size

* fix edge case where imports file still generates css

* prevent calcite web from being imported twice

* #806 - fix pre for nested first child on largest screen size

* add test case for nested column with pre- to grid example

* #889 - font size 17px => 16px ⚠️

* #847 - use unitless line-height

* tweak button padding so they remain more stable after font-size switch

* #901 - change theme toggle markup in hopes of bug fixing

* remove extra pre-/post- classes
macandcheese pushed a commit that referenced this issue Apr 3, 2018
* fix pre and post on large screen size

* fix edge case where imports file still generates css

* prevent calcite web from being imported twice

* #806 - fix pre for nested first child on largest screen size

* add test case for nested column with pre- to grid example

* #889 - font size 17px => 16px ⚠️

* #847 - use unitless line-height

* tweak button padding so they remain more stable after font-size switch

* #901 - change theme toggle markup in hopes of bug fixing

* remove extra pre-/post- classes

* icon classes should have vertical-align:middle

* tweak size/padding on code and label elements

* simplify quick reference
@paulcpederson
Copy link
Member

all of these have been converted, will be part of next release

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

No branches or pull requests

4 participants