-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Adding BEM naming convention to the stylelint rules #28954
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +23 kB (+2%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
@gziolo the checks have failed since the rule is now applied in the linter. What should I do in this case, disable the rule and keep only the regex change, or change the CSS rules as well? 😅 |
packages/stylelint-config/index.js
Outdated
@@ -98,7 +98,7 @@ module.exports = { | |||
'selector-attribute-operator-space-before': 'never', | |||
'selector-attribute-quotes': 'always', | |||
'selector-class-pattern': [ | |||
'^([a-z][a-z0-9]*)(-[a-z0-9]+)*$', | |||
'^([a-z][a-z0-9]*)(-[a-z0-9]+)*((__([a-z][a-z0-9]*)(-[a-z0-9]+)*)?(--([a-z][a-z0-9]*)(-[a-z0-9]+)*)?)$', |
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 suggest to also set the resolveNestedSelectors
option and the message needs an update too. See this example for inspiration.
Nice one, the number of violations reported is so long 😅
We definitely should ignore those violations for React Native files ( |
@gziolo Gotcha. There's probably an option... I tried it on #28988. |
@gziolo Gotcha. There's probably an option... I tried it on #28988.
I commented on the PR and asked the mobile team to share their thoughts on the approach proposed. Great idea to automate it, it would be too much hassle otherwise 😄
It sounds promising. We should also disable this rule in https://github.com/WordPress/gutenberg/blob/master/packages/components/src/date-time/datepicker.scss since we forked https://github.com/airbnb/react-dates so those class names are completely different. With that, the number of violations should be way smaller. |
* Scope selector-class-pattern stylelint rule to web stylesheets Native stylesheets currently do not adhere to the same BEM selector patterns as web stylesheets. This introduces a custom stylelint plugin to exclude native stylesheets from the selector-class-pattern rule. * Fixing unit tests to use correct base directory * Append native suffix to avoid web-specific lint rules All stylesheet files that do not have the native suffix are linted with web-specific selector rules. Adding the suffix disables those irrelevant rules for the native stylesheet. Co-authored-by: Rafael Galani <galani.rafael@gmail.com>
Hi, @rafaelgalani The original issue came up during Weekly Editor Bug Scrub. I just wanted to check if you've time to continue working on this PR or how I can help to move this forward. Thanks |
It would be great to move forward with this PR, just saying 😄 |
Description
Fixes #28616.
It's basically an increment to a
stylelint
rule validation regex, extending it to also allow BEM naming convention (block__element--modifier
).How has this been tested?
npx stylelint **/*.scss
and making sure tests wouldn't break.Types of changes
Bugfix - I guess. Since the validation was disabled...
I'm not sure how to categorize it.
Checklist: