move file.theme.css to classy

There are three situations where we are adding the css file through a library, we need to remove them and make sure we are loading these files in the right places in Classy.

core/modules/file/src/Element/ManagedFile.php:
   46        '#extended' => FALSE,
   47        '#attached' => [
   48:         'library' => ['file/drupal.file'],
   49        ],
   50      ];

core/modules/file/src/Plugin/Field/FieldFormatter/GenericFileFormatter.php:
   50      if (!empty($elements)) {
   51        $elements['#attached'] = array(
   52:         'library' => array('file/drupal.file.formatter.generic'),
   53        );
   54      }

core/modules/file/src/Plugin/Field/FieldType/FileItem.php:
  114      $element = array();
  115  
  116:     $element['#attached']['library'][] = 'file/drupal.file';
  117  
  118      $element['display_field'] = array(

Done with the changes mentioned in #3. Please verify.

+++ b/core/modules/file/src/Plugin/Field/FieldFormatter/GenericFileFormatter.php
@@ -48,9 +48,6 @@ public function viewElements(FieldItemListInterface $items) {
-      $elements['#attached'] = array(
-        'library' => array('file/drupal.file.formatter.generic'),
-      );

I don't think this can be removed since it's a different library?

Update #4 patch.
@LewisNyman I think we dont need remove 'library' => ['file/drupal.file'] and $element['#attached']['library'][] = 'file/drupal.file'
Because in drupal.file used file.js and dependencies to core/jquery and etc.
We need remove only component:css/file.theme.css: {}.
Вut we can remove drupal.file.formatter.generic: and include in classy in template file-managed-file.html.twig.
Also I moved icons to classy. Tested and add screenshots.

I have some error in patch #8. Update patch.

Trying fix patch #13. Include style to file-link.html.twig. And remove from include style field.html.twig.

Here is a new version of the patch. I moved the file icons within a separate folder. Having them into the root dir of classy seemed so wrong.

I could not find any differences via visual regression testing after applying the patch.

I attached the json config file for the visual regression testing I did. I used the drupalcore-frontend-toolkit to create the screenshots. See here: https://github.com/lewisnyman/drupalcore-frontend-toolkit/pull/15

Great. Thanks for the work here. This is the first issue where we have added image to Classy, we need a review from a Classy maintainer to ensure that the folder structure is correct.

+++ b/core/themes/classy/classy.libraries.yml
@@ -14,10 +14,16 @@ drupal.comment.threaded:
\ No newline at end of file

New line needed to the end of file

Hmm. Something's not right here. Right now the Book CSS was added into a "navigation" folder, matching the organization of the templates folder. But comment and search were not. They were added in folders that matched the module they came from. We need one or the other.

Also, the files are being moved with *.theme in the name. I thought we were going to remove that since we aren't going to move any *.module files to Classy so there is no need for the differentiation.

@morten, I think we did have that conversation. So that is right. But at this point, with the amount of time that has passed and how close we are to RC, I think we need to start eliminating the two step processes. There is no guarantee we'll get follow ups in, so we should decide on naming and folder structure, and have these patches go in that way.

Ok that sounds good, it makes sense to remove the .theme. stuff in Classy because it's the same convention we follow in Seven and Bartik.

Ok, I've renamed the folder structure to match the templates and I've rename the file to file.css. I've also renamed the folder to icons, which feels more appropriate if we want to put more stuff in there.

Should these just go in a components folder? My concern is about the stuff we will move that is not directly tied to a template. What will we do with all the system CSS we just split? Are we going to move that all over as the "components" folder and leave the other CSS in template-based folders? That would seem inconsistent with how we organize the other themes, which is SMACSS-based.

@davidhernandez I was also thinking about this today and realised that splitting the CSS in to separate folders really harms findability. One of the benefits of having one component per file is it's really easy to find the file when they are sorted alphabetically. This makes it a lot harder.

TLDR I agree and will write a new patch to move the CSS file structure inline with Seven and Bartik

Thanks, this just needs manual testing to make sure the new patch hasn't introduced regressions and I haven't messed up the image paths :)

Patch is working fine tested on my system.

Thank you!

+++ b/core/themes/classy/classy.libraries.yml
@@ -8,16 +8,22 @@ book-navigation:
-      css/navigation/book.theme.css: {}
+      css/components/book.theme.css: {}
...
-      css/comment/comment.theme.css: {}
+      css/components/comment.theme.css: {}
...
-      css/search/search.theme.css: {}
+      css/components/search.theme.css: {}

Can someone explain why is these changes in this patch?

We agreed to put the css files into the components folder, so maybe Lewis was testing it and included it by accident? Even if we tried to sneak it in :) the names should be changed too. Anyway, lets make a separate issue for fixing that.

Reroll.

This is fixing the file names. I didn't split off the the file renames for search, comment and book into another issue. If someone wants to open that, it would be straightforward.

Or.... just change the issue title. :)

+++ b/core/themes/classy/classy.libraries.yml
@@ -26,4 +26,10 @@ search.results:
-      css/search/search.theme.css: {}
+      css/components/search.css: {}
...
diff --git a/core/themes/classy/css/navigation/book.theme.css b/core/themes/classy/css/components/book.css

similarity index 100%
rename from core/themes/classy/css/navigation/book.theme.css

rename from core/themes/classy/css/navigation/book.theme.css
rename to core/themes/classy/css/components/book.css

rename to core/themes/classy/css/components/book.css
diff --git a/core/themes/classy/css/comment/comment.theme.css b/core/themes/classy/css/components/comment.css

diff --git a/core/themes/classy/css/comment/comment.theme.css b/core/themes/classy/css/components/comment.css
similarity index 100%

similarity index 100%
rename from core/themes/classy/css/comment/comment.theme.css

rename from core/themes/classy/css/comment/comment.theme.css
rename to core/themes/classy/css/components/comment.css

We still have unrelated changes in the patch :)

I have removed any changes relating to book, comment and search.

Patch and interdiff attached.

Thanks for the fix!

+++ b/core/themes/classy/classy.libraries.yml
@@ -27,3 +27,9 @@ search.results:
+drupal.file:

s/drupal.file/file

You could also take a look on the Git configuration documentation to make reviewing of your patches that do moving of files a little easier :)

The only

+++ b/core/themes/classy/classy.libraries.yml
@@ -27,3 +27,9 @@ search.results:
+drupal.file:

I wonder if we can just call this library file? The drupal part seem redundant.

Yes, no drupal.* please. I don't think it is necessary for any of the libraries.

Quite clear in #41 / #42 / #43 what to do next.

This should fix the remaining concerns.

+++ b/core/themes/classy/classy.libraries.yml
@@ -27,3 +27,9 @@ search.results:
+    theme:
+      css/components/file.css: {}

We've just been having this whole component/theme discussion in other issues. These should be component files.

Thanks for the interdiff

Needs a reroll.

rerolled.

This looks good, setting to RTBC/postponed on #2566771: [Voltron patch] Move all remaining *.theme.css to Classy