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
Add ignoreContextFunctionalPseudoClasses to selector-max-id #4835
Add ignoreContextFunctionalPseudoClasses to selector-max-id #4835
Conversation
these tests aren't failing properly, am I missing something?
this has the same functionality as live
Looks like the coveralls code coverage testing is failing on a |
I restarted the tests and they're all 👍 this time. Must have been a temporary gremlin. |
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.
@malsf21 Looking good, thanks!
I've requested some minor changes as suggestions. Mainly to align with our conventions, e.g.:
- the minimum amount of code possible to communicate the pattern, e.g. if the rule targets selectors then use an empty rule, e.g.
{}
Let's use Given:
and just an array for now in the example so that it's consistent with the READMEs of the other rules. There's an issue (#4808) to update these across the board to use the full config in the examples.
Mostly focused on conciseness Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Great, thank you for the feedback; batch-committed everything in the code review! |
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 making the changes.
One last suggestion, then LGTM.
Consistency w/ other READMEs (will be changed later) Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Sounds good, updated! |
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.
Excellent, thank you.
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, thank you 👍
Updated changelog:
|
Hi there,
This PR implements an option requested in #4619, the verbose
ignoreContextFunctionalPseudoClasses
toselector-max-id
. In addition to modifying the rule, it also introduces a set of simple test cases (only testing on:not
,:has
, and:matches
currently), and adds documentation to explain the usage of the rule.Closes #4619.
As mentioned in the issue discussion, it looks like
stylelint/lib/utils/isLogicalCombination.js
needs to be updated to support more pseudo-classes, such as:is
and:where
(and possibly:nth-child
); will follow up on this!