Add font-size to html tag to make rem units "safe"
#2404
Conversation
馃 Changeset detectedLatest commit: 75bfbe9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
@langermank Is this to override the user's font-size preferences?
Maybe fine since it doesn't seem to have an effect on current production. But it might limits other places that use Primer CSS? Not sure if we care. If it's a concern, an alternative might be to add it to the tokens-v2 bundle, since it seems that is currently breaking when users change their settings.
There was a problem hiding this comment.
@langermank and I chatted about this, and the accessibility considerations it represents.
@simurai is correct that this will impact a user's custom set font size. Setting it to 16px means that it may reduce the font size for some folks who have set a larger size, which may negatively impact their ability to read GitHub content.
Font size adjustment and browser zoom are two separate concerns, although they both can be utilized at the same time. Both features allow for increasing or decreasing text size, provided things are coded in a way that allows it.
A static 16px size declared on the html element will prevent user-initiated font size adjustment. User-initiated browser zoom will not.
That said, GitHub's use of pixel values does have some side effects that may negatively impact accessibility, namely media queries, content obscured by overflow, and some text resizing techniques. rem units are a more flexible, accessible, and modern way of specifying measurements for a lot of UI components, and I'm heavily invested in them being adopted in place of px.
I am okay with merging this knowing that the current px-based approach is already giving us accessibility debt. @langermank's changes, however, would provide a stable scaffold to begin to update Primer to deprecate px units in favor of rem units. This would, in turn would give us a lot more predictability about GitHub UI's behavior when zooming and font size adjustments take place.
Once we have rems in place, we could then intentionally remove this declaration.
Applicable Success Criterion currently impacted/may be impacted during our interim state:
Potentially also:
What are you trying to accomplish?
We've had a feature flag in dotcom for awhile testing
remunits. We know that nothing is "breaking" with this change, so it's safe to rollout with the next Primitives major release v8. However, since parts of GitHub (most of it) still use pixels, the browser zoom experience is degraded by introducingremunits. Some UI will scale, and other's won't, which causes a lot of visual bugs.I'm setting a hard pixel value on the
htmltag that allows us to keep usingremunits combined with pixels while we transition. This shouldn't introduce any new accessibility issues, and ideally creates a safe environment to slowly transition toremunits.What should reviewers focus on?
Whaddya think, good idea? Bad idea?
Can these changes ship as is?