MPP-3946: Add @content to mozilla protocol text mixins #5174
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 addresses the Sass 1.80 deprecation warning about mixed declarations. It adds duplicate versions of
@mozilla-protocol/[email protected]
typography mixins to/frontend/src/styles/typography.scss
that take a content block.This code fragment raises a warning:
The warning:
This changed code eliminates the warning and results in identical CSS:
See below for more context on the issue and solution.
How to test
Similar to PR #5166, my goal was to eliminate the warning without changing the generated CSS. When reviewing, you may want to ignore whitespace, to see that the
@include text-*
contents were indented and not changed.Run
cd frontend; npm run build
./frontend/out/_next/static/css
, which means the contents are identical to the previous versions:135df0ac639034c3.css
4e63d6fb12ba45b3.css
58e1c80c0e503be3.css
644d429e5da963c9.css
7830c6fa1d166113.css
794f7edf20c99f0e.css
7b7332110014f2b1.css
bcb82ac07001d0c5.css
c34d49dcc57a01db.css
fce17def8370eee1.css
The
.map
files in the same folder, such as/frontend/out/_next/static/css/135df0ac639034c3.css.map
, have changed because the source file changed.Sass Mixed Declarations Deprecation
Our code triggers this warning with this pattern, from
/frontend/src/components/dashboard/EmailForwardingModal.modules.scss
:This uses the
text-title-xs
mixin from@mozilla-protocol/[email protected]
, with this source:https://github.com/mozilla/protocol/blob/f318aafa0f3b5ff8815c4b859d5a2de9146657f4/assets/sass/protocol/includes/mixins/_typography.scss#L100-L108
which is compiled to the (compressed) CSS:
In the future, Dart Sass will follow CSS's conventions, disallowing this form or turning it this into something like:
This may have consequences for our content. For example, the
font-weight: 100
may not be applied on desktop-sized displayed.The change is to create a new version of the
@mixin
that allows injecting our CSS properties before the@media
nested rule:These
@mixin
overrides are in/frontend/src/styles/typography.scss
, along with a long explanatory comment. This allows minimal changes to eliminate the warning for the same CSS@include "path/to/styles/typography";
after@include "~@mozilla-protocol/core/protocol/css/includes/lib";
or similar@include text-*
declarationFor example:
Future work
Relay is currently on
@mozilla-protocol/[email protected]
, well behind the current version v19.2.0. Upgrading to future versions will require copying and modifying the new typography mixins.Bedrock also has this issue (mozilla/bedrock#15117), but on
@mozilla-protocol/[email protected]
. They may solve the issue in a similar manner, find a different solution, or even create an upstream fix inmozilla-protocol/core
. We can adopt their solution and eliminate this one.An experienced CSS writer may use a different version of this fix. For example, this may be equivalent in our supported browsers:
It may better communicate the intent "Use the protocol extra-small title style, but with a different font weight". However, I am not an experienced CSS writer, so my guiding principle was "no CSS changes".