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

Update line-height behavior to respect token value, size headings relative to body text #291

Merged
merged 2 commits into from
Feb 8, 2022

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 7, 2022

Why:

  • So that assigned line-heights match expectations based upon the available token options, not influenced by either font family or scale
  • So that heading line-heights can be smaller than body content, but only in the case that the actual value would be greater than that of body text (and in all other cases, using the body line-height for headings)

Note that this diverges slightly from original decisions to use a line-height value of 1.33 for headings. USWDS includes a line-height token for 1.35. Previously, due to normalization, this value would never take effect. However, with this pull request also removing the normalization behavior, the 1.35 actual value for headings can be used. Since this is very close to 1.33, and avoids circumventing USWDS tokens, would we be okay with this value, or should we force it to 1.33?

Live preview URL: https://federalist-2f194a10-945e-4413-be01-46ca6dae5358.app.cloud.gov/preview/18f/identity-style-guide/aduth-normalize-heading-line-heights/utilities/typography/

Effective difference in line-height:

Heading Before After
h1 1.4 1.35
h2 1.4 1.35
h3 1.4 1.5
h4 1.4 1.5
h5 1.4 1.5
h6 1.4 1.5

Screenshots:

Before After
design login gov_utilities_typography_ localhost_4000_utilities_typography_

**Why**:

- So that assigned line-heights match expectations based upon [the available token options](https://github.com/uswds/uswds/blob/61a0d99f0e6b36c3758948ba6ac46140abc5e585/src/stylesheets/theme/_uswds-theme-typography.scss#L363-L371), not influenced by either font family or scale
- So that heading line-heights can be smaller than body content, but only in the case that the actual value would be greater than that of body text (and in all other cases, using the body line-height for headings)
@nickttng
Copy link
Member

nickttng commented Feb 7, 2022

Okay w/ 1.35 as the value for both H1 and H2.

@anniehirshman-gsa Any preference?

Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line height is looking good! And I agree, 1.35 it is

@aduth aduth merged commit 67b124d into main Feb 8, 2022
@aduth aduth deleted the aduth-normalize-heading-line-heights branch February 8, 2022 16:36
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

Successfully merging this pull request may close these issues.

3 participants