Split system.module.css and system.theme.css files into SMACSS style components

hmm, I can't find any documentation on component styling for modules in the standards. I think we should still maintain the .module.css and .theme.css split.

We had a similar problem in #2375673: Split Bartik's CSS into SMACSS style components - Here is a link to Wim's comment there: https://www.drupal.org/node/2375673#comment-9375013

I think that we have already bitten off a lot in one patch. The more CSS we change in one patch the harder it becomes to test. We need some visual regression testing to verify that we haven't broken anything.

I think quite a lot of things will break before #2389735: Core and base theme CSS files in libraries override theme CSS files with the same name is committed.

1. +++ b/core/modules/system/system.libraries.yml
   @@ -13,7 +40,15 @@ admin:
   +      css/layout.css: {}
   

   This layout file isn't loading because Seven has this file already. I think for now we should rename it to layout.admin.css?

2. +++ b/core/modules/system/system.libraries.yml
   @@ -13,7 +40,15 @@ admin:
   +      css/components/module-page-styles.css: {}
   

   I think this file can just be called module page

Postponed on #2389735: Core and base theme CSS files in libraries override theme CSS files with the same name

@pguillard That's ok, it will hopefully still apply after the other is committed

#2389735: Core and base theme CSS files in libraries override theme CSS files with the same name is in

Ok, so I ran some visual regression tests and I didn't get any regressions! It seems like we still to move some more stuff out of the system css files and into separate files, so setting to needs work.

Nice, thanks! I went over the file names to make sure that they match the name of the component class. This makes it easy for a frontend developer to easily find the name of the file after looking at the HTML class in a browser inspector. The exceptions are modules-page and appearance-page which are not yet componentised.

