[Feature:Developer] Add CSS color linter rules#12475
[Feature:Developer] Add CSS color linter rules#12475williamjallen merged 4 commits intoSubmitty:mainfrom
Conversation
Added color-named: never and color-no-hex: true rules to enforce usage of CSS variables from colors.css instead of hardcoded colors. Legacy files that currently violate these rules are temporarily exempted via an overrides block and will be addressed in separate PRs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12475 +/- ##
============================================
- Coverage 21.70% 21.66% -0.05%
- Complexity 9626 9638 +12
============================================
Files 268 268
Lines 36188 36233 +45
Branches 486 486
============================================
- Hits 7856 7849 -7
- Misses 27850 27902 +52
Partials 482 482
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@williamjallen I've opened a PR to address this: #12475 It adds the |
JManion32
left a comment
There was a problem hiding this comment.
This change would be helpful for enforcing our CSS conventions, but I'm not a fan of overriding the rule in certain files (except for colors.css, that makes sense).
In the past, we've added inline ignores for existing violations. This allows us to enforce the new rule for all future PRs, and the inline ignores make it easier to complete the refactor in the future.
Would it be feasible to add inline ignores for each violation, or possibly find a way to get the count of existing violations then add a check to ensure new code doesn't increase this count?
Removed legacy CSS files from the Stylelint overrides array, replacing them with inline /* stylelint-disable-next-line */ comments above each violating line as requested by the reviewer.
|
@JManion32 Thanks for the feedback I completely agree — inline ignores are much safer and will prevent new violations from slipping into those files. I've updated the PR to remove the global |
williamjallen
left a comment
There was a problem hiding this comment.
@Akarshkushwaha Please see failing CI. It looks like you need to specify multiple rules in a few cases instead of listing multiple single-rule next-line ignores. See the docs here for more details/
Combined multiple adjacent '/* stylelint-disable-next-line */' comments into a single comma-separated list to resolve 'Unexpected !important declaration-no-important' errors thrown by the CI pipeline.
Why is this Change Important & Necessary?
Closes #11224
One of the most common review comments for CSS files is a reminder that colors from
colors.cssare preferred over hardcoded colors. This PR automates that check by adding Stylelint rules to prohibit hardcoded CSS colors.What is the New Behavior?
Added
color-named: "never"andcolor-no-hex: truerules to [site/.stylelintrc.json]. These rules will now flag any new CSS code that uses hardcoded color names (e.g.,red,blue) or hex codes (e.g.,#FF0000), enforcing usage of CSS variables fromcolors.css.Legacy files that currently violate these rules are temporarily exempted via an
overridesblock in the config. These will be addressed in separate follow-up PRs.What steps should a reviewer take to reproduce or test the bug or new feature?
npm run css-stylelintin thesite/directory — it should pass.color: red;orcolor: #FF0000;to any non-exempted CSS file and runnpm run css-stylelintagain — it should fail with acolor-namedorcolor-no-hexerror.Automated Testing & Documentation
The Stylelint CI check will automatically enforce these rules on all future PRs. No additional automated tests are needed.
Other information
This is not a breaking change. No migrations are needed. The 20 legacy CSS files that currently use hardcoded colors are listed in the
overridesblock and will be cleaned up in subsequent PRs.