Skip to content
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

[FIX] cssVariables: Don't use CSS vars within @font-face directives #257

Merged
merged 2 commits into from
Dec 1, 2022

Conversation

matz3
Copy link
Member

@matz3 matz3 commented Nov 30, 2022

Browsers do not seem to support usage of custom CSS properties
(aka CSS variables) within @font-face directives.

The skeleton style-sheet should therefore not contain a reference to the
variables but just use the actual value provided by the LESS variable.

BCP: 2270189546

Browsers do not seem to support usage of custom CSS properties
(aka CSS variables) within @font-face directives.

The skeleton style-sheet should therefore not contain a reference to the
variables but just use the actual value provided by the LESS variable.

BCP: 2270189546
@@ -24,6 +25,10 @@ CSSVariablesCollectorPlugin.prototype = {
return this.mixinStack.length > 0 || this.parenStack.length > 0;
},

_isInFontFaceDirective() {
return this.directiveStack.some(({name}) => name === "@font-face");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be faster to have a dedicated "font-face-directive" stack and check for it's length instead of searching the whole array of directives for every rule? Are directives a rare thing to have in the stack?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense.

@matz3 matz3 requested a review from RandomByte December 1, 2022 10:41
@RandomByte
Copy link
Member

LGTM

@matz3 matz3 merged commit a8dfd17 into master Dec 1, 2022
@matz3 matz3 deleted the fix-cssVariables-font-face-var branch December 1, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants