[WNMGDS-2895] Update font and focus color design tokens in CMS.gov theme #3227
Conversation
malloryiden
added
Impacts: CMSgov
malloryiden
requested review from
tamara-corbalt,
jack-ryan-nava-pbc and
andream-cmsds
| @@ -1149,6 +1149,16 @@ | |||
| "codeSyntax": {} | |||
| } | |||
| } | |||
| }, | |||
| "semibold": { | |||
There was a problem hiding this comment.
just a heads up that this could cause issues. semibold has no meaning on the web.
- some fonts don't have a semibold/600 value
- as far as markup is concerned, there's no "semibold" tag to use in CMS (content management systems) - we only have normal text tags (like
<p>) and<strong>
I wonder if it makes sense to abstract the idea of bold vs not-bold for future theming? Will themes have a need for nuanced weights (probably not since non-retina screens won't be able to differentiate), so maybe having a token representing font weights as "normal" or "default" and "heavy" (or whatever standin word for your bolded type) could prevent the issues presented above?
What I mean is all themes would have a "normal" and "heavy" font weight token, but the value of those tokens could differ across themes. Mgov could have its "heavy" as 700 and CMS could have its heavy as 600 without the need to give all themes all weights.
There was a problem hiding this comment.
Thanks for this! Digging this idea. Could you speak more on the use cases where this would introduce issues? I think variable fonts also can solve part of this (your point 1), but perhaps we'd still run into the CMS issue.
Could this introduce other issues in a case where a theme wants to use two "heavy" weights (like both 600 and 700)?
There was a problem hiding this comment.
sure thing!
so first, we used to support bold, semibold and regular font weights (i think also thin at one point?) but we reduced them to the current selection to shrink our bundle size.
the variable "semibold" was recently deprecated from the DS (as of 3 weeks ago). but the value for it (600 weight) is still active in the system. you can see this on the mgov tokens
variable fonts could solve this problem, but are also large file sizes, so there's a tradeoff as far as asset downloads go. the same asset size issue exists when adding additional font weight files.
There was a problem hiding this comment.
Great context, thank you! (Also lol for deprecating semibold and then introducing it again now).
|
Oh! And since this is a font family change, I think this is considered a breaking change (major version) |
|
| @@ -1158,6 +1158,16 @@ | |||
| "codeSyntax": {} | |||
| } | |||
| } | |||
| }, | |||
| "semibold": { | |||
There was a problem hiding this comment.
this is what i'm talking about above - the "bold" token is mapped to the 600 weight, meaning there is no true bold/700 weight font in mgov
| @@ -1149,6 +1149,16 @@ | |||
| "codeSyntax": {} | |||
| } | |||
| } | |||
| }, | |||
| "semibold": { | |||
There was a problem hiding this comment.
sure thing!
so first, we used to support bold, semibold and regular font weights (i think also thin at one point?) but we reduced them to the current selection to shrink our bundle size.
the variable "semibold" was recently deprecated from the DS (as of 3 weeks ago). but the value for it (600 weight) is still active in the system. you can see this on the mgov tokens
variable fonts could solve this problem, but are also large file sizes, so there's a tradeoff as far as asset downloads go. the same asset size issue exists when adding additional font weight files.
|
@malloryiden can you update the title of this PR to be WNMGDS-2895? |
malloryiden
changed the title
|
The number of snapshots changed is absolutely massive. I'm wondering if it makes sense to make a few of these changes in broken up pull requests so we don't get a diff of 800+ files? What do you think @kim-cmsds @tamara-corbalt @malloryiden ? |
There was a problem hiding this comment.
I'm approving! Nice work @malloryiden
I do think we should have a larger conversation around an acceptable number of files changed. 846 is a lot, and maybe we can come up with strategies for breaking up big pull requests (or pull requests with lots of visual changes) so we keep the size to something more manageable.
cc @kim-cmsds @tamara-corbalt I'm really curious about what you two think. Thanks!
|
I am curious about what would have happened if this work was split out into 4 separate PRs based on the bullet points that describe the changes made. |
Summary
How to test
Checklist
[WNMGDS-####] Titleor [NO-TICKET] if this is unticketed work.Type(only one) label for this PR, if it is a breaking change, label should only beType: BreakingImpacts, multiple can be selected.