Problem/Motivation

Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the selector-type-no-unknown to be consistent with https://www.drupal.org/docs/develop/standards/css/css-coding-standards

Proposed resolution

Brief instructions on running stylelint - you'll need npm...

All the commands below take place in DRUPAL_ROOT/core
To install stylelint

npm install

This will install Drupal 8's npm dependencies of which stylelint is one.

To run it on all core css files. Apply this issue's patch and do the following command from DRUPAL_ROOT/core

npm run lint:css

Remaining tasks

User interface changes

None

API changes

None

CommentFileSizeAuthor
#12 interdiff-2866823-10-12.txt428 bytesharsha012
#12 2866823-12.patch1.8 KBharsha012
#10 interdiff-2866823-6-10.txt826 bytesharsha012
#10 2866823-10.patch1.84 KBharsha012
#6 2866823-6.patch1.77 KBharsha012
#3 selector-type-no-unknown-2866823-3.patch500 bytesmukeysh

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: #2866822: Update stylelint rule selector-pseudo-element-colon-notation to be consistent with Drupal's CSS standards » #2865971: Use stylelint as opposed to csslint in core
mukeysh’s picture

mukeysh commented
Status: Active » Needs review
StatusFileSize
new selector-type-no-unknown-2866823-3.patch500 bytes

made "selector-type-no-unknown": true,

Only found 2 issues with this
themes/seven/css/base/elements.css
124:1 × Unexpected unknown type selector "nobr" selector-type-no-unknown
153:1 × Unexpected unknown type selector "quote" selector-type-no-unknown

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Version: 8.4.x-dev » 8.5.x-dev
Status: Needs review » Needs work

The issues don't seem to be fixed in the patch.

misc/dialog/off-canvas.base.css
 129:20  ✖  Unexpected unknown type selector "nobr"   selector-type-no-unknown

themes/seven/css/base/elements.css
 124:1  ✖  Unexpected unknown type selector "nobr"    selector-type-no-unknown
 153:1  ✖  Unexpected unknown type selector "quote"   selector-type-no-unknown

themes/stable/css/core/dialog/off-canvas.base.css
 129:20  ✖  Unexpected unknown type selector "nobr"   selector-type-no-unknown
harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866823-6.patch1.77 KB

fixed the as per #5

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Thanks @harsha012 for getting the remainder and @Mukeysh and @alexpott for starting this.

I've applied the patch, tested with and without the changes to see they were caught. Also checked that nobody was trying to use a <nobr> tag in core.

Everything is wonderful. 👌

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/misc/dialog/off-canvas.base.css
@@ -125,7 +125,7 @@
-#drupal-off-canvas nobr {
+#drupal-off-canvas {
   white-space: nowrap;
 }

Shouldn't we just be removing the rule? This is a change to what's applying to the #drupal-off-canvas styles.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Oh I didn't catch that, good eye both of those should be removed!

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866823-10.patch1.84 KB
new interdiff-2866823-6-10.txt826 bytes

fixed as per comment #8

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work 
+++ b/core/.stylelintrc.json
@@ -31,7 +31,7 @@
-    "selector-type-no-unknown": null,
+    "selector-type-no-unknown": true,

If the default is true let's just remove the line here.

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866823-12.patch1.8 KB
new interdiff-2866823-10-12.txt428 bytes
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Thanks @harsha012 and @alexpott. I tested by running the linter with the CSS changes and without them to see that they were being tested.
And looked at the patch to make sure they changes are removing the fake tag selectors

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

https://caniuse.com/#search=nobr - so no browser supports nobr and quote has never been an html element.

Committed 60a2dc0 and pushed to 8.5.x. Thanks!

  • alexpott committed 60a2dc0 on 8.5.x 
    Issue #2866823 by harsha012, Mukeysh, joelpittet: Update stylelint rule...

Status: Fixed » Closed (fixed)

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