1. +++ b/core/modules/system/css/components/container-inline.css
   @@ -0,0 +1,33 @@
   + * @file
   + * Styles for inlien items. Rows 7-14 from system.module.css.
   + * Rows 16-33 from system.theme.css
   + */
   +
   +.container-inline div,
   +.container-inline label {
   +  display: inline;
   +}
   +/* Details contents always need to be rendered as block. */
   +.container-inline .details-wrapper {
   +  display: block;
   +}
   +
   +/**
   + * Inline items.
   + */
   +.container-inline label:after,
   +.container-inline .label:after {
   

   Can we remove the top comment and merge this file together?

2. +++ b/core/modules/system/css/components/progress.css
   @@ -0,0 +1,115 @@
   + * @file
   + * Visual styles for progress bar.
   + *
   + * from system.theme.css
   ...
   + * Progress behavior.
   + *
   + * from system.module.css
   

   Can we remove these comments that reference the original files?

3. +++ /dev/null
   @@ -1,370 +0,0 @@
   -/**
   - * Panel.
   - * Used to visually group items together.
   - */
   -.panel {
   -  padding: 5px 5px 15px;
   -}
   -.panel__description {
   -  margin: 0 0 3px;
   -  padding: 2px 0 3px 0;
   -}
   -
   

   We need to move this CSS into panel.css and delete admin-panel.css

4. +++ /dev/null
   @@ -1,370 +0,0 @@
   -/**
   - * System compact link: to toggle the display of description text.
   - */
   -.compact-link {
   -  margin: 0 0 0.5em 0;
   -}
   

   The name of the corresponding file should be 'compact-link' instead of 'compact-links'

5. +++ /dev/null
   @@ -1,370 +0,0 @@
   -/**
   - * Quick inline admin links.
   - */
   -small .admin-link:before {
   -  content: ' [';
   -}
   -small .admin-link:after {
   -  content: ']';
   

   The name of this file should be 'admin-link'

6. +++ b/core/modules/system/css/system.module.css
   @@ -4,37 +4,6 @@
   - * Autocomplete.
   - *
   - * @see autocomplete.js
   - */
   -
   -/* Animated throbber */
   -.js input.form-autocomplete {
   

   Can we call this file form-autocomplete?

7. +++ b/core/modules/system/css/system.theme.css
   @@ -154,37 +106,6 @@ abbr.ajax-changed {
   -/**
   - * Show buttons as links.
   - */
   -button.link {
   -  background: transparent;
   

   The name of this file should be 'link.css' instead of 'button.css'

8. +++ b/core/modules/system/css/system.theme.css
   @@ -232,361 +153,3 @@ summary {
   -/**
   - * Markup generated by breadcrumb.html.twig.
   - */
   -.breadcrumb {
   

   This file should be called 'breadcrumb.css' instead of 'breadcrumbs.css'

9. +++ b/core/modules/system/css/components/textarea.css
   --- /dev/null
   +++ b/core/modules/system/css/components/throbber.css
   
   +++ b/core/modules/system/css/components/throbber.css
   +++ b/core/modules/system/css/components/throbber.css
   @@ -0,0 +1,46 @@
   
   @@ -0,0 +1,46 @@
   +/**
   + * @file
   + * Visual styles for throbbers.
   + */
   +
   +.ajax-progress {
   

   Can we call this file ajax-progress?

+++ b/core/modules/system/css/components/links.css
@@ -13,6 +13,6 @@ ul.inline li {
 }
-ul.links a.active {
+ul.links a.is-active {
   color: #000;

Can we avoid making improvements and fixes in this issue? It becomes harder to track the movement of the code if it changes.

We had a discussion about how to structure the CSS files across the modules and Classy, see: #2489460: [Meta] Move module.theme.css files to Classy or Seven

In this issue we need to maintain the seperation between the system.module.css file and the system.theme.css file so we can move the theme style to Classy later.

The naming conventions should be: [component].module.css and [component].theme.css

Thanks for this!

+++ b/core/modules/system/css/system.admin.css
@@ -9,6 +9,7 @@
+
 .layout-container:after {
   content: "";
   display: table;

@@ -19,26 +20,33 @@
+
   .layout-column {
-    float: left;  /* LTR */
+    float: left; /* LTR */

This patch makes a few spacing changes to system.admin.css - we should avoid making these kinds of changes in this issue. Setting to needs work.

There is still a lot of CSS in system.module.css and system.theme.css. I'll make some suggestions for the remaining CSS in system.theme.css:

.node--unpublished {
  background-color: #fff4f4;
}

This CSS is already in node-unpublished.theme.css but it should be called node.theme.css, no idea why this is in system instead of node but let's move/delete it in another issue.

th.is-active img {
  display: inline;
}
td.is-active {
  background-color: #ddd;
}

This is already moved into tablesort.css so we can remove this

.form-item,
.form-actions {
  margin-top: 1em;
  margin-bottom: 1em;
}

All the form stuff should go into form.theme.css

.icon-help {
  background: url(../../../misc/help.png) 0 50% no-repeat; /* LTR */
  padding: 1px 0 1px 20px; /* LTR */
}

This should be in icons.theme.css

details {
  border: 1px solid #ccc;
  margin-top: 1em;
  margin-bottom: 1em;
}
details > .details-wrapper {
  padding: 0.5em 1.5em;
}
/* @todo Regression: The summary of uncollapsible details are no longer
     vertically aligned with the .details-wrapper in browsers without native
     details support. */
summary {
  cursor: pointer;
  padding: 0.2em 0.5em;
}

We shouldn't be styling default elements in modules... but for now let's put this in details.theme.css

.collapse-processed > summary {
  padding-left: 0.5em;
  padding-right: 0.5em;
}
.collapse-processed > summary:before {
  background: url(../../../misc/menu-expanded.png) 0px 100% no-repeat; /* LTR */
  content: "";
  float: left;
  height: 1em;
  width: 1em;
}

collapse-processed.theme.css

Bumping this to major because many issues rely on this before they can make improvements to this CSS. It also affects a lot of CSS that's loaded on every page!

Thanks, there are now only two groups of CSS left in system.theme.css:

/**
 * Publishing status.
 */
.node--unpublished {
  background-color: #fff4f4;
}

We can delete this, it's already been moved.

.button,
.image-button {
  margin-left: 1em;
  margin-right: 1em;
}
.button:first-child,
.image-button:first-child {
  margin-left: 0;
  margin-right: 0;
}

This code can go in button.theme.css

Also:

/**
 * @file
 * Visual styles for buttons.
 */
button.link {
  background: transparent;
  border: 0;
  cursor: pointer;
  margin: 0;
  padding: 0;
  font-size: 1em;
}
label button.link {
  font-weight: bold;
}

The class here is .link so this code should be in link.theme.css instead of button.theme.css

1. +++ b/core/modules/system/css/components/button.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/collapse-processed.theme.css
   
   +++ b/core/modules/system/css/components/collapse-processed.theme.css
   +++ b/core/modules/system/css/components/collapse-processed.theme.css
   @@ -0,0 +1,27 @@
   
   @@ -0,0 +1,27 @@
   +.collapse-processed > summary {
   

   This file needs a @file comment

2. +++ b/core/modules/system/css/components/collapse-processed.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/container-inline.theme.css
   
   +++ b/core/modules/system/css/components/container-inline.theme.css
   +++ b/core/modules/system/css/components/container-inline.theme.css
   @@ -0,0 +1,33 @@
   
   @@ -0,0 +1,33 @@
   +/**
   + * @file
   + * Styles for inlien items. Rows 7-14 from system.module.css.
   + * Rows 16-33 from system.theme.css
   

   There is a typo here, also can we remove the information about the rows?

3. +++ b/core/modules/system/css/components/details.theme.css
   @@ -0,0 +1,21 @@
   + * Collapsible details.
   + *
   + * @see collapse.js
   + * @thanks http://nicolasgallagher.com/css-background-image-hacks/
   + */
   +details {
   

   There should be a blank line after the @file comment block

4. +++ b/core/modules/system/css/components/details.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/exposed-filters.theme.css
   
   +++ b/core/modules/system/css/components/exposed-filters.theme.css
   +++ b/core/modules/system/css/components/exposed-filters.theme.css
   @@ -0,0 +1,45 @@
   
   @@ -0,0 +1,45 @@
   +/**
   + * @file
   + * Visual styles for exposed filters.
   + */
   +.exposed-filters .filters {
   

   There should be a blank line after the @file comment block

5. +++ b/core/modules/system/css/components/form.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/icons.theme.css
   
   +++ b/core/modules/system/css/components/icons.theme.css
   +++ b/core/modules/system/css/components/icons.theme.css
   @@ -0,0 +1,9 @@
   
   @@ -0,0 +1,9 @@
   +/* Style for the help icon. */
   +.icon-help {
   

   This needs to be a proper @file comment block

6. +++ b/core/modules/system/css/components/table-select.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/tabledrag.theme.css
   
   +++ b/core/modules/system/css/components/tabledrag.theme.css
   +++ b/core/modules/system/css/components/tabledrag.theme.css
   @@ -0,0 +1,117 @@
   
   @@ -0,0 +1,117 @@
   +/**
   + * @file
   + * Visual styles for table drag
   

   This comment needs a full stop

7. +++ b/core/modules/system/css/components/throbber.theme.css
   --- a/core/modules/system/css/system.admin.css
   +++ b/core/modules/system/css/system.admin.css
   
   +++ b/core/modules/system/css/system.admin.css
   +++ b/core/modules/system/css/system.admin.css
   @@ -14,13 +14,12 @@
   
   @@ -14,13 +14,12 @@
      display: table;
      clear: both;
    }
   -
   
   @@ -55,6 +54,14 @@
   +.admin-panel {
   +  margin: 0;
   +  padding: 5px 5px 15px 5px;
   +}
   +.admin-panel .description {
   +  margin: 0 0 3px;
   +  padding: 2px 0 3px 0;
   +}
   

   Can we revert all changes to system.admin.css? So we don't touch it at all in this patch.

1. +++ b/core/modules/system/css/components/button.theme.css
   @@ -0,0 +1,14 @@
   + * @file
   + * Visual styles for buttons.
   + */
   +.button,
   

   This @file comment news a blank line

2. +++ b/core/modules/system/css/components/item-list.theme.css
   --- /dev/null
   +++ b/core/modules/system/css/components/links.theme.css
   
   +++ b/core/modules/system/css/components/links.theme.css
   @@ -0,0 +1,29 @@
   +ul.links a.is-active {
   +  color: #000;
   +}
   +button.link {
   +  background: transparent;
   

   I think that it's better to put the button.link stuff into link.theme.css - because the two classes are not related even though they sound it

3. +++ b/core/modules/system/system.libraries.yml
   @@ -4,8 +4,37 @@ base:
          css/system.theme.css: { every_page: true, weight: -10 }
   

   We need to remove this this file from the yml file. Also, I guess we should also apply those every_page and weight settings to all of the files?

@lewis This is already in links.theme.css or am i missing something.

Yep :) I think that we should put it in link.theme.css

Thanks for working on this! I'll also go through the system.module.css file and make suggestions. There are a lot of utility classes in there that would be really tedious to have in individual files.

In system.module.css:

/**
 * Collapsible details.
 *
 * @see collapse.js
 */
.js details:not([open]) .details-wrapper {
  display: none;
}

details.module.css

/**
* Resizable textareas.
*/
.form-textarea-wrapper textarea {
display: block;
margin: 0;
width: 100%;
box-sizing: border-box;
}
.resize-none {
resize: none;
}
.resize-vertical {
resize: vertical;
min-height: 2em;
}
.resize-horizontal {
resize: horizontal;
max-width: 100%;
}
.resize-both {
resize: both;
max-width: 100%;
min-height: 2em;
}

resize.module.css

/**
 * TableDrag behavior.
 *
 * @see tabledrag.js
 */
body.drag {
  cursor: move;
}
tr.region-title {
  font-weight: bold;
}
tr.region-message {
  color: #999;
}
tr.region-populated {
  display: none;
}
tr.add-new .tabledrag-changed {
  display: none;
}
.draggable a.tabledrag-handle {
  cursor: move;
  float: left; /* LTR */
  height: 1.7em;
  margin-left: -1em; /* LTR */
  overflow: hidden;
  text-decoration: none;
}
[dir="rtl"] .draggable a.tabledrag-handle {
  float: right;
  margin-right: -1em;
  margin-left: 0;
}
a.tabledrag-handle:hover {
  text-decoration: none;
}
a.tabledrag-handle .handle {
  background: url(../../../misc/icons/787878/move.svg) no-repeat 6px 7px;
  height: 14px;
  margin: -0.4em 0.5em 0;
  padding: 0.42em 0.5em;
  width: 14px;
}
a.tabledrag-handle:hover .handle,
a.tabledrag-handle:focus .handle {
  background-image: url(../../../misc/icons/000000/move.svg);
}
.touch .draggable td {
  padding: 0 10px;
}
.touch .draggable .menu-item__link {
  display: inline-block;
  padding: 10px 0;
}
.touch a.tabledrag-handle {
  height: 44px;
  width: 40px;
}
.touch a.tabledrag-handle .handle {
  background-position: 40% 19px;
  height: 21px;
}
.touch .draggable.drag a.tabledrag-handle .handle {
  background-position: 50% -32px;
}
div.tree-child {
  background: url(../../../misc/tree.png) no-repeat 11px center; /* LTR */
}
div.tree-child-last {
  background: url(../../../misc/tree-bottom.png) no-repeat 11px center; /* LTR */
}
[dir="rtl"] div.tree-child,
[dir="rtl"] div.tree-child-last {
  background-position: -65px center;
}
div.tree-child-horizontal {
  background: url(../../../misc/tree.png) no-repeat -11px center;
}
.tabledrag-toggle-weight-wrapper {
  text-align: right; /* LTR */
}
[dir="rtl"] .tabledrag-toggle-weight-wrapper {
  text
}
.indentation {
  float: left; /* LTR */
  height: 1.7em;
  margin: -0.4em 0.2em -0.4em -0.4em; /* LTR */
  padding: 0.42em 0 0.42em 0.6em; /* LTR */
  width: 20px;
}
[dir="rtl"] .indentation {
  float: right;
  margin: -0.4em -0.4em -0.4em 0.2em;
  padding: 0.42em 0.6em 0.42em 0;
}

tabledrag.module.css

/**
 * TableHeader behavior.
 *
 * @see tableheader.js
 */
table.sticky-header {
  background-color: #fff;
  margin-top: 0;
  z-index: 500;
  top: 0;
}

sticky-header.css

/**
 * Progress behavior.
 *
 * @see progress.js
 */
.progress {
  position: relative;
}
.progress__track {
  background-color: #fff;
  border: 1px solid;
  margin-top: 5px;
  max-width: 100%;
  min-width: 100px;
  height: 16px;
}
.progress__bar {
  background-color: #000;
  height: 1.5em;
  min-width: 3%;
  max-width: 100%;
}
.progress__description,
.progress__percentage {
  color: #555;
  overflow: hidden;
  font-size: .875em;
  margin-top: 0.2em;
}
.progress__description {
  float: left; /* LTR */
}
[dir="rtl"] .progress__description {
  float: right;
}
.progress__percentage {
  float: right; /* LTR */
}
[dir="rtl"] .progress__percentage {
  float: left;
}
.progress--small .progress__track {
  height: 7px;
}
.progress--small .progress__bar {
  height: 7px;
  background-size: 20px 20px;
}

progress.module.css

/* Throbber */
.ajax-progress {
  display: inline-block;
  padding: 1px 5px 2px 5px;
}
[dir="rtl"] .ajax-progress {
  float: right;
}
.ajax-progress-throbber .throbber {
  background: transparent url(../../../misc/throbber-active.gif) no-repeat 0px center;
  display: inline;
  padding: 1px 5px 2px;
}
.ajax-progress-throbber .message {
  display: inline;
  padding: 1px 5px 2px;
}
tr .ajax-progress-throbber .throbber {
  margin: 0 2px;
}
.ajax-progress-bar {
  width: 16em;
}

/* Full screen  throbber */
.ajax-progress-fullscreen {
  /* Can't do center:50% middle: 50%, so approximate it for a typical window size. */
  left: 49%;
  position: fixed;
  top: 48.5%;
  z-index: 1000;
  background-color: #232323;
  background-image: url("../../../misc/loading-small.gif");
  background-position: center center;
  background-repeat: no-repeat;
  border-radius: 7px;
  height: 24px;
  opacity: 0.9;
  padding: 4px;
  width: 24px;
}

ajax-progress.module.css

/**
 * Inline items.
 */
.container-inline div,
.container-inline label {
  display: inline;
}
/* Details contents always need to be rendered as block. */
.container-inline .details-wrapper {
  display: block;
}

container-inline.module.css

/* Display form elements horizontally. */
.form--inline .form-item {
  float: left; /* LTR */
  margin-right: 0.5em; /* LTR */
}
[dir="rtl"] .form--inline .form-item {
  float: right;
  margin-right: 0;
  margin-left: 0.5em;
}
.form--inline .form-item-separator {
  margin-top: 2.3em;
  margin-right: 1em; /* LTR */
  margin-left: 0.5em; /* LTR */
}
[dir="rtl"] .form--inline .form-item-separator {
  margin-right: 0.5em;
  margin-left: 1em;
}
.form--inline .form-actions {
  clear: left; /* LTR */
}
[dir="rtl"] .form--inline .form-actions {
  clear: right;
}

form--inline.module.css

/**
 * Prevent text wrapping.
 */
.nowrap {
  white-space: nowrap;
}

nowrap.module.css

/**
 * For anything you want to hide on page load when JS is enabled, so
 * that you can use the JS to control visibility and avoid flicker.
 */
.js .js-hide {
  display: none;
}

/**
 * For anything you want to show on page load only when JS is enabled.
 */
.js-show {
  display: none;
}
.js .js-show {
  display: block;
}

js.module.css

/**
 * Hide elements from all users.
 *
 * Used for elements which should not be immediately displayed to any user. An
 * example would be collapsible details that will be expanded with a click
 * from a user. The effect of this class can be toggled with the jQuery show()
 * and hide() functions.
 */
.hidden {
  display: none;
}

/**
 * Hide elements visually, but keep them available for screen readers.
 *
 * Used for information required for screen reader users to understand and use
 * the site where visual display is undesirable. Information provided in this
 * manner should be kept concise, to avoid unnecessary burden on the user.
 * "!important" is used to prevent unintentional overrides.
 */
.visually-hidden {
  position: absolute !important;
  clip: rect(1px, 1px, 1px, 1px);
  overflow: hidden;
  height: 1px;
  width: 1px;
  word-wrap: normal;
}

/**
 * The .focusable class extends the .visually-hidden class to allow
 * the element to be focusable when navigated to via the keyboard.
 */
.visually-hidden.focusable:active,
.visually-hidden.focusable:focus {
  position: static !important;
  clip: auto;
  overflow: visible;
  height: auto;
  width: auto;
}

/**
 * Hide visually and from screen readers, but maintain layout.
 */
.invisible {
  visibility: hidden;
}

hidden.module.css

/**
 * Float clearing.
 *
 * Based on the micro clearfix hack by Nicolas Gallagher, with the :before
 * pseudo selector removed to allow normal top margin collapse.
 *
 * @see http://nicolasgallagher.com/micro-clearfix-hack
 */
.clearfix:after {
  content: "";
  display: table;
  clear: both;
}

clearfix.module.css

/**
 * Text alignment classes.
 */
.text-align-left {
  text-align: left;
}
.text-align-right {
  text-align: right;
}
.text-align-center {
  text-align: center;
}
.text-align-justify {
  text-align: justify;
}

/**
 * Alignment classes (images, videos, blockquotes …).
 */
.align-left {
  float: left;
}
.align-right {
  float: right;
}
.align-center {
  display: block;
  margin-left: auto;
  margin-right: auto;
}

align.module.css

/*
 * Remove browser styles, especially for <buttons> and so on.
 */
.reset-appearance {
  -webkit-appearance: none;
  -moz-appearance: none;
  appearance: none;
  border: 0 none;
  background: transparent;
  padding: 0;
  margin: 0;
  line-height: inherit;
}

reset-appearance.module.css

/*
 * Contain positioned elements.
 */
.position-container {
  position: relative;
}

position-container.module.css

We should, but in the issues where we've been moving and splitting up files, we've been avoiding making changes. Just to keep it simple. We can create issues to clean up all of these files individually.

It looks like the patch is #59 is missing a few of the changes from #57 as it's 10kb less. I would recommend working from the patch in #57.

Thanks

1. +++ b/core/modules/system/system.libraries.yml
   @@ -3,9 +3,54 @@ base:
   +      css/ajax-progress.module.css: { every_page: true, weight: -10, }
   +      css/align.module.css: { every_page: true, weight: -10, }
   ...
   +      css/container-inline.module.css: { every_page: true, weight: -10, }
   +      css/clearfix.module.css: { every_page: true, weight: -10, }
   +      css/details.module.css: { every_page: true, weight: -10, }
   +      css/form--inline.module.css: { every_page: true, weight: -10, }
   +      css/hidden.module.css: { every_page: true, weight: -10, }
   +      css/js.module.css: { every_page: true, weight: -10, }
   +      css/nowrap.module.css: { every_page: true, weight: -10, }
   +      css/position-container.module.css: { every_page: true, weight: -10, }
   +      css/progress.module.css: { every_page: true, weight: -10, }
   +      css/reset-appearance.module.css: { every_page: true, weight: -10, }
   +      css/resize.module.css: { every_page: true, weight: -10, }
   

   Some files are in the css folder and the rest are in the css/components folder. Can we move them all into the components folder?

2. +++ b/core/modules/system/system.libraries.yml
   @@ -3,9 +3,54 @@ base:
   +      css/components/animated-throbber.module.css: {}
   +      css/components/fieldgroup.module.css: {}
   ...
   +      css/sticky-header.css: {}
   ...
   +      css/components/action-links.theme.css: {}
   +      css/components/breadcrumbs.theme.css: {}
   +      css/components/button.theme.css: {}
   

   There are loads of files that need the 'every_page true, weight: -10' options that the other files have

3. +++ b/core/modules/system/system.libraries.yml
   @@ -3,9 +3,54 @@ base:
          css/system.module.css: { every_page: true, weight: -10, }
   

   This file is deleted so we need to remove it from the yml file

4. +++ b/core/modules/system/system.libraries.yml
   @@ -3,9 +3,54 @@ base:
   +      css/components/tree-child.css: {}
   

   This file name should be tree-child.theme.css

Thanks!

Im not sure which files should have the 'every_page true, weight: -10' or if they should they all have as a default? Looking at some of the declarations in core like seven etc don't have it applied

They all should have them, because the CSS files we are replacing had them before. We want to ensure that these files are loaded before any other module CSS files.

I ran this through visual regression testing and I found that we need to update the background-image url() paths because all files are now one folder level deeper.

+++ b/core/modules/system/css/components/ajax-progress.module.css
@@ -0,0 +1,41 @@
+.ajax-progress-throbber .throbber {
+  background: transparent url(../../../misc/throbber-active.gif) no-repeat 0px center;

Eg. This path becomes url(../../../../misc/throbber-active.gif)

I made the changes and ran the visual regression testing again. I ran it using three separate lists of URLs just to be sure we had enough coverage.

The last one includes a lot of screenshots! It took about 30 seconds enough for the browser to print it as a PDF. I'll upload it separately.

25.6MB!

I tested the patch with csslint and found that we had more csslint errors than before. I'm investigating...

OK that took ages but I found a few CSS files that were duplicates and also a few lines that were in two files. Thanks CSSlint!

I also went through and cleaned up the @file comments. If someone could have a look over that would be great.

@davidhernandez++++

Thanks for the thorough review. I've been distracted by usability stuff. Rerolling this was tricky, but I think I caught the recent changes to system.theme.css and system.libraries.yml: #2472431: Do not load normalize.css in all themes, load it in Classy and #2501903: inline form errors classnames to follow namestandard

Most of your comments were simple fixes, here are a few comments for the less obvious ones.

More link has this technical detail. Can more help benefit from something similar, instead of the simple comment that is there?

more-help-link has been removed, so I deleted the file. See https://www.drupal.org/node/2462579

Do we really need separate files for all the little things? nowrap is in its own file. clear fix is its own file. There are a lot of files that have one thing in them. It would be worth separating if they will later be conditionally included, or there was a reason to remove them. Is it necessary for something like nowrap or all the others?

Yeah, the long term aim here is consistency and findability. It should always be obvious where to find the CSS for a class. That means each class (or CSS component) should get it's own file.

Why do we have two things that animate throbber? .ajax-progress and animater-throbber.module.css

Good point, I've renamed the animater-throbber.module.css to match the class closer. autocomplete-loading.module.css

The breadcrumb file name is plural but the class name is singular.

Good catch, renamed to match the class.

Yep sounds good to me

Ah good point.

@Wim Leers There isn't a follow up issue, but we did discuss at Drupalcon that this issue would enable more fine grained loading. We still have a lot of #2489460: [Meta] Move module.theme.css files to Classy or Seven to do. I don't know which release would be a candidate target for that.

WOOHOO! Amazing. Well done everyone. Now for the easy work ;-)

FYI this commit accidentally reverted #2486431: Progress bar starts at 100%