-
-
Notifications
You must be signed in to change notification settings - Fork 21
fix: var() with fallback value in no-invalid-properties
#184
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
@@ -37,6 +37,23 @@ ruleTester.run("no-invalid-properties", rule, { | |||
":root { --my-color: red; }\na { color: var( --my-color ) }", | |||
":root { --my-color: red;\n.foo { color: var(--my-color) }\n}", | |||
".fluidHeading {font-size: clamp(2.1rem, calc(7.2vw - 0.2rem), 2.5rem);}", | |||
"a { color: var(--my-color, red) }", |
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.
Can you add some invalid tests that use fallbacks with incorrect or missing values? Based on the code changes in this PR, I think the error locations will be off.
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.
Seems you are right, I think it still needs some work, like multiple var in the property and this, so marking it as draft for now. Thanks!
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.
Have updated the code to fix this and added some tests as well?
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.
LGTM, nice job. Would like another review before merging.
@Tanujkanti4441 There are few ci failures, can you please check once? |
The failures have already been fixed on |
Prerequisites checklist
What is the purpose of this pull request?
To fix the lining errors while using
var()
with fallback values.What changes did you make? (Give an overview)
Added a function to remove fallbacks when the variable value is present and a function to get a list of fallback values to use it, when there is no reference present of the variable.
So these cases are the valid:
Related Issues
fixes #180
Is there anything you'd like reviewers to focus on?