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

Line height tokens should not have units #54

Closed
geoffrich opened this issue Jun 16, 2021 · 3 comments
Closed

Line height tokens should not have units #54

geoffrich opened this issue Jun 16, 2021 · 3 comments
Assignees
Labels
Type: Design New or related Design work

Comments

@geoffrich
Copy link

Describe the bug
The tokens used for line height in WCSS use rem units. Per MDN, line height values should be unitless.

--auro-text-body-height-xs: 1rem;
--auro-text-body-height-sm: 1.25rem;
--auro-text-body-height-default: 1.5rem;
--auro-text-body-height-lg: 1.625rem;

WCSS uses auro-text-body-height-default to set a default line height, effectively setting line height to 24px regardless of font-size. At font sizes larger than 24px, the text will start to run into each other.

https://github.com/AlaskaAirlines/WebCoreStyleSheets/blob/6fcc2f36af78655186cc501e5d31d6aa4924173b/src/_essentials.scss#L66

To Reproduce
See this CodePen for an example.

The second paragraph is auro-size-xl and the text starts to run into itself.

Expected behavior
Line height does not have a unit

Screenshots
If applicable, add screenshots to help explain your problem.

image

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Firefox
  • Version: 89

Additional context
We ran into this in one of our sites and had to override the line-height to 1.5 in our styles.

@geoffrich geoffrich added the Type: Bug Bug or Bug fixes label Jun 16, 2021
@blackfalcon blackfalcon added Type: Design New or related Design work Type: Question and removed Type: Bug Bug or Bug fixes labels Jun 18, 2021
@blackfalcon blackfalcon removed their assignment Jun 18, 2021
@blackfalcon
Copy link
Member

This is a debate to take back to the design team. When the latest gen of tokens was created this was a debate between myself and the lead designer at the time.

In the end, the use of units was not accidental.

Is there a situation where the use of these tokens in production is causing issues with the development of UIs?

There is a growing discussion about the use of typography and this is not something engineering can address alone.

Who is the designer you are working with? I'd suggest raising this issue up to design leadership.

This issue has been reassigned and re-labeled accordingly.

@geoffrich
Copy link
Author

I can understand why we'd want to define line height units on our height tokens for heading levels and such, so I retract my request to remove units from the height tokens. However, I think setting a default line-height value with a unit on body has too many side effects. Since line-height is inherited, all children will have a default line-height of 24px regardless of their font size, which causes the issues I screenshotted above. 1.5 would be a more sensible default and reduce unintended consequences. Users would then be free to override the line-height on a per-element basis.

Maybe this should be a WCSS issue instead?

@blackfalcon
Copy link
Member

Maybe this should be a WCSS issue instead?

Yes. This is an issue regarding the expectation of type with design. Really, this is not even a WCSS issue, but something that needs to be proposed with the designers.

I will mention this to the UI designers and see if there are things we need to address.

leeejune added a commit that referenced this issue Jan 20, 2022
leeejune added a commit that referenced this issue Jan 25, 2022
leeejune added a commit that referenced this issue Jan 25, 2022
AuroDesignSystem pushed a commit that referenced this issue Jan 28, 2022
# [3.6.0](v3.5.0...v3.6.0) (2022-01-28)

### Features

* **new token release:** add new tier color tokens ([#54](#54)) ([5f73915](5f73915))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Design New or related Design work
Projects
None yet
Development

No branches or pull requests

3 participants