wagtail/stylelint-config-wagtail

Shareable stylelint config for CSS and SCSS, following Wagtail’s code style.

stylelint-config-wagtail's Issues

declaration-property-value-blacklist is deprecated

• Deprecation Warning: 'declaration-property-value-blacklist' has been deprecated. Instead use 'declaration-property-value-disallowed-list'

Potential issue with sorting

I just started to resolve wagtail/wagtail#10719 and have realised that our sorting rules could be causing some issues.

When I run the stylelint formatter / fixer on the Wagtail codebase I see that it's moving all our @include mixins to the top of the selector block.

stylelint-config-wagtail/index.js

Lines 36 to 43 in b5986bb

	// Mixins should always be first in declarations
	'order/order': [
	{
	name: 'include',
	type: 'at-rule',
	},
	'declarations',
	],

From my understanding, this will actually cause a few problems where the styles will not be overridden at breakpoints.

Context of this change.

• #14
• #17

I know I merged this in but just wondering if there are some nuances for how mixins can be used that we should be allowing.

Allow all CSS system colors in stylesheets without variables usage

See wagtail/wagtail#8945

The approach to block all non-variable usage is a bit too strict - allow system colors to be used.

Introduce selector-max-combinators, selector-max-specificity & max-nesting-depth

Problem

• Currently in Wagtail there are issues with very nested CSS making it hard for other CSS to override it, this will also create problems as we add Tailwind utilities here and there.
• Stylelint provides two ways to restrict the nesting & specificity.

Details

Assessment of selector-max-combinators & selector-max-specificity on current Wagtail codebase.

level	errors
** Level 1 **	148 problems found
'selector-max-combinators': 3,	21
'selector-max-specificity': '0,3,3',	127
---------------------------------------	------------------
** Level 2 **	161 problems found
'selector-max-combinators': 3,	21
'selector-max-specificity': '0,3,2',	140
---------------------------------------	------------------
** Level 3 **	269 problems found
'selector-max-combinators': 2,	129
'selector-max-specificity': '0,3,2',	140
---------------------------------------	------------------
** Level 4 **	301 problems found
'selector-max-combinators': 2,	129
'selector-max-specificity': '0,3,1',	172
---------------------------------------	------------------

Assessment of max-nesting-depth

• Adding at level 2 only introduces 106 errors, depth 3 introduces zero errors.

Recommendation

• To have a balance between adding strictness and ensuring we do not have to rebuild all the styles;
• 'selector-max-specificity': '0,3,3'
• 'max-nesting-depth': 2'
• 'selector-max-combinators': 2'

Links

• wagtail/wagtail#7689

Enforce declarations order with stylelint

Initially mentioned in wagtail/wagtail#5246 (comment), and discussed again in a UI team meeting.

We want to use stylelint-order for this, we want to enforce mixins are first in declarations, and I think that’s about it.

Add scss/at-rule-no-unknown in this config

Based on the stylelint configuration for Wagtail. I think this should be the default directly in this package:

'scss/at-rule-no-unknown': [
      true,
      {
        ignoreAtRules: [
          'tailwind',
          'apply',
          'variants',
          'responsive',
          'screen',
          'layer',
        ],
      },
    ],

Linting rule to prevent forced-color-adjust usage

Follow-up to the Contrast Themes GSoC project. There are a number of cases where we started using forced-color-adjust in Wagtail’s stylesheets.

In all but one of those cases, the property is used incorrectly, leading to issues for end users in high-contrast mode. I believe we should disallow all usage of forced-color-adjust, as there are very few cases where it’s the correct approach (the only one being if the exact color being used is so important that it should always be used, at the expense of the legibility / contrast of the content).

Update to support Stylelint 16

https://stylelint.io/migration-guide/to-16/

Reconsider declaration-block-no-redundant-longhand-properties

Follow-up to wagtail/wagtail#10719 / wagtail/wagtail#11151 – where I’ve disabled this rule for now.

I’m not sure how much I like the idea of enforcing this rule for margin / padding properties. The main annoyance from my side is that when you want to set both properties with a similar-enough values, it’s nice to have them both side by side on the same line.

But when the values are quite complex, it would be nice to split into separate lines. Though this can be worked around with variables.

It’s also a bit annoying when you want to add one new longhand property in a situation where a shorthand would be possible – and the diff of the change becomes muddled up between "new" changes and refactorings due to the enforcement of a rule like this.

---

Not something I’d feel strongly about but I’d like to hear what others think?

stylelint config additions to consider

Follow-up to wagtail/wagtail#5246. This config is working quite well as-is, but there are a few things we discussed adding:

• Selector ordering, wagtail/wagtail#5246 (comment)
• Sass imports, pixels vs ems, specificity, class naming conventions, wagtail/wagtail#5246 (comment)
• Cross-browser support linting with https://github.com/ismay/stylelint-no-unsupported-browser-features
• Accessibility-related checks with https://github.com/YozhikM/stylelint-a11y
• This kind of "everything must be variables" enforcement: https://github.com/AndyOGo/stylelint-declaration-strict-value

Feel free to edit this comment or add more if there is more we should discuss and consider for inclusion.

---

Here is a config I worked on recently which includes some of the above, for consideration: https://github.com/torchbox/stylelint-config-torchbox.

Add a linting rule to block additional usage of `text-transform`

Adding text-transform: uppercase (or others) creates added friction for translated content.

While it may not be critical we want to ensure that our stylelint config makes it simple to make things translatable and usable.

