Problem/Motivation

This module is running CSSLint and we get a few CSS coding standards reported.

Steps to reproduce

The output can be seen in the regular tests on https://www.drupal.org/pift-ci-job/2564331
There are four css files with a total of 11 errors:

css/module_filter.css
5	Element (input.form-search) is overqualified, just use .form-search without element name.
15	Negative text-indent doesn't work well with RTL. If you use text-indent for image replacement explicitly set direction for that item to ltr.

css/module_filter.modules.css
1	Don't use IDs in selectors.
2	Don't use IDs in selectors.

css/module_filter.modules_tabs.css
109	Element (input.table-filter-text) is overqualified, just use .table-filter-text without element name.
115	Element (col.checkbox) is overqualified, just use .checkbox without element name.
118	Element (col.name) is overqualified, just use .name without element name.
121	Element (col.version) is overqualified, just use .version without element name.
124	Element (col.links) is overqualified, just use .links without element name.

css/module_filter.update_status.css
1	Don't use IDs in selectors.
5	Don't use IDs in selectors.

Proposed resolution

  1. Do not run CSSLINT [done]
  2. Re-evaluate when #2866840: Use stylelint as opposed to csslint in DrupalCI is finished

Issue fork module_filter-3332586

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented
Issue summary: View changes

Stylelint has actually superceded CSSLint as the standard checking tool that is used for Core. See the following:

#2865971: Use stylelint as opposed to csslint in core
#3166360: Disable csslint testing in core
#2866840: Use stylelint as opposed to csslint in DrupalCI

Therefore we should not run CSSLINT on the module's css files. This should be achievable via drupalci.yml.

When STYLELINT is available to contrib module testing we can add a step to run that.

Note: I was wrong in the original issue summary when I said that adding drupalci.yml file caused csslint to start running. It was being run by default all the time, even before we had out own drupalci.yml file

jonathan1055 opened merge request !16

jonathan1055’s picture

jonathan1055 commented

Dispatcher log looks good - no mention of csslint.

While we are changing drupalci.yml we might as well fix the problems reported by eslint

/var/www/html/modules/contrib/module_filter/drupalci.yml
Empty mapping values are forbidden. (yml/no-empty-mapping-value)

The steps need to appear in drupalci.yml, otherwise the test bot does not run them. We can resolve the empty mapping problem by specifying a parameter, say 'halt-on-fail' true/false, rather than let it use the default (even if that was the same as what we want). This also makes the file more explicitly clear.

jonathan1055’s picture

jonathan1055 commented

The issue was getting larger and off-topic, so let's keep this just on the CSS changes.

New issue is #3333862: YML coding standards

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review

Ready for review. This can be committed but then keep the issue open, as we will want to evaluate and add STYLELINT when that becomes available in Contrib testing.

smustgrave’s picture

smustgrave commented
Status: Needs review » Fixed
jonathan1055’s picture

jonathan1055 commented
Issue summary: View changes
Status: Fixed » Postponed

Thanks. We have not fixed the coding standards, merely hidden them until Stylelint becomes available in crontrib. Hence postponing this issue - waiting for #2866840: Use stylelint as opposed to csslint in DrupalCI

smustgrave’s picture

smustgrave commented
Status: Postponed » Fixed

Moving back to Fix, addressing any stylelint in the gitlab ticket now.

Status: Fixed » Closed (fixed)

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