Color module doesn't look for CSS files declared in a library

Will check this one...

Any thoughts on how to fix this one?

+++ b/core/modules/color/color.module
@@ -90,6 +90,33 @@ function color_css_alter(&$css) {
+  ¶

Gave me a whitespace error. New patch added.

@nod_: What kind of tests does it need? Manual testing or ... If I could help, give a shout! :)

@nod_: Great, thanks. I'll try this out, assigning to myself.

Any progress on this, sqndr? It would be nice to get this in so we can get stylesheets out of info.yml and just have them in libraries.yml.

@mdrummond: No progress so far. You can pick this up if you want.

I have added a new library to color_test_theme and declared a css file from that library to the color module. It seems that current tests are checking file rewriting ( 'Make sure the color stylesheet is included in the content' assertion').
Also list_themes() function is deprecated now. So i use \Drupal::service('theme_handler')->listInfo() in patch.
Should we create a new issue about replacing list_themes() function occurrences ?

Part of the reason for this patch is so that theme info.yml files don't have stylesheet attachments inside them anymore. So it would be nice if this patch pulled those declarations out of info.yml. Alternatively that could be handled in a follow-up, but the todo points at this issue for that task.

Other things I spotted:

1. +++ b/core/modules/color/color.module
   @@ -91,6 +91,36 @@ function color_css_alter(&$css) {
   +  global $theme_key;
   

   Is this the way this is being handled elsewhere? Using a global like this doesn't feel like a very d8 way to do this.

2. +++ b/core/modules/color/src/Tests/ColorTest.php
   @@ -66,8 +66,8 @@ function setUp() {
   -        'scheme_color' => '#3b3b3b',
   ...
   +        'scheme_color' => '#3c3c3c',
   
   +++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/color/color.inc
   @@ -8,13 +8,25 @@
   +        'text' => '#3b3b3b',
   ...
   +        'text' => '#3c3c3c',
   
   +++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/css/colors.css
   @@ -0,0 +1,7 @@
   +    color: #3b3b3b;
   

   Why are some of these 3b3b3b and some 3c3c3c?

3. +++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/color_test_theme.info.yml
   @@ -3,3 +3,5 @@ type: theme
   \ No newline at end of file
   
   +++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/color_test_theme.libraries.yml
   @@ -0,0 +1,5 @@
   \ No newline at end of file
   
   +++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/css/colors.css
   @@ -0,0 +1,7 @@
   \ No newline at end of file
   

   Missing newlines at the ends of these files.

#17.1, #2228093: Modernize theme initialization *just* went in, so see https://www.drupal.org/node/2324935

Here's a patch - and an interdiff with #14.

Not sure if you meant that tag? :p

Haha - brilliant. Sorry 'bout that :D

+++ b/core/modules/color/src/Tests/ColorTest.php
@@ -66,8 +66,8 @@ protected function setUp() {
+        'scheme_color' => '#3c3c3c',

+++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/color/color.inc
@@ -8,13 +8,25 @@
+        'text' => '#3b3b3b',
...
+        'text' => '#3c3c3c',

We still have one for 3b3b3b and two for 3c3c3c. Maybe there is something going on there that I don't understand. But if not, can we get these consistent?

Part of the point of this is that it allows us to no longer have stylesheet declarations in a theme info.yml file. Guessing we will handle that in a separate issue? The @todo points to this issue for that.

+++ b/core/modules/color/color.module
@@ -93,6 +93,36 @@ function color_css_alter(&$css) {
+  $theme_key = \Drupal::theme()->getActiveTheme()->getName();
+  $themes = \Drupal::service('theme_handler')->listInfo();
+  $theme_libraries = $themes[$theme_key]->info['libraries'];

No reason to disturb theme_handler

$active_theme = \Drupal::theme()->getActiveTheme();
$theme_libraries = $active_theme->getLibraries();

1. RE: the first bit of #25, regarding the confusing colors: it took me trial & error and debugging to understand what's going on. I've changed the colors a bit to hopefully make it easier to understand what's going on. I doubt ColorTest worked as intended before sqndr's changes — it didn't actually test anything for color_test_theme — fortunately, it did test Bartik correctly.
2. Like the last bit of #25 says, the original goal of this patch was to allow for CSS to be defined in a library, and the theme's info YML file to have a libraries key. That worked, but Color module didn't support that yet.
   So, restoring the second hunk from #4 to do that again, and things work wonderfully well for Bartik :)
3. #26 fixed.
4. The latest reroll of this patch lost some of the earlier hunks: see interdiff-lost-hunks.txt.

The test-only patch should have two failures: one for Bartik, one for the test theme.

It would be nice to get this in so we can get stylesheets out of info.yml and just have them in libraries.yml.

+1!

Bumping to major because this is a blocker to shipping with Bartik's assets in a libraries.yml file, and hence it prevents a best practice from being visible in Drupal 8's only front-end theme.

Heh :)

+1 zillion to this.

Theme sanity, yay!

Committed ce7fefd and pushed to 8.0.x. Thanks!

#2387535: Color module can't replace CSS files loaded via a library in hook_page_attachments_alter()

there was only one issue (this one with Developer Experience tag. I think DX (Developer Experience) is the one being used mostly.