Convert header and body font sizes to rem

[koddsson]

From https://github.com/github/accessibility/issues/147:

The Web Content Accessibility Guidelines (WCAG) does not specify a minimum font size for the web.

The Dotcom is using px as absolute units for font-size. The WCAG recommends to use relative units like em and rem for font sizes. All major browsers have a default font size of 16px, so we can set that size as the base font-size and calculate the rest proportionately with rem units.

em units are relative to the font size of their own element, and rem units are relative to the root element.

From WAI:

The font size should be defined in relative units, such as percentages, em or rem.

This pull request creates a px to rem converter and uses that function to ensure that the body font size and the header variables output rem units.

The concrete benefit here is that users who change their font size in their browser will see text larger or smaller after their changes to the settings. Right now, github.com will not resize its fonts even when a user changes this setting.

References

• https://www.w3.org/WAI/tutorials/page-structure/styling/
• https://css-tricks.com/accessible-font-sizing-explained/
• https://www.sitepoint.com/understanding-and-using-rem-units-in-css/

/cc @primer/ds-core

[changeset-bot]

🦋 Changeset detected

Latest commit: 3f5419b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/css	Minor

Not sure what this means? Click here to learn what changesets are.

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

[jonrohan]

That test failure is weird, (I don't think something you did wrong) going to take a closer look

[jonrohan]

We have a rem( mixin already for this in support.

[koddsson]

I did see one in the marketing mixin directory but thought since it wasn't in the base we'd want to keep it separate? Should I use that one?

[bolonio]

It sounds like we can either use the same, or if they're in different repos, maybe clone it and maintain both at the same time, since we don't have a share library of utilities, right?

[jonrohan]

One thing I'd like to see us do in the implementation is, to not use the sass mixin at all. Currently it's using some deprecated math sass functions that is blocking us from moving the build to dart-sass. If we could compute these new rem values and enter them directly that would be ideal. We could show our math in a CSS comment above each line to show that it's the px equivalent.

[simurai]

Here a PR where the values are already computed to rems: #1538

[simurai]

The concrete benefit here is that users who change their font size in their browser will see text larger or smaller after their changes to the settings. Right now, github.com will not resize its fonts even when a user changes this setting.

Hard to say for sure without testing, but my guess is that mixing rem with existing px values would cause certain things to be misaligned once the user starts to change the font-size in their browser settings. It would probably be an "all or nothing" approach. There is a proposal to change all sizing here: https://github.com/github/primer/issues/41 /cc @vdepizzol

I might can try to do some sort of local test before we would merge this.

[bolonio]

@simurai totally agree with you in the "all or nothing", where all is rem and nothing is px. Of course, we need more testing, but the computed values will always be in px. No design or content shall be affected. We're trying to fix an accessibility issue here, so a small part of @vdepizzol proposal github/primer#41

See as well the whole proposal here: https://github.com/github/primer/issues/257

[simurai]

but the computed values will always be in px.

I think this PR would change the computed value to rem so that the browser would read the font size as:

body {
  font-size: .875rem;
}

mixing rem with existing px values would cause certain things to be misaligned once the user starts to change the font-size in their browser settings.

Here a simulation what I meant by the ☝️ above that only certain parts of the UI would be affected but some parts stay the same with a fixed px value:

This is probably not 100% correct, I just overrode the font-size of a few components in DevTools to font-size: .875rem;.

[simurai]

An alternative that is a bit more predictable and maybe still a "quick win" is to opt-in areas like the comments and descriptions of issues/PRs:

.comment-body {
  font-size: .875rem;
}

[broccolini]

I believe this PR should probably be closed for now as per this comment after evaluating the proposal. I really appreciate y'all exploring this, it will be part of new work, but we want to do it the right way. Happy to discuss more about where we're heading with theming work to help you understand the direction we're heading in, and to help you with future contributions.