Problem/Motivation

Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the declaration-block-no-shorthand-property-overrides 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
#11 2866806-11.patch2.03 KBharsha012
#9 2866806-9.patch3.1 KBharsha012
#6 2866806-6.patch3.13 KBharsha012
#3 declaration-block-no-shorthand-property-overrides-2866806-3.patch3.13 KBmukeysh

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: #2866805: Update stylelint rule declaration-block-no-redundant-longhand-properties 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 declaration-block-no-shorthand-property-overrides-2866806-3.patch3.13 KB

Added patch for this.

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
Status: Needs review » Needs work

Patch doesn't apply EDIT: with fuzz but could use a reroll and there is one more thing that snuck in:

Checking patch core/themes/bartik/css/components/node-preview.css...
error: while searching for:
  padding: 10px;
}
.node-preview-backlink {
  background-color: #419ff1;
  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, -webkit-linear-gradient(top, #419ff1, #1076d5);
  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, linear-gradient(to bottom, #419ff1, #1076d5); /* LTR */
  border: 1px solid #0048c8;
  border-radius: .4em;
  box-shadow: inset 0 1px 0 rgba(255, 255, 255, .4);

error: patch failed: core/themes/bartik/css/components/node-preview.css:10
error: core/themes/bartik/css/components/node-preview.css: patch does not apply
harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866806-6.patch3.13 KB

fixed issue.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Version: 8.4.x-dev » 8.5.x-dev
Issue tags: +Needs reroll

This needs a reroll against 8.5.x, I'm still seeing some of the changes in #5 and it won't apply.

Checking patch core/.stylelintrc.json...
error: while searching for:
    "comment-empty-line-before": null,
    "declaration-block-no-duplicate-properties": null,
    "declaration-block-no-redundant-longhand-properties": null,
    "declaration-block-no-shorthand-property-overrides": null,
    "declaration-block-trailing-semicolon": null,
    "declaration-colon-space-after": null,
    "function-comma-space-after": null,

error: patch failed: core/.stylelintrc.json:11
error: core/.stylelintrc.json: patch does not apply
Checking patch core/themes/bartik/css/components/node-preview.css...
error: while searching for:
  padding: 10px;
}
.node-preview-backlink {
  background-color: #419ff1;
  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, -webkit-linear-gradient(top, #419ff1, #1076d5);
  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, linear-gradient(to bottom, #419ff1, #1076d5); /* LTR */
  border: 1px solid #0048c8;
  border-radius: .4em;
  box-shadow: inset 0 1px 0 rgba(255, 255, 255, .4);

error: patch failed: core/themes/bartik/css/components/node-preview.css:10
error: core/themes/bartik/css/components/node-preview.css: patch does not apply
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work
harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866806-9.patch3.1 KB

fixed the issue.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work
Issue tags: -Needs reroll

Very close!

  1. +++ b/core/.stylelintrc.json
@@ -8,7 +8,7 @@
-    "declaration-block-no-shorthand-property-overrides": null,
+    "declaration-block-no-shorthand-property-overrides": true,

    This is default so we can just remove the null. 

  2. +++ b/core/themes/bartik/css/components/node-preview.css
@@ -10,9 +10,8 @@
-  background-color: #419ff1;
-  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, -webkit-linear-gradient(top, #419ff1, #1076d5);
-  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, linear-gradient(to bottom, #419ff1, #1076d5); /* LTR */
+  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, -webkit-linear-gradient(top, #419ff1, #1076d5) #419ff1;
+  background: url(../../../../misc/icons/000000/chevron-left.svg) left no-repeat, linear-gradient(to bottom, #419ff1, #1076d5) #419ff1; /* LTR */

    The background-color values should just be removed because it is overwritten by the shorthand after it.

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866806-11.patch2.03 KB

fixed as per #10

joelpittet’s picture

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

Thats the ticket! Thanks @harsha012, @Mukeysh and @alexpott!

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

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

Committed and pushed 19426fd74a to 8.6.x and 2deb0f1d08 to 8.5.x. Thanks!

  • alexpott committed 19426fd on 8.6.x 
    Issue #2866806 by harsha012, Mukeysh, joelpittet: Update stylelint rule...

  • alexpott committed 2deb0f1 on 8.5.x 
    Issue #2866806 by harsha012, Mukeysh, joelpittet: Update stylelint rule...

Status: Fixed » Closed (fixed)

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