Automattic
A4A: Introduce new mixins for consistent typography scale
Related P2 post: pfunGA-1Q2-p2
Proposed Changes
This PR introduces new SCSS @mixins that can be easily used to create a consistent typographic scale across all of A4A. It also updates most A4A touch points to use these new styles.
The introduced styles match those presented in pfunGA-1zi-p2 and in Figma https://www.figma.com/design/Ij0yoCBbL4ZeZffvv1278l/A4A-Library?node-id=6431-138763&t=oTgu09K8vee8VOPd-11
They also match Gutenberg's nomenclature; we've added XXL as a new style.
- Heading XXL:
a4a-font-heading-xxl - Heading XL:
a4a-font-heading-xl - Heading LG:
a4a-font-heading-lg - Heading MD:
a4a-font-heading-md - Body LG:
a4a-font-body-lg - Body MD:
a4a-font-body-md - Body SM:
a4a-font-body-sm - Label / Button:
a4a-font-label-button
They can be used by either using them wholly, or by overriding specific properties. They all share these properties: $font-size, $font-weight, $line-height. Additionally, Heading XXL, XL and LG support an additional property: $letter-spacing.
Usage examples:
- Wholly:
@include a4a-font-heading-xl - Overriding one property:
@include a4a-font-body-md($font-weight: 500) - Overriding multiple properties:
@include a4a-font-heading-xxl($font-weight: 200; $letter-spacing: 4px)
Questions
Got a couple of questions for @Automattic/jetpack-genesis:
- Is it ok to add global mixins to Calypso like this 6c7cc1ff493e641f34791a9a16ec9379a1869dab ? Is it going to affect the package size of Calypso? I couldn't figure out a different way of adding these locally to A4A that would work. CC'ing @sgomes if you have any tips here.
- For some reason, the mixins mentioned above didn't work in the Marketplace section, so I had to duplicate them here: 8516639144485bd4abc9c9591d10a2e75ddbd7dd — any reason why the global mixins aren't usable here?
Testing Instructions
- Fire up this PR.
- Go to A4A.
- Make sure all touch points look ok and that nothing is breaking.
Screenshots
| Before | After |
|---|---|
Jetpack Cloud live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-112735&env=jetpack |
Automattic for Agencies live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-112735&env=a8c-for-agencies |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:
Sections (~22 bytes removed 📉 [gzipped])
name parsed_size gzip_size
domains -40 B (-0.0%) -22 B (-0.0%)
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.
Legend
What is parsed and gzip size?
Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.
Generated by performance advisor bot at iscalypsofastyet.com.
Hey @keoshi! 👋
Is it ok to add global mixins to Calypso like this https://github.com/Automattic/wp-calypso/commit/6c7cc1ff493e641f34791a9a16ec9379a1869dab ? Is it going to affect the package size of Calypso? I couldn't figure out a different way of adding these locally to A4A that would work. CC'ing @sgomes if you have any tips here.
Mixins are an exclusively compile-time construct that gets used to generate CSS, which is to say that they don't actually end up in the generated CSS, nor do they introduce modularization issues through runtime dependencies. They do have the potential to bloat the resulting code when used carelessly (because of combinatorial explosion), but that's no different than many other Sass features.
In this case, it appears that there isn't a significant difference between what was there before and what the mixin usage will generate 👍
I'm going to add the Loop team from Dotcom as a reviewer. We are touching on common styles in the ItemsDashboard and in the Layout.
Thanks for doing that, @cleacos.
Note for myself to fix: the one obvious regression I see here is in the preview panel's title:
| Before | After |
|---|---|
Edit: fixed in https://github.com/Automattic/wp-calypso/pull/91798/commits/3454db68a5c40dd661aa810afb56fdbd1668e10f
@yashwin Thanks for the thorough review, appreciate it!
Re: font-weight comments: yes, I went through them one by one and standardized everything making sure they all looked good. It's going to differ from what we have now, but it's going to create much more consistency across the entire system.
I removed the code, and it looks like it is working fine. Am I missing something here?
It wasn't working for me for some reason, but if it works than even better! Just to be clear, you're referring to completely removing this file, right? client/a8c-for-agencies/sections/marketplace/mixins.scss
Re:
font-weightcomments: yes, I went through them one by one and standardized everything making sure they all looked good. It's going to differ from what we have now, but it's going to create much more consistency across the entire system.
Awesome!
It wasn't working for me for some reason, but if it works than even better! Just to be clear, you're referring to completely removing this file, right?
client/a8c-for-agencies/sections/marketplace/mixins.scss
No, I meant removing the newly added code from that file starting from:
// ==========================================================================
// Automattic for Agencies: Typography definitions
//
We will still need this file.
Where's the facepalm animated emoji when you need it? Sorry, @yashwin, of course we still need that file. 😅
For some reason the Marketplace wasn't taking on the new styles without these local mixins, but I just confirmed it works now. 🤷
This should now be ready for a final review. Thanks so much!
This PR modifies the release build for the following Calypso Apps:
For info about this notification, see here: PCYsg-OT6-p2
- notifications
- odyssey-stats
- wpcom-block-editor
To test WordPress.com changes, run install-plugin.sh $pluginSlug update/a4a-type-scale on your sandbox.
Thanks so much for your review @Addison-Stavlo. Fixed the letter-spacing difference for the Dotcom dashboard in https://github.com/Automattic/wp-calypso/pull/91798/commits/ea8ee680810888dfe206452ab93e4ba0905ae8de
Could you take another look, please? Thanks in advance!
"SF Pro Display", sans-serif is also what Im seeing on the corresponding domains/plugins pages.
That shouldn't be happening in the first place, @Addison-Stavlo. "SF Pro” corresponds to the system font in Apple (-apple-system), so we should always use the system font stack defined here:
https://github.com/Automattic/wp-calypso/blob/a1b36280a13857334ca6afd7a3d93ad357f0e053/packages/components/src/styles/_typography.scss#L1
Plus, the “Display” variant of “SF Pro” is deprecated and should not be used.
FYI @lucasmendes-design: we're making the typeface changes here.
Interesting @keoshi !
Il defer to @lucasmendes-design what we want to do regarding those fonts on the dotcom side. Note this PR makes them inconsistent (changes the one on Sites but not domains/plugins), but we can handle that on our side with a follow up if needed.
Note this PR makes them inconsistent (changes the one on Sites but not domains/plugins)
This was fixed for Domains in https://github.com/Automattic/wp-calypso/pull/91798/commits/f3bba30af4d20d07f0fc300eead10d27da502d8b and for Plugins in https://github.com/Automattic/wp-calypso/pull/91798/commits/d426e929c896d64c1db6259c24487bc2d55111f2