Drupal depends on stylelint-config-standard 23.0.0. However, the latest version is 28.0.0: https://www.npmjs.com/package/stylelint-config-standard. Release notes: https://github.com/stylelint/stylelint-config-standard/blob/main/CHANGEL...

Release notes snippet

stylelint-config-standard, which enforces Drupal's CSS style, has been upgraded from 23.0.0 to 28.0.0. Developers who relied on this ruleset may have to make minor tweaks to their CSS to comply with the new standards.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii’s picture

Status: Active » Needs review
FileSize
8.1 KB

Had to make couple of changes to CSS to make it comply with the new stylelint rules. The alpha function usage removal is the only functional change but that can be removed because it was only there for < IE 7 support.

Status: Needs review » Needs work

The last submitted patch, 3: 3308821-2.patch, failed testing. View results

lauriii’s picture

Status: Needs work » Needs review
nod_’s picture

Status: Needs review » Reviewed & tested by the community

all good, lint still works, no change in the build.

bnjmnm’s picture

  • bnjmnm committed 5e8e5e8 on 10.1.x
    Issue #3308821 by lauriii, nod_: Update stylelint-config-standard to 28....
longwave’s picture

Status: Reviewed & tested by the community » Needs work

9.5.x patch needs work for Seven:

themes/seven/css/components/jquery.ui/theme.css
  46:11  ✖  Unexpected unknown function "alpha"  function-no-unknown
  51:11  ✖  Unexpected unknown function "alpha"  function-no-unknown
 334:11  ✖  Unexpected unknown function "alpha"  function-no-unknown

  • bnjmnm committed bd2a6b9 on 10.0.x
    Issue #3308821 by lauriii, nod_: Update stylelint-config-standard to 28....
bnjmnm’s picture

Version: 10.0.x-dev » 9.5.x-dev
Status: Needs work » Patch (to be ported)

Committed to 10.1.x and cherry picked to 10.0.x. As mentioned in #9, a little more work is needed to port to 9.5.x

bnjmnm’s picture

Status: Patch (to be ported) » Needs review
FileSize
5.62 KB
655 bytes

The alpha() use is not needed for any browser currently supported by Drupal 9, so it can be removed here like it was in 10.

nod_’s picture

+++ b/core/themes/claro/css/components/details.pcss.css
@@ -318,11 +318,11 @@
+[open] > .claro-details__summary--accordion:not(:focus, :active)::after,
+[open] > .claro-details__summary--accordion-item:not(:focus, :active)::after,

That doesn't work in IE11 though https://caniuse.com/css-not-sel-list

longwave’s picture

Status: Needs review » Needs work

NW for #13.

Is there a CSS linter that can take browserslist config and tell us whether we are using unsupported CSS? (instead of autoprefixer, which would try to fix it for us)

longwave’s picture

To answer my own question with 5 seconds of Google: should we consider adding https://github.com/ismay/stylelint-no-unsupported-browser-features to stylelint?

bnjmnm’s picture

Status: Needs work » Needs review
FileSize
5.68 KB
1.92 KB

Good catch @nod_! I was focusing on the filter calls and not the other changes. That definitely won't fly in IE11.

lauriii’s picture

Status: Needs review » Needs work
+++ b/core/themes/claro/css/components/details.css
@@ -332,6 +332,8 @@ td .claro-details {
+/* stylelint-disable selector-not-notation */

I think we would have to change this rule globally because it's not a great experience to have stylelint say something that we can't commit.

bnjmnm’s picture

Status: Needs work » Needs review
FileSize
4.84 KB
1.77 KB
longwave’s picture

I was about to say that it looks like this needs reverting as well:

+++ b/core/themes/olivero/css/components/comments.pcss.css
@@ -183,7 +183,7 @@
-  & > .comment:not(:last-of-type):not(.has-children):before {
+  & > .comment:not(:last-of-type, .has-children):before {

But is PostCSS translating this to the correct syntax for us? In the built CSS this is present as:

.indented > .comment:not(:last-of-type):not(.has-children):before {

Similarly in the interdiff in #16 note that the .pcss.css file changed, but the selectors in the .css file didn't change.

So if this is true, can we in fact use :not() with a selector list and PostCSS will translate it for us?

bnjmnm’s picture

Good catch @longwave!

longwave’s picture

Status: Needs review » Reviewed & tested by the community
lauriii’s picture

Status: Reviewed & tested by the community » Needs work

#19 is true for .pcss.css files but Stylelint applies on .css files too. I think we should still change the rule for 9.5.x to ensure that we don't accidentally add CSS that is not compatible with IE11.

bnjmnm’s picture

Status: Needs work » Needs review
FileSize
4.21 KB

I didn't set things up to get a nice interdiff, but the patch itself is likely easier to read as a whole vs the diff anyway.

longwave’s picture

Status: Needs review » Needs work

Huh, we are already using the advanced syntax!

themes/seven/css/components/form.css
 14:6  ✖  Expected simple :not() pseudo-class notation  selector-not-notation

The line in question:

input:not([type="file"],.form-text, .form-textarea) {
bnjmnm’s picture

Status: Needs work » Needs review
FileSize
4.68 KB
477 bytes

Apparently I missed that running commit-code-check locally. Glad to see stylelint doing its job 😎

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! RTBC assuming bot agrees.

  • lauriii committed 2a26adc on 9.5.x
    Issue #3308821 by bnjmnm, lauriii, longwave, nod_: Update stylelint-...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 2a26adc and pushed to 9.5.x. Thanks!

longwave’s picture

Issue summary: View changes
Issue tags: +9.5.0 release notes, +10.0.0 release notes

Added a release note snippet.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.