Problem/Motivation

I know morten likes to complain about the CSS in Drupal core. And while he does love arguing, its much easier to change the CSS in core if we have useful metrics to show why the CSS is crappy.

Proposed resolution

Stubbornella (Nicole Sullivan) has been a long-time advocate of simple CSS to improve front-end performance. Complex selectors take more time for browsers to match against them. Needlessly-complex selectors lead to crazy cold-war-scenario specificity escalation as each rule tries to out-specify the more general rules.

She has also just released an open-source tool called csslint that we can use to test our current CSS rules.

http://www.stubbornella.org/content/2011/06/15/css-lint-open-sourced/
AND
http://csslint.net/

We've declared which rules we choose to follow inline with our CSS standards in #2222049: Add a .csslintrc file that's in line with our CSS standards

For those wishing to run csslint locally, follow the instructions for the Drupalcore frontend toolkit

Remaining tasks

Module CSS

Module Issues
block #2422399: Rewrite block.admin.css inline with our CSS standards
config_translation #2485397: Clean up config translation CSS inline with our CSS standards
content_translation #2485409: Clean up content translation CSS inline with our CSS standards
contextual #2485417: Clean up contextual CSS inline with our CSS standards
editor #2485425: Clean up editor CSS inline with our CSS standards
field_ui #2408469: Rewrite field ui components inline with our CSS standards
file #2485431: Clean up file CSS inline with our CSS standards
language #2485439: Clean up language CSS inline with our CSS standards
locale #2485497: Clean up locale CSS in line with our CSS standards
node #2485505: Remove CSSlint errors from node module css
quickedit #2408561: Rewrite Quick Edit CSS to meet our CSS standards
shortcut #2485375: Clean up shortcut CSS inline with our CSS standards
simpletest #2483913: Clean up simpletest.module.css
system #2395853: Split system.module.css and system.theme.css files into SMACSS style components #1663170: Clean up system.module.css #1663184: Clean up system theme css
toolbar #2419135: Change the used CSS classes to follow the coding standards
tour #2485293: Clean up tour.module.css
views_ui #2408525: Rewrite Views UI CSS inline with our CSS standards - Part 1

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Stalski’s picture

Subscribe. Agreed as good starting point.

Maikel’s picture

Subscribe. Interested in seeing how this will be implemented.

Sidenote: Does this also mean that selectors (ids, classes etc) need to be altered too? Maybe interesting to look at additional ways to minify and cleanup the css code OOCSS for example. Or will that be a step too far for implementing in core.

fubhy’s picture

... And after that we just need to move the entire Core CSS into a "Core Theme"... Please?!

jyve’s picture

Subscribing. Sounds good to have a guideline to follow, especially in the world of CSS.

maartenverbaarschot’s picture

Would be extremely nice to have a built-in patch checker for this at drupal.org. Like we currently have for PHP tests. Ideally one checks manually before uploading the patch, but it could automate part of the reviewing process.

mortendk’s picture

+1 :)

dcmouyard’s picture

+1 Sounds good to me!

Hopefully this will lead to applying OOCSS principles to Drupal.

tlattimore’s picture

I am all for this!
+1

Jeff Burnz’s picture

Cool, over the past year or so I've become a big fan of lint tools, so +1.

I don't think there is any problem getting buy-in on this issue - so maybe my initial question is scope. What I might suggest is we start running our CSS through these lint tools to get a documented list of issues - this will give us some scope and may well offer some inspiration on how we can improve on the current situation with core CSS.

What I might suggest is..

1. get a list of all core CSS
2. create a list of suitable lint tools
3. create a meta issue and break it down into separate issues per module/stylesheet
4. run the tests
5. collate the results and see what we come up with

Could even be a sprint.

JohnAlbin’s picture

3. create a meta issue and break it down into separate issues per module/stylesheet

I believe that's what this issue is. :-)

1. get a list of all core CSS

I've got one of those in Zen’s drupal7-reference.css file. (Assuming no one has changed the D8 list yet.) I went ahead and put this in the initial issue description above to make tracking the changes a bit easier.

AdrianB’s picture

Subscribing.

Jacine’s picture

Issue tags: +Front end

There's an initiative page for cleaning up CSS here. It'd be great to add some info about CSS Lint and the core stylesheets to that page. I'll do it if no one else gets to it first. I also think it'd be awesome to get it in our automated patch testing as @mverbaar mentioned. Tagging "Front end" to get this on the radar of people that are interested in helping out with this via @drupalfrontend.

