-
Notifications
You must be signed in to change notification settings - Fork 3
feat(dsdl): Reintroduce separate heading and body font stacks #554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for cal-itp-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@cmajel Please review THIS preview site with the change to Noto for body and let me know if anything stands out as unreleasable. Thanks! |
| /* Space Grotesk: Variable weight, 400–700 */ | ||
| /* Source Code Pro: 400 only */ | ||
| @import url('https://fonts.googleapis.com/css2?family=Space+Grotesk:wght@400..700&family=Source+Code+Pro&display=swap'); | ||
| @import url('https://fonts.googleapis.com/css2?family=Noto+Sans:ital,wght@0,400..800;1,400..800&family=Source+Code+Pro&family=Space+Grotesk:wght@400..700&display=swap'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we're using full 800 weight anywhere for Noto Sans. also, what are the double set of semicolon delimited weight value ranges for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DSDL does specify 800 (extra-bold) as an available option, however, that was in the context of Space Grotesk being used for everything. None of the core styles would have Noto Sans set in extra-bold. AND Christine mentioned wanting to use 600 as the standard bold in body copy, so limiting it to that might be a foolproof way to guarantee it 🤔
| h5, .h5, | ||
| h6, .h6 { | ||
| font-family: var(--dsdl-heading-font-stack); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these exact same declarations are repeated below, yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, you're right. I wrote it before adding the ones below and forgot to drop it.
jgravois
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know i'm not @cmajel but i think this iteration looks imminently shippable
👍
|
Looks good to me! Ship it 🚀 |
|
thx @cmajel 🙏 . since @Scotchester is out today i'm going to merge this one (and #547) to unblock #537. if relevant, my nits can certainly be addressed in a follow-up PR. |
ℹ️ PR is to base branch
feature/dsdlℹ️Per feedback from @cmajel in #547