fix(font-size): properly set rem values #2119
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dancormier
added
the
do-not-merge
🦋 Changeset detectedLatest commit: 87f7070 The changes in this PR will be included in the next version bump. 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 |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
dancormier
changed the title
giamir
approved these changes
Jan 12, 2026
Contributor
giamir
left a comment
giamir
left a comment
There was a problem hiding this comment.
Thanks Dan for taking the time to research and fix this issue. ❤️
It looks good to be merged for me.
mukunku
approved these changes
Jan 12, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug with our font sizing that had downstream effects on styles that leveraged our
--s-fullcustom property. Previously, we were setting our base font size to a static14pxvalue at thehtmllevel. This resulted in values uses in classes such as our atomic static width/height classes to be inflated.For example, check out the width of the
.s-topbar--containerin our docs:Notice that it is
1361.23pxwide. It haswidth: var(--s-full), which should result in a width of1264px. With the changes in this PR, we get the intended width:As a bonus, this simplifies our usage of rem values dramatically since sizing more often tends to be divisible by
16px(the default base font size set on thehtmltag - keep reading for more details on this). This should also make the upcoming atomic size class updates simpler 🤞How this works
Standard browser default font size is
16px. By settingfont-size: 100%onhtml, you're saying use 100% of the size of the browser's set font size. If the user overrides that font size, the user's preference will instead be applied.On the
bodytag, arem-based font size can be set. This will be relative to the font size set onhtml. Since the default is16px, we can achieve our desired base font-size by setting thebodyfont size to0.825rem(which computes to14px).From MDN:
Note
The above quote mentions
embut the same logic applies toremwhen operating at thebodytag level.Notable changes
htmlnow has afont-sizeor100%--fs-basewith--fs-body1and removed--fs-basealtogether.s-proseof different sizes should have had font sizes that scaled relative to the parent's font size. We were usingremvalues when we should useem.--fs-*-relativecustom properties. They we're working as intended and were all either referenced in.s-proseor not at allremvalues through our codebase to their base-16 equivelent values (this px to rem converter is handy if you'd like to check my math)Other details
This should result nearly zero changes to the look of components (I think the
.s-prosesizes are the most notable change). I'm relieved to see that the visual regression tests have passed 😅How to test
I suggest hitting the local instance (or deploy preview) and just poking around a bunch. Notice that the docs site is no longer too wide. Verify that the font sizes compute to what is expected. Verify that any modified component still renders as expected.
For bonus points, try scaling your browser font size and notice the text scale with it (more work will need to be done in SPARK-154 to have all scaling live in harmony).