steinmb’s picture

+1

Jacine’s picture

The page I referred to in #12 was updated after the HTML5 Initiative meeting today. @johnvsc created all the issues and linked them in a table here for those interested in helping: http://drupal.org/node/1089868

dodorama’s picture

Oh yes

JohnAlbin’s picture

I'm going to try to resurrect this issue.

cosmicdreams’s picture

Happy I found this issue.

I'm going to "explode" this issue to each of the stylesheets this weekend (Saturday) if it's still a good idea. For the most part, this effort is inline with the CSS cleanup efforts of the HTML5 initiative. using CSSLint at http://csslint.net provides a quick way to code-sniff our css and tweak styles until they pass the tests.

So, as I see it, what needs to be done is:

1. Create an issue for each of the stylesheets that are mentioned in the OP
2. For each stylesheet copy and paste the it into the online tool and test it.
3. Fix any warnings or errors
4. Create a patch and see if the patch passes Drupal's automated tests.
5. Debate about the changes.

cosmicdreams’s picture

Issue summary: View changes

added link to online css lint tool.

ZenDoodles’s picture

Issue summary: View changes

Updated issue summary.

webchick’s picture

Probably. If it's the exact same "robotic" changes just in lots of different files though, it might be easier to review/commit as a single patch. If we're talking a 500K+ patch, then probably not since the re-rolls alone would be really irritating.

Might as well try it and see what we end up with then go from there.

tlattimore’s picture

Thanks a lot for chiming in on this, webchick. I anticipate it will likely be a very large patch if all the changes a rolled into one. There has been a great deal of effort to clean things up (noted in the issue summary). But my guess is that css lint will still have a boat load of changes suggested.

tlattimore’s picture

Issue summary: View changes

Updated issue summary.

ZenDoodles’s picture

Title: Use csslint as a weapon to beat the crappy CSS out of Drupal core » Meta: Use csslint as a weapon to beat the crappy CSS out of Drupal core
RobLoach’s picture

I ran CSSLint through node, and this is the result.

droplet’s picture

Ahh. that's good.

There also a CSS Cleaup mission. Some of them are duplicated issue
http://drupal.org/node/1089868

barraponto’s picture

Aren't we cleaning up the core themes CSS as well?

barraponto’s picture

btw, here's how I roll: find core/modules/openid/ -type f -name *.css -exec csslint {} \;

barraponto’s picture

Issue summary: View changes

Updated issue summary.

kerasai’s picture

Issue summary: View changes

Updated issue summary.

greggles’s picture

Issue summary: View changes

removing openid issue - it's still valid in the contrib openid but not relevant for this core meta-issue.

LewisNyman’s picture

I just created #2222049: Add a .csslintrc file that's in line with our CSS standards. It would be nice to get some opinions please

LewisNyman’s picture

Issue tags: +CSS, +frontend
LewisNyman’s picture

So #2222049: Add a .csslintrc file that's in line with our CSS standards is now committed which means we can use it to get better feedback from CSSlint that's inline with our standards. I've attached the patch for a grunt task you can run that pulls in the csslintrc file.

LewisNyman’s picture

Component: markup » CSS

I had a look over the current status of CSS in core, here are the results:

https://gist.github.com/lewisnyman/10c756cb6f7a204485ca

mortendk’s picture

auch - well now we know what to do the next couple of weeks ;)

cosmicdreams’s picture

Making progress with ids is an interesting challenge. These ids have been deeply rooted for a long time. Should we endeavor to eliminate them?

In the meantime Lewis, it might be good to put the gulpfile you're using to automate the css linting. I assume you also have a package.json that defines how to import the necessary packages with bower.

Establishing a project like that would make the future work faster for all of us.

mortendk’s picture

ID's are bad practice they always have and always will be.

LewisNyman’s picture

@cosmicdreams Funny you should say that because I started working on something like this today ;-) https://github.com/lewisnyman/drupalcore-frontend-toolkit

I don't know if it's completely usable yet but feel free to give it a go.

RobLoach’s picture

There's now an HTML Report up at http://lewisnyman.co.uk/drupalcore-frontend-toolkit/ so you can get an overview.

tim.plunkett’s picture

Title: Meta: Use csslint as a weapon to beat the crappy CSS out of Drupal core » [meta] Use csslint as a weapon to beat the crappy CSS out of Drupal core
mortendk’s picture

