-
Notifications
You must be signed in to change notification settings - Fork 148
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
Expressions are not being parsed #388
Comments
I’m sorry, is this a new CSS feature that I don’t know about yet? |
Not really. I believe I oversimplified the example 😃 Consider this: div {
background: green;
width: calc((100vw - 100px) / 2);
height: 100px;
} Without this fix the value in |
I encountered a similar issue in the AMP plugin, which uses this library. ampproject/amp-wp#7291 Example: :where(.container) {
width: min(var(--container-max-width), 100% - calc(var(--container-padding) * 2));
margin-inline: auto;
} |
On the basis of the examples here, the issue seems to be with the nested brackets. Either with The OP example is definitely new to me. I don't know if it's allowed for the |
For me, it seems to be that the use of arithmetic causes issues: .heading {
font-size: clamp(1.4rem, 3vw + 1rem, 2rem);
} Whereas this example gets parsed without a problem: .heading {
font-size: clamp(1.4rem, 3vw, 2rem);
} This approach for using |
Parentheses in math-functions are an implicit calc as far as I understand, e.g. https://drafts.csswg.org/css-values/#funcdef-calc
And this is also valid for regular math function and their parameters (and you don't need extra parentheses, either), e.g. https://drafts.csswg.org/css-values/#example-67532822
You can write something like |
Yes, that seems to be the case. However,
I think this is saying only the width: calc(100% / 3);
height: calc(calc(100% / 3) / calc(16 / 9)); Or, in other words,
would be invalid. It needs an outer So the example in the OP is invalid, and should be dropped (even if it is more correctly I've made a new year resolution to move forward to resolve this and related issues... HNY. |
Yeah, I agree that the original example is invalid and should be discarded. With the
Lots of great ways to produce bugs that are hard to track down that I'm totally not going to add our candidate tests. |
The current implementation has a lenient/strict mode parsing option. The latter (re-)throws exceptions upon invalid constructs, whereas the former silently drops them. Moving forward, the plan is to replicate browser behaviour as much as possible - i.e. operate in lenient mode; remove the strict mode option, and instead accept a
Indeed.
Absolutely. We don't care for the meaning, just the syntax. @jhard, we would welcome any PRs that would go some way to resolving some of these issues. There are a few open and outstanding that may be used for reference, inc;uding this. The code base has moved on, so it may be difficult to rebase the existing PRs. |
Simple expressions do not seem to be parsed. Example:
This ends up being treated as:
div {}
The text was updated successfully, but these errors were encountered: