-
Notifications
You must be signed in to change notification settings - Fork 82
Remove all mixins supporting vendor prefixes (Fix #957) #1043
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
559b65b to
e535cd3
Compare
janbrasna
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.
💟 Love it!
Unfortunately I can't seem to identify the mzp-a-fade-* build issue just from glancing it, pretty sure it's all in the correct order… 🤷
Some copy/formatting nits in the meantime:
|
I'm pretty sure the |
|
Do Not Merge until Icon release is published |
| display: -webkit-box; | ||
| display: -moz-box; | ||
| display: -ms-flexbox; | ||
| display: -webkit-flex; | ||
| display: flex; | ||
| } | ||
|
|
||
| @mixin flex($values: auto) { | ||
| -webkit-box-flex: $values; | ||
| -moz-box-flex: $values; | ||
| -webkit-flex: $values; | ||
| flex: $values; | ||
| } | ||
|
|
||
| @mixin flex-direction($value: column) { | ||
| -webkit-box-direction: $value; | ||
| flex-direction: $value; | ||
| } | ||
|
|
||
| @mixin flex-wrap($value: wrap) { | ||
| -ms-flex-wrap: $value; | ||
| flex-wrap: $value; | ||
| } | ||
|
|
||
| @mixin align-items($align: stretch) { | ||
| -webkit-box-align: $align; | ||
| align-items: $align; | ||
| } | ||
|
|
||
| @mixin justify-content($justify: flex-start) { | ||
| -webkit-box-pack: $justify; |
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.
FYI these all might be necessary to remove before upgrading https://github.com/mozilla/protocol/actions/runs/17982808350/job/51153182926 — what's interesting is that new property-no-deprecated rule uncovered a handful of other occurrences directly in the components and declarations.
(I'll probably open a separate issue for visibility.)
e0578a6 to
19d2f6c
Compare
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 removes mixins that added vendor prefixes for CSS properties, as modern browser support for these properties is now universal. The change affects numerous components and base styles across the Protocol design system.
Key Changes:
- Removed all vendor-prefix utility mixins from
_utils.scss(transition, transform, flexbox, border-box, appearance, etc.) - Replaced mixin calls throughout components with standard CSS properties
- Updated changelog with migration guidance and list of affected mixins
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
assets/sass/protocol/includes/mixins/_utils.scss |
Removed vendor-prefix mixins (transition, transform, flexbox, border-box, appearance, animation, etc.) |
assets/sass/protocol/includes/mixins/_grid.scss |
Replaced border-box mixin calls with standard CSS property |
assets/sass/protocol/includes/mixins/_details.scss |
Replaced background-size, transition, and transform mixin calls |
assets/sass/protocol/includes/forms/_index.scss |
Replaced border-box mixin calls with standard property |
assets/sass/protocol/components/forms/_choice.scss |
Replaced border-box mixin with standard property |
assets/sass/protocol/components/_sticky-promo.scss |
Replaced animation and background-size mixins |
assets/sass/protocol/components/_split.scss |
Replaced border-box mixin calls |
assets/sass/protocol/components/_sidebar-menu.scss |
Replaced transform mixin call |
assets/sass/protocol/components/_picto.scss |
Replaced border-box mixin call |
assets/sass/protocol/components/_notification-bar.scss |
Replaced border-box mixin call |
assets/sass/protocol/components/_navigation.scss |
Replaced transition, transform, and background-size mixin calls |
assets/sass/protocol/components/_modal.scss |
Replaced animation, background-size, transition, and transform mixin calls |
assets/sass/protocol/components/_menu.scss |
Replaced border-box, transition, transform, and animation mixin calls |
assets/sass/protocol/components/_menu-list.scss |
Replaced background-size, transition, and transform mixin calls |
assets/sass/protocol/components/_menu-item.scss |
Replaced transition mixin calls |
assets/sass/protocol/components/_footer.scss |
Replaced border-box, background-size, transition, and transform mixin calls |
assets/sass/protocol/components/_feature-card.scss |
Replaced grid-column-gap and flexbox mixin calls |
assets/sass/protocol/components/_card.scss |
Replaced border-box and transition mixin calls |
assets/sass/protocol/components/_callout.scss |
Replaced grid-column-gap and flexbox mixin calls |
assets/sass/protocol/components/_button.scss |
Replaced border-box and transition mixin calls |
assets/sass/protocol/components/_billboard.scss |
Replaced border-box, align-items, flexbox, and justify-content mixin calls |
assets/sass/protocol/base/elements/_forms.scss |
Replaced appearance and border-box mixin calls |
CHANGELOG.md |
Added changelog entry with migration guidance and list of affected mixins |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I ran some visual regression tests against this branch locally and no differences were found in Chrome. |
This version removes mixins which added vendor-prefixes. Browser support for these is now excellent. - One migration path is to edit your code to use the unprefixed versions. - Another option is to move these utility mixins into your own code base (though, be aware they are no longer used in Protocol and this will not give you backwards compatible Protocol components). - If you need that level of vendor prefix support consider adding a tool such as [autoprefixer](https://github.com/postcss/autoprefixer) to your code base. - Effected mixins are: - `animation` - `appearance` - `background-size` - `box-decoration-break` - `box-sizing` - `flexbox`, `flex`, `flex-direction`, `flex-wrap`, `align-itmes`, `justify-content,` - `transform`, `transform-origin`, `transform-style` - `transition`, `transition-property`, `transition-duration`, `transition-delay` - Disable no-unknown-animation rule on animations
e9a7ea7 to
921576a
Compare
Description
This version removes mixins which added vendor-prefixes. Browser support for these is now excellent.
CHANGELOG.md.Issue
Fix #957
Testing
View the affected components to make sure they are still displaying properly.