-
Notifications
You must be signed in to change notification settings - Fork 82
Refactor typography system to use CSS custom properties #1107
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR refactors the typography system to use CSS custom properties instead of Sass variables, improving maintainability and enabling responsive typography through theme-based CSS variables. The changes include renaming variables with a --token- prefix for consistency, removing deprecated mixins, and eliminating pixel-based font size fallbacks.
- Introduces
--theme-*CSS variables that adjust based on media queries for responsive typography - Refactors typography mixins to use CSS custom properties and removes pixel fallbacks
- Updates all component styles to use the new variable naming convention
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| assets/sass/protocol/includes/mixins/_typography.scss | Core typography mixin refactoring to use CSS variables and remove deprecated mixins |
| assets/sass/protocol/includes/themes/_mozilla.scss | Mozilla theme updates with --token- prefix for all typography variables |
| assets/sass/protocol/includes/themes/_firefox.scss | Firefox theme updates with --token- prefix for all typography variables |
| assets/sass/protocol/includes/themes/_index.scss | Introduces --theme-* variables that map to token variables for responsive behavior |
| assets/sass/protocol/includes/_themes-sass.scss | Removes Sass variable definitions for fonts and type scale, now handled by CSS variables |
| Multiple component files | Updates to use new --token-* variable naming throughout the codebase |
| CHANGELOG.md | Documents breaking changes and migration guide |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
08bea80 to
5f6d322
Compare
|
No visual regressions found running #1106 locally. |
| // This file contains all the theme variables for both the Firefox and Mozilla | ||
| // Mixins and functions can draw from these variable maps, swapping to | ||
| // a different map based on the value of the global $brand-theme variable. | ||
| // Default theme variables for legacy browsers (IE 11 etc) |
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.
Note for reviewer: I'm working on removing these. Colour and grid changes will follow.
1599e07 to
0ec134f
Compare
| line-height: $text-body-line-height; | ||
| margin: 0; | ||
| padding-bottom: $label-v-spacing; | ||
| @include text-body-sm; |
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.
Note for reviewer: Line-height is included in mixin now.
ed88390 to
19ab519
Compare
Refactor typography system to use CSS custom properties. This is the first PR in a refactor of how we use CSS variables (#982) and renaming title to heading (#668). Some kind of basic fallback support for older browsers will be added back in a latter PR once we can get a clear picture of how broken things are. I know this looks big but for the most part it's just a straight find/replace on variable names or moving @includes to the end of declarations. Modernization: Default to CSS vars for font family, size, and line-height Removed @supports declarations if they only had font declarations @include text-body-* and @include text-heading-* mixins use CSS vars now Edited @mixin font-size() to stop outputting a pixel fallback Reorganization: Renamed -title- to -heading- in mixins and CSS vars and mzp-u-heading-* utility classes component HTML/CSS classes will follow in a separate PR Added font-family declaration to @include text-body-* mixins Added --theme- prefix to variables expected to morph Added --token- prefix to unchanging variables Re-name font-size tokens to use a scale instead of tshirt sizes for names Added --theme-button-line-height var Bug fixes: Moved @include text-* to end of declarations
19ab519 to
b3d5a33
Compare
Description
Refactor typography system to use CSS custom properties. This is the first PR in a refactor of how we use CSS variables (#982) and renaming
titletoheading(#668). Some kind of basic fallback support for older browsers will be added back in a latter PR once we can get a clear picture of how broken things are.I know this looks big but for the most part it's just a straight find/replace on variable names or moving
@includesto the end of declarations.@supportsdeclarations if they only had font declarations@include text-body-*and@include text-heading-*mixins use CSS vars now@mixin font-size()to stop outputting a pixel fallback-title-to-heading-in mixins and CSS varsmzp-u-heading-*utility classes@include text-body-*mixins--theme-prefix to variables expected to morph--token-prefix to unchanging variables--theme-button-line-heightvar@include text-*to end of declarationsIssue
#982
Testing
CHANGELOG.md.