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

Add selector-not-notation #5974

Closed
Mouvedia opened this issue Mar 17, 2022 · 8 comments
Closed

Add selector-not-notation #5974

Mouvedia opened this issue Mar 17, 2022 · 8 comments
Labels
status: wip is being worked on by someone type: new rule an entirely new rule

Comments

@Mouvedia
Copy link
Contributor

Mouvedia commented Mar 17, 2022

check #5974 (comment)


What is the problem you're trying to solve?

prompted by #5575 (comment)

What solution would you like to see?

One rule that fix it for ::slotted() which would have one option for :not() (disabled by default*).
Someone will have to check whether there are other functions that require simple selectors.

* because the level 4 :not() is not affected

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Mar 17, 2022
@jeddy3
Copy link
Member

jeddy3 commented Mar 17, 2022

Thanks for opening a new issue.

Am I right in thinking that ::slotted can only accept simple selectors, whereas :not() can accept a simple selector or s selector list depending on what browsers a user is targeting?

If so, we can add a rule to our enforce conventions category that'll let the user enforce their preference. It would be similar to the selector-pseudo-element-colon-notation rule, e.g. selector-not-notation: "simple"|"list".

Where "simple" will warn for:

:not(div, .foo) {}

And accept:

:not(div):not(.foo) {}

And "list" would do the opposite.

@Mouvedia
Copy link
Contributor Author

Mouvedia commented Mar 17, 2022

Am I right…

You are.

we can add a rule to our enforce conventions category that'll let the user enforce their preference

If you think that it should be its own separate rule, Ill comply.
Feel free to rename this issue's title.

selector-not-notation: "simple"|"list"

  1. Would "simple" reject :not(a.b[c]) as well? If "simple" corresponds to level 3 then it should.
  2. Is a.b[c] considered a list?

I think "simple"|"complex" would be better.
ref https://developer.mozilla.org/en-US/docs/Web/CSS/:not#syntax (uses complex)

@jeddy3
Copy link
Member

jeddy3 commented Mar 18, 2022

I think "simple"|"complex" would be better.

SGTM.

  • Name: selector-not-notation
  • Primary option: "simple"|"complex"
  • Secondary options: None
  • Autofixable: Yes, but can be only for "complex" to help people modernise their code (see color-function-notation for precedence)
  • Message: Expected ${primary} :not() pseudo-class notation"
  • Description: "Specify complex or simple notation for the :not() pseudo-class"
  • Extended description: (Something about level 3 vs level 4, and fix option. Also see color-function-notation)
  • Section: "Enforce conventions" -> "Selectors"

@jeddy3 jeddy3 added status: wip is being worked on by someone type: new rule an entirely new rule and removed status: needs discussion triage needs further discussion labels Mar 18, 2022
@jeddy3 jeddy3 changed the title Add function-simple-selector or selector-pseudo-simple-selector Add selector-not-notation Mar 18, 2022
@Mouvedia
Copy link
Contributor Author

Mouvedia commented Mar 18, 2022

Autofixable: Yes

It's my first rule, I don't plan to make it fixable in that PR.

but only for "complex" (see color-function-notation for precedence)

there's one test that I considered unfixable for "simple"
the rest is doable

Description
Extended description

Ill need help for that.

@jeddy3
Copy link
Member

jeddy3 commented Mar 18, 2022

It's my first rule, I don't plan to make it fixable in that PR.

That's fine, we can open a follow-up issue.

Ill need help for that.

Sure thing. I'll add something to the pull request.

@jeddy3
Copy link
Member

jeddy3 commented Mar 27, 2022

Added in #5975

@jeddy3 jeddy3 closed this as completed Mar 27, 2022
@carlosjeurissen
Copy link
Contributor

The fix method is a non-safe method as it changes specificity. So not sure including this in the general fix method is a good idea.

@jeddy3
Copy link
Member

jeddy3 commented Apr 19, 2022

@carlosjeurissen Can you open a documentation issue, please? We can add a note to rule's README that the fix may change the specificity.

So not sure including this in the general fix method is a good idea.

Rules can be turned on and off individually. Quite a few other rules come with caveats regarding their autofix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: new rule an entirely new rule
Development

No branches or pull requests

3 participants