it should be called "Drupals css wall of shame" ;)
lewis its very usable and gives us a really good tool to find all the little things, that probably needs a cleanup (like the use of important etc)

mortendk’s picture

and for those that are running it locally heres my node csslint
csslint core --format=compact --exclude-list=core/assets/vendor/,core/modules/ckeditor/,core/vendor/ > lintcss-trap.txt
yes im ignoring venor, cause we have enough crap as it is b our self ;)

martin107’s picture

Issue summary: View changes

Move, along nothing to see here... just moving the command line suggestion into the issue summary, just to save me digging around for #37 later. :-)

LewisNyman’s picture

The status as of February 10th:

57 lint free files out of 145
846 errors

LewisNyman’s picture

The status as of February 12th:

62 lint free files out of 145
802 errors

mortendk’s picture

83 files on the wall - 83 files on the wall, clean one up & sent it to review
82 files on the wall ....

LewisNyman’s picture

Title: [meta] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-789] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status:

63 lint free files out of 144
789 errors

LewisNyman’s picture

Title: [META-789] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-782] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status:

63 lint free files out of 144
782 errors

dawehner’s picture

Well, do you really want to fix every single issue with their own issue?

LewisNyman’s picture

Well, do you really want to fix every single issue with their own issue?

Our strategy at the moment is to fix individual files/components. It becomes very hard to test regressions when we have one patch that fixes one problem across all files. Some files have 40+ errors in them, so we can be quite productive still.

mortendk’s picture

we need to go through each file & clean them up, that at the same time have the advantage that we then as well quickly gets into cleaning them up as well with the MAT filenaming & putting the right css where it belong. That should if the might Drupal gods wants it, make it easier for us to clean the css even more

Jeff Burnz’s picture

The issue for Forum CSS linked to in the IS is postponed on an issue that itself is postponed to an issue that is now closed and didn't really appear to fix anything - the closed issue says changes were merged into the D8 mobile initiative branch but I have no idea what happened to that since those CSS changes are not in core: #2028049: Clean up the CSS for Forum module, can we re-open #1217002: Clean up the CSS for Forum module and link to in the IS?

LewisNyman’s picture

Yep that makes sense, looks like there are a few issues that were closed as duplicates that we could reopen and tag correctly?

#1217054: Clean up the CSS for User module #1217052: Clean up the CSS for Update module #1217048: Clean up the CSS for Toolbar module #1217040: Clean up the CSS for Simpletest module

mgifford’s picture

Can we remove #1663144: Clean up css in overlay?

Are we worried about D7 at this point in this issue?

LewisNyman’s picture

@mgifford I guess this meta s only for 8.x so we can remove the reference for the overlay issue?

LewisNyman’s picture

Title: [META-782] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-780] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Todays status

62 lint free files out of 143
780 errors

LewisNyman’s picture

Title: [META-780] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-692] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status
65 lint free files out of 144
692 errors

NIce!

LewisNyman’s picture

Issue summary: View changes

Today's status
65 lint free files out of 145
690 errors

LewisNyman’s picture

Title: [META-692] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-690] Use csslint as a weapon to beat the crappy CSS out of Drupal core
LewisNyman’s picture

Issue tags: +csslint
LewisNyman’s picture

Issue summary: View changes
LewisNyman’s picture

Today's status
67 lint free files out of 144
679 errors

LewisNyman’s picture

Title: [META-690] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-667] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status
71 lint free files out of 145
665 errors

LewisNyman’s picture

Title: [META-667] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-665] Use csslint as a weapon to beat the crappy CSS out of Drupal core
LewisNyman’s picture

LewisNyman’s picture

Title: [META-665] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [META-565] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status
74 lint free files out of 145
565 errors

Down by 100 in just over a week :-) We are getting those devdays issues committed!

LewisNyman’s picture

Issue summary: View changes

I updated the issue summary with a table of each module so we can see which issues need to be created

LewisNyman’s picture

Issue summary: View changes
Manjit.Singh’s picture

Issue summary: View changes
LewisNyman’s picture

Issue summary: View changes
LewisNyman’s picture

Issue summary: View changes
LewisNyman’s picture

Issue summary: View changes
Manjit.Singh’s picture

Issue summary: View changes
LewisNyman’s picture

Title: [META-565] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [545] Use csslint as a weapon to beat the crappy CSS out of Drupal core
Category: Task » Plan

Today's status

79 lint free files out of 145
545 errors

We've fixed some of the smaller files but we still have a lot of files to go.

LewisNyman’s picture

Title: [545] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [445] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Just like that, we lose 100 errors

83 lint free files out of 145
445 errors

LewisNyman’s picture

Title: [445] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [422] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status

86 lint free files out of 145
422 errors

We are down to about half the number of CSSLint issues we had 6 months ago. Well done everyone!

LewisNyman’s picture

Title: [422] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [421] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status

111 lint free files out of 185
421 errors

We've added about 40 CSS files in #2395853: Split system.module.css and system.theme.css files into SMACSS style components

LewisNyman’s picture

Title: [421] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [389] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status
114 lint free files out of 190
389 errors

New files and fixes from #2398463: Clean up the "content" component in Bartik

attiks’s picture

I've added an issue to add automated testing to the testbot #2575497: DrupalCI should report code style issues - CSS, so once all css is clean it can be enforced.

LewisNyman’s picture

Title: [389] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [333] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status
124 lint free files out of 197
333 errors

elachlan’s picture

Extra eyes would be appreciated for #2575497: DrupalCI should report code style issues - CSS and associated child issues.

We should be in a position to switch this on as soon as core is clean.

LewisNyman’s picture

Core won't be clean until 9.x now. We have CSSlint errors in Classy and Stable. :(

LewisNyman’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Active » Postponed

Postponing until 8.1 as we can still clean up Seven, Bartik, and module CSS.

cosmicdreams’s picture

Is this something we can continue progress on in it's own branch?

LewisNyman’s picture

@cosmicdreams We can make progress on the linting errors in module CSS, Bartik, and Seven for 8.1. Maybe it makes sense to work on a branch for Stable and Classy, as they are frozen until 9.x

LewisNyman’s picture

Title: [333] Use csslint as a weapon to beat the crappy CSS out of Drupal core » [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Also, #2575737: Copy templates, CSS, and related assets to Stable and override core libraries' CSS added around 250 csslint errors to core as it copied all the module errors over :(.

175 lint free files out of 283
573 errors

steinmb’s picture

Doh, that kinda suck. Getting DrupalCI to fail on those should stop this from happening again.

cosmicdreams’s picture

Or if making it fail is too extreme perhaps list them as warnings.

elachlan’s picture

I believe the idea was to list them as warnings until core is clear of warnings. Then switch it over to failure. This might be a bit extreme for css. Nothing is set in stone yet.

catch’s picture

Version: 8.1.x-dev » 8.0.x-dev
Status: Postponed » Active

If there's no functional change, we can possibly commit these kinds of changes to patch releases, moving back for more discussion.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.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

I'd be really awesome if people wanted to start to work on this again. There are hundreds of errors in HEAD - see https://www.drupal.org/pift-ci-job/634063 for example.

cosmicdreams’s picture

Well, Midcamp is coming up! So yea, this would be an awesome issue to work on this weekend.

droplet’s picture

Want to ask a quick question here before creating a new issue thread.

How about migrating it to stylelint (https://stylelint.io/). It's a better Modern tool for editors and more automatic formatting and better for now and then (e.g. SCSS in CORE, or SCSS / SASS / Whatever in Contrib)

cosmicdreams’s picture

It seems that stylelint.io has an npm dependency. I'm not sure if anything else we've included has a npm dependency so this looks to be the first one.

On the plus side it seems that stylelint has good PhpStorm support: https://www.jetbrains.com/help/phpstorm/2016.3/using-stylelint-code-qual... so for me that's cool.

@droplet: would we need to define a custom ruleset for the linter to follow with stylelint or will using stylelint-config-standard be sufficient.

cosmicdreams’s picture

Oh you can install stylelint globally and hook use it via the CLI or with PhpStorm/...your IDE of choice. That's cool. So nevermind about that node requirement.

cosmicdreams’s picture

Hey @droplet go ahead and create that issue for stylelint please. Also if you could, can you please show an example for what the contents of the .stylelintrc should be?

I tried this but it didn't find anything wrong with all the css.

{
  "extends": "stylelint-config-standard"
}

Also, this should just scan css files right?

alexpott’s picture

@droplet, @cosmicdreams - created the issue - #2865971: Use stylelint as opposed to csslint in core

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.

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.

joelpittet’s picture

Status: Active » Closed (outdated)