Clean up system theme css

https://gist.github.com/3006391#L3648

First pass. Still some outstanding issues that I'm not entirely sure how to address.

#2 patch is no longer executable - the code has changed a lot.
Cleared some styles with CSS Lint, sorted them with CSS Comb
Also changed some dimensions from px to em (as in most of the css)
Patch applies both for system.theme.css and system.theme-rtl.css

#3: system_theme_css_cleanup-1663184-3.patch queued for re-testing.

Upon testing the patch:

system_theme_css_cleanup-1663184-3.patch:161: trailing whitespace.
abbr.form-required, 
system_theme_css_cleanup-1663184-3.patch:162: trailing whitespace.
abbr.tabledrag-changed, 
error: core/modules/system/system.theme-rtl.css: No such file or directory
error: core/modules/system/system.theme.css: No such file or directory

Patch should be rerolled.

Starting with the reroll.

Rerolled.

Sorry this now needs a reroll. I will try and review it soon after it is posted next time. Also I'm not sure if we should run the CSS through CSS comb here, we aren't doing that anywhere else? We should probably run every CSS file through it in one patch for consistency.

Patch Re rolled!

Bah! Looks like this got knocked out on the same day by #2372023: Description of display settings takes too much space

Rerolled.

Patch no longer applies after #2417705: Autocomplete suggestions visual regression after modal and jQuery UI update was committed.

error: patch failed: core/modules/system/css/system.theme.css:200
error: core/modules/system/css/system.theme.css: patch does not apply

Re-rolled and fixed a few CSSLint errors. Still needs some word in tabs__tab and progress bars. Working on it.

Not sure on the attribute shuffling, we don't use CSS comb anywhere else. It also doesn't help the review process or make the code any more readable than before. It also doesn't throw any warnings on CSS Lint. @LewisNyman do you think we can remove this part of the patch?

+++ b/core/modules/system/css/system.theme.css
@@ -126,8 +126,7 @@ abbr.ajax-changed {
-.container-inline .form-actions,
-.container-inline.form-actions {
+.container-inline .form-actions {

The concatenated selector was introduced in #162047: .container-inline class overridden by system.theme.css; also not working with .form-actions to fix the display of .container-inline on #type => 'action' form elements. This pattern is still in use for example on the 'Recent log messages' page so this line should not be removed.

If we need it then I guess we need it, it's going to be hard to fix every error in this issue without having to focus on an individual component, so let's try and fix the easy wins here and maybe introduce follow ups to deal with tricky components

Reroll made and fixed some warnings in CSS Lint.

+++ b/core/modules/system/css/system.theme.css
@@ -446,73 +483,73 @@ ul.tabs {
+  background: no-repeat 10px 17px;  /* LTR */

Remove extra space before /* LTR */

Reroll and I removed extra space before /* LTR */

I found three errors when I ran it though csslint:

1. +++ b/core/modules/system/css/system.theme.css
   @@ -210,34 +211,38 @@ details > .details-wrapper {
   +  -ms-transform: rotate(-90deg);
   +  -moz-transform: rotate(-90deg);
   +  -o-transform: rotate(-90deg);
   ...
   +  -ms-transform: rotate(90deg);
   +  -moz-transform: rotate(90deg);
   +  -o-transform: rotate(90deg);
   

   According to caniuse data, we no longer need the -moz- and -o- prefixes

2. +++ b/core/modules/system/css/system.theme.css
   @@ -278,14 +283,55 @@ th.checkbox {
   +  background-image: -moz-linear-gradient(#e7e7df, #f0f0f0);
   +  background-image: -webkit-gradient(linear, color-stop(0%,#e7e7df), color-stop(100%,#f0f0f0));
      background-image: -webkit-linear-gradient(#e7e7df, #f0f0f0);
   -  background-image:         linear-gradient(#e7e7df, #f0f0f0);
   +  background-image: -o-linear-gradient(#e7e7df, #f0f0f0);
   +  background-image: -ms-linear-gradient(#e7e7df,  #f0f0f0);
   +  background-image: linear-gradient(#e7e7df, #f0f0f0);
   ...
   +    -moz-linear-gradient( top, rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.15) ),
   +    -moz-linear-gradient( left top,
   ...
   +    -o-linear-gradient( top, rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.15) ),
   +    -o-linear-gradient( left top,
   ...
   +    -ms-linear-gradient( top, rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.15) ),
   +    -ms-linear-gradient( left top,
   

   We don't need any of these vendor prefixes according to caniuse data

3. +++ b/core/modules/system/css/system.theme.css
   @@ -446,80 +484,80 @@ ul.tabs {
    .button-action:before {
   -  content: '+';
   -  font-weight: 900;
      margin-left: -0.1em; /* LTR */
      padding-right: 0.2em; /* LTR */
   +  content: '+';
   +  font-weight: 900;
    }
    [dir="rtl"] .button-action:before {
   -  margin-left: 0;
      margin-right: -0.1em;
   -  padding-left: 0.2em;
   +  margin-left: 0;
      padding-right: 0;
   +  padding-left: 0.2em;
    }
   

   It looks like we don't add the margin-right where we should for RTL

4. +++ b/core/modules/system/css/system.theme.css
   @@ -446,80 +484,80 @@ ul.tabs {
    .messages + .messages {
   -  margin-top: 1.538em;
   +  margin-top: 1.54em;
   ...
    .messages__item + .messages__item {
   -  margin-top: 0.769em;
   +  margin-top: 0.77em;
   

   Can we keep these values as they are? I think these now round to uneven pixel units

Is it fine?
Thanks!

Upload an interdiff. Sorry!

Thanks, this definitely needs regression testing.

+++ b/core/modules/system/css/system.theme.css
@@ -46,8 +46,8 @@ td.is-active {
+.odd .form-item,
+.even .form-item {

@@ -55,17 +55,16 @@ tr.even .form-item {
-label.option {
+.option {

I am worried that these classes are too generic and will cause issues in other, unintended places.

I'm tempted to postpone this on #2395853: Split system.module.css and system.theme.css files into SMACSS style components, what do other people think?

I ran the patch through siteeffect and I found a lot of regressions, especially related to lists and tabs. See the gif as an example, it's taken from /admin/appearance/settings/seven

+1 to postpone as Lewis suggested in #35

Please find the attached patch which removed all the csslint issues.

Postponed on #2395853: Split system.module.css and system.theme.css files into SMACSS style components

Can we replace these issues with individual issues for each file? It would make it easier to work on. Does anyone want to create the issues? :)

Update the issue summary. system.theme.css is not available in core/modules/system/

@Maninders yeah right !!! All the css (system.theme.css) has moved to the seven as it is backend theme so rather we have to close this issue or need to update the issue summary.

Unable to apply patch against 8.2.x for ajax-progress.module.css and progress.module.css

Re-roll.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.

Some of the changes of the patch appear to have already landed but not all.

Also this was tagged for visual regression testing and issue summary update that still needs to happen.

The parent issue #1190252: [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core was closed in favour of #2865971: Use stylelint as opposed to csslint in core. I suspect that means the steps in the issue summary are no longer applicable. Can we close this one?

Since there's been no follow up going to assume we can close this one.