Add new typography tokens to variables and utilities

[langermank]

What are you trying to accomplish?

This swaps in the new typography design tokens to our existing variables and utilities. Since this is all behind a feature flag in dotcom, it seems like the easiest way to test is by replacing the values of existing vars/utils.

What approach did you choose and why?

Since the new tokens are behind a feature flag, we rely on fallbacks for general support. In this PR, I chose to use the fallback slot for the original value rather than a 1-1 map to the new design token. That way, there should be no visual changes in dotcom while we still have the feature flag in place.

For any variable that does not have a fallback, that's because the original CSS didn't specify a value in the first place. The browser will just ignore it if it can't find the variable.

What should reviewers focus on?

• Confirm that the mixins I deleted aren't being used (I checked dotcom and couldn't find anything)
• Confirm that the fallbacks match the original value

Can these changes ship as is?

• Yes, this PR does not depend on additional changes. 🚢

[changeset-bot]

⚠️ No Changeset found

Latest commit: c87673c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[langermank]

@vdepizzol I'm a bit concerned about this one. It seems like most typography settings actually just fallback to this line-height on the body. I did set explicit line-height values on all the headings in PCSS, but there might be more in dotcom that we don't see reflected here. Any thoughts?

[vdepizzol]

@langermank I think it makes sense to keep the body line-height at 1.5, and then apply the body-lineHeight-medium token to any component that requires the body-medium typography style. Wdyt?

[langermank]

Sounds good to me! I wonder if we should add font-size and line-height to the p tag in typography-base 🤔

[simurai]

Confirm that the mixins I deleted aren't being used

Generally I haven't seen mixins being used much outside of PCSS. That said, if we remove them, should we mark this PR as a major change so that it won't break anything?

[tobiasahlin]

I have an open question that I’m struggling a bit with… do y'all have any thoughts on performance and overhead, and how it relates to this work? Because in a lot of cases we’re essentially going from shipping

.text-light: 300;

to shipping

:root {
  --base-text-weight-light: 300;
}

.text-light {
   font-weight: var(--base-text-weight-light, 300) !important;
}

In this example about 17 characters to define one style rule, to about 117 characters in total. I feel like we’re at risk of ballooning the size of the bundle(s) by applying this approach broadly, and in many instances (including this one) I'm not sure if we're adding value with the new abstraction layer. We obviously want the source of truth to be the design tokens but… could we do that in some other way? Do we need these values to stay CSS vars as we distribute primer/css? (One value with SCSS variables is that they're compiled down to raw values, which in many cases (definitely not all, for example for colors) would be enough, and produces a smaller bundle)

[langermank]

Generally I haven't seen mixins being used much outside of PCSS. That said, if we remove them, should we mark this PR as a major change so that it won't break anything?

@simurai excellent point. I think for this PR, I'm actually going to put them back in so we can get away with a patch here for testing. Then, once we're ready to roll out without fallbacks we can cut a major release where we remove unused code etc etc.

[keithamus]

In this example about 17 characters to define one style rule, to about 117 characters in total.

Let's not forget we use gzip; so character count is less important as repetition is deduplicated. So while characters go from 45 to 123 (173% increase), the byte gz byte count goes from 62 to 100 (105% size increase):

$ echo '.text-light { font-weight: 300 !important; }' | wc -c
45
$ echo '.text-light { font-weight: 300 !important; }' | gzip -c9 | wc -c
62

$ echo ':root {
      --base-text-weight-light: 300;
    }

    .text-light {
       font-weight: var(--base-text-weight-light, 300) !important;
    }' | wc -c
123
$ echo ':root {
      --base-text-weight-light: 300;
    }

    .text-light {
       font-weight: var(--base-text-weight-light, 300) !important;
    }' | gzip -c9 | wc -c
100

Brotli is even better due to the shared dictionary:

$ echo '.text-light { font-weight: 300 !important; }' | gzip -c9 | wc -c
40
$ echo ':root {
      --base-text-weight-light: 300;
    }

    .text-light {
       font-weight: var(--base-text-weight-light, 300) !important;
    }' | brotli | wc -c
93

[langermank]

@tobiasahlin thanks for raising this! I started a discussion so we can gather some insight on how to proceed.

Regardless of where this discussion goes, we'll need to use CSS variables for this initial test so we can provide a fallback to support everything outside of our feature flag. I'd love to move forward with this PR, and once we're satisfied with the test we'll use whatever method we agree on for utility classes later 🙌

[github-actions]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

[langermank]

Closing in favor of #2302