-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: Add support for media conditions in require-baseline rule #49
feat: Add support for media conditions in require-baseline rule #49
Conversation
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.
Thanks for the submission, this is looking really good. Just left one note about an edge case.
continue; | ||
} | ||
|
||
const conditionLevel = mediaConditions.get(child.name); |
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.
conditionLevel
can be undefined
if someone is using an unknown media condition. In that case, we should just exit early because no-invalid-at-rules
should catch it.
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.
Good point! I've fixed it.
Add check for invalid at-rules
@@ -556,6 +556,11 @@ export default { | |||
node, | |||
) { | |||
for (const child of node.children) { | |||
// ignore unknown media conditions - no-invalid-at-rules already catches this |
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 also a add a test that has an invalid media condition to verify that it's ignored?
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.
I've added.
test: ignore unknown media conditions
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. Thanks so much!
Prerequisites checklist
What is the purpose of this pull request?
This pull request adds support for media conditions in the
require-baseline
rule.What changes did you make? (Give an overview)
@media
queries.baseline-data.js
to include media conditions.Related Issues
fixes #48
Is there anything you'd like reviewers to focus on?