Patch #32888

Use stylelint to avoid errors and enforce conventions in CSS files

Added by Marius BALTEANU about 1 month ago. Updated 9 days ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Code cleanup/refactoring
Target version:4.2.0

Description

In the last few months we had some cases when we added/fixed broken CSS code (ex: r18173, r18144, #32838). These types of errors can be easily avoid if we use a lint tool for CSS (as we started using Rubocop for Ruby).

My proposal is to add Stylelint which is a very well know library, actively maintained, with good community and integrated by many tools.

Attached is a patch that:
  1. Adds package.json and yarn.lock[1] to request the library as development dependency. To install and run it, I propose to use Yarn.
  2. Adds .stylelintrc with the initial list of rules from here. For now, I propose to start only with the possible-errors rules.
  3. Adds .stylelintignore to ignore some files from linting (jquery-ui-*.ccs for now).
  4. Updates the documentation
Beside this, if the overall idea is accepted, I have in plan to add the following:
  • Add more rules to enforce some coding standards in CSS files
  • Add a Javascript lint tool like ESLint
  • Use package.json to handle the version of the current JS libraries (ex: jQuery, jQuery UI, Chart, Raphael, Tribute, etc)
  • Give a shoot to integrate Webpack as assets pipeline (In Rails 6, is default)

Any feedback is really appreciated.

1 I prefer to have yarn.lock committed in the repository in order to enforce the same version between Redmine installations.

0001-Adds-Stylelint-to-lint-CSS-files.patch Magnifier (87 KB) Marius BALTEANU, 2020-01-26 21:04


Related issues

Related to Redmine - Patch #32890: Fix violations reported by Stylelint Closed

History

#1 Updated by Marius BALTEANU about 1 month ago

  • File 0001-Adds-Stylelint-to-lint-CSS-files.patch added

#2 Updated by Marius BALTEANU about 1 month ago

Running Stylelint over the existing CSS code from public folder, returns the following violations:

root@df2fd1389bf4:/work# node_modules/.bin/stylelint "public/stylesheets/**/*.css" 

public/stylesheets/application.css
  561:1   ✖  Unexpected duplicate selector "div#issue-changesets div.changeset", first used at line 560   no-duplicate-selectors                   
  682:77  ✖  Unexpected duplicate "white-space"                                                           declaration-block-no-duplicate-properties
 1084:1   ✖  Unexpected duplicate selector "table.progress", first used at line 1075                      no-duplicate-selectors                   
 1560:1   ✖  Unexpected duplicate selector ".sort-handle", first used at line 706                         no-duplicate-selectors                   
 1619:83  ✖  Unexpected duplicate "color"                                                                 declaration-block-no-duplicate-properties

public/stylesheets/responsive.css
 137:5   ✖  Unexpected duplicate "height"                                                                                                           declaration-block-no-duplicate-properties    
 264:18  ✖  Unexpected missing generic font family                                                                                                  font-family-no-missing-generic-family-keyword
 399:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 400:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 401:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 423:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 424:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 425:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 492:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 493:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 494:5   ✖  Unexpected duplicate "display"                                                                                                          declaration-block-no-duplicate-properties    
 510:5   ✖  Unexpected duplicate "height"                                                                                                           declaration-block-no-duplicate-properties    
 804:3   ✖  Unexpected duplicate selector "#issue_tree .issue > td:not(.checkbox), #relations .issue > td:not(.checkbox)", first used at line 787   no-duplicate-selectors                       
 828:3   ✖  Unexpected duplicate selector "#issue_tree .issue > td.subject,   #relations .issue > td.subject", first used at line 799               no-duplicate-selectors                       

public/stylesheets/rtl.css
 322:3   ✖  Unexpected duplicate "margin-left"    declaration-block-no-duplicate-properties
 322:21  ✖  Unexpected duplicate "margin-right"   declaration-block-no-duplicate-properties
 377:14  ✖  Unexpected empty block                block-no-empty

#3 Updated by Marius BALTEANU about 1 month ago

  • File deleted (0001-Adds-Stylelint-to-lint-CSS-files.patch)

#4 Updated by Marius BALTEANU about 1 month ago

Updated the patch to ignore consecutive duplicated properties with different values in order to not trigger a violation for the following case:

display: -webkit-flex;
display: -ms-flexbox;
display: -webkit-box;
display: flex;

#5 Updated by Marius BALTEANU 30 days ago

  • Related to Patch #32890: Fix violations reported by Stylelint added

#6 Updated by Go MAEDA 9 days ago

  • Target version changed from Candidate for next major release to 4.2.0

I think it is nice to have Stylelint. Setting the target version to 4.2.0.

#7 Updated by Bernhard Rohloff 9 days ago

It's a good idea. And I'm completely with you Marius that we need some coding standards on the stylesheets.

Also available in: Atom PDF