Originally posted by @lb- in wagtail/wagtail#7624 (comment)

v1.0.0 release

Our config has been pretty stable over the last year and a half – I think it might be time for a v1.0.0 release?

We’d still be as able to make configuration changes as currently, breaking or not. The only difference in using the major version number will be:

• Communicating this is stable software, no longer experimental
• Being able to do minor upgrades without breaking changes and communicating that with the version number (in SemVer, v0.5.0 -> v0.6.0 makes no guarantee on compatibility).

Add ability to block selectors that select data attributes

Overview

• The Wagtail development documentation advises that classes should be used for styles and data attributes should be used for JS. https://docs.wagtail.org/en/latest/contributing/ui_guidelines.html#html-guidelines
• As the CSS side of this is not enforced it means that it is up to any core contributor to remember this when reviewing new code, it would be much easier if this was flagged as a linting error instead.

Proposed solution

Add 'selector-max-attribute': [0, { ignoreAttributes: ['/^(?!data-).*/'] }], - which only blocks the use of data-* attributes when applying styles to components.

All other usage should still be allowed, such as hidden, aria-hidden, disabled, type etc.

Additional details

• https://stylelint.io/user-guide/rules/list/selector-max-attribute/

Issues in Wagtail Core

The following 11 issues will be flagged in Wagtail core if this rule is applied.

• data-tabs-animate on the tabs can be resolved by using classes instead (or just drop this feature as it is unused)
• tippy styles - probably would be left as is and this rule would be ignored in the overrides folder as we have no control over these components
• Wagtail bird 'parts' can be resolved using BEM classes

client/scss/components/_tabs.scss
 69:3  ✖  Expected ".w-tabs[data-tabs-animate] .w-tabs__panel" to have no more than 0 attribute selectors             selector-max-attribute
 72:5  ✖  Expected ".w-tabs[data-tabs-animate] .w-tabs__panel.animate-in" to have no more than 0 attribute selectors  selector-max-attribute

client/scss/overrides/_vendor.tippy.scss
  8:1  ✖  Expected ".tippy-box[data-placement^='top'] > .tippy-arrow::before" to have no more than 0 attribute selectors     selector-max-attribute
 12:1  ✖  Expected ".tippy-box[data-placement^='bottom'] > .tippy-arrow::before" to have no more than 0 attribute selectors  selector-max-attribute
 16:1  ✖  Expected ".tippy-box[data-placement^='left'] > .tippy-arrow::before" to have no more than 0 attribute selectors    selector-max-attribute
 20:1  ✖  Expected ".tippy-box[data-placement^='right'] > .tippy-arrow::before" to have no more than 0 attribute selectors   selector-max-attribute
 25:1  ✖  Expected ".tippy-box[data-theme='dropdown']" to have no more than 0 attribute selectors                            selector-max-attribute
 28:3  ✖  Expected ".tippy-box[data-theme='dropdown'] .tippy-content" to have no more than 0 attribute selectors             selector-max-attribute

client/src/components/Sidebar/modules/WagtailBranding.scss
 54:7  ✖  Expected ".sidebar-wagtail-branding--wagging:hover [data-part='tail']" to have no more than 0 attribute selectors         selector-max-attribute
 60:7  ✖  Expected ".sidebar-wagtail-branding--wagging:hover [data-part='eye--open']" to have no more than 0 attribute selectors    selector-max-attribute
 64:7  ✖  Expected ".sidebar-wagtail-branding--wagging:hover [data-part='eye--closed']" to have no more than 0 attribute selectors  selector-max-attribute

Add ability to block union classes (via parent selector)

Problem

The ITCSS guide recommends that nesting be limited and that full selectors provides a more ergonomic way to scan through and search code.

https://www.xfive.co/blog/itcss-scalable-maintainable-css-architecture/

Limit nesting to 2 levels. Many developers think that using the parent selector (&) is a requirement just because they use Sass. There’s nothing wrong with flat structure with full selectors expanded, either. Often it’s easier to scan and search the code.

.c-teaser {
   padding: 2em;
}

.c-teaser--small {
  padding: 1em;
}

.c-teaser__title {
  font-size: 2em;
}

Use the style you prefer, but avoid deep nesting, which results in overqualified selectors. Those are against the spirit of ITCSS.

Details

• With the Stylelint SCSS library we can enforce no union class, this makes it easier for us to adopt less nesting in the future and makes it easier to read through and search for classes as they are in the styles.
• https://github.com/stylelint-scss/stylelint-scss/blob/master/src/rules/selector-no-union-class-name/README.md
• ❌ .w-header { &__subtitle { background-color: green; } }
• 👍 .w-header { .w-header__subtitle { background-color: green; } }

Upgrade dependencies

• stylelint to 15*
• stylelint-config-recommended to 13*
• Others

See wagtail/wagtail#10671

Add ability to enforce BEM classes with prefix

Problem

• Wagtail's code is meant to use BEM classes, however this has never been enforced and only really partially adopted.
• Enforcing this at the stylelint level will help the project align with this standard

Details

• It is unclear if this should be in the base stylelint config or as a separate plugin also exported by this package.
• It will be important to be able to set a prefix (e.g. w-header w-header--merged OR wx-header wx-header--merged ).

Links

• https://github.com/simonsmith/stylelint-selector-bem-pattern which uses https://github.com/postcss/postcss-bem-linter
• http://getbem.com/naming/
• wagtail/wagtail#7689