Problem/Motivation

When we started the development for Claro, we kept our post-processed and the source CSS assets in different subdirectories.
Right before Claro was committed to core, #3060153: Use PostCSS in core, initially only for Claro landed as well, and it uses a different approach for managing these assets by keeping the source and the post-processed CSS files in the same folder.

On my opinion, there is no reason anymore to keep the CSS files inside the src subfolder.

Proposed resolution

  • Move every CSS assets from ./css/scr/ to ./css/.
  • Refactor the url() references.

Remaining tasks

Patch, review.

User interface changes

Expected: nothing at all! (No broken images or pseudo content!)

API changes

Nothing.

Data model changes

Nothing.

Release notes snippet

8.8.0-alpha1 introduced the experimental Claro administrative theme, which uses PostCSS to compile versions of its CSS compatible with all of Drupal 8.8's supported browsers. In that release, the unprocessed CSS was in a src subdirectory, but this subdirectory was in the same directory as the compiled CSS. 8.8.0-rc1 simplifies this directory structure so that the sources and compiled CSS are in the same directory.<

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

huzooka created an issue. See original summary.

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Active » Needs review
FileSize
97.7 KB

I also removed some redundant images (except of the icons on the status report page).

huzooka’s picture

rename to core/themes/claro/css/components/menus-and-lists.css
index cf0c42a8d6..ec534f68d6 100644

index cf0c42a8d6..ec534f68d6 100644
--- a/core/themes/claro/css/src/components/menus-and-lists.css

--- a/core/themes/claro/css/src/components/menus-and-lists.css
+++ b/core/themes/claro/css/components/menus-and-lists.css

+++ b/core/themes/claro/css/components/menus-and-lists.css
+++ b/core/themes/claro/css/components/menus-and-lists.css
@@ -27,11 +27,17 @@

@@ -27,11 +27,17 @@
 .menu-item--collapsed {
   list-style-type: disc;
   list-style-image: url(../../../images/menu-collapsed.png);
...
+[dir="rtl"] .item-list ul li.collapsed,
+[dir="rtl"] .menu-item--collapsed {
+  list-style-image: url(../../../../misc/menu-collapsed-rtl.png);
 }

Note: these RTL styles are still missing from Seven theme.

huzooka’s picture

Assigned: Unassigned » huzooka
Status: Needs review » Needs work
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
98.07 KB
8.44 KB

Status: Needs review » Needs work

The last submitted patch, 6: claro-simplify_css_dir-3088805-6.patch, failed testing. View results

martin107’s picture

A little fix.

fhaeberle’s picture

+1

huzooka’s picture

Just asked @lauriii why we test our elements.css specifically.

The answer is:
> It was the first asset listed in global-styling library.

I'd add this comment as well.

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

lauriii’s picture

Status: Needs review » Needs work
+++ b/core/tests/Drupal/FunctionalTests/Theme/ClaroTest.php
@@ -43,6 +43,8 @@ public function setUp() {
+    // Testing that the global library is attached.
+    // The 'elements.css' file is the first asset of the global library.

I'm not sure we actually need this inline documentation. Updating the method documentation to be more specific on what we're testing should be sufficient.

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
99.41 KB
1.39 KB
huzooka’s picture

Assigned: Unassigned » huzooka
Status: Needs review » Needs work
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
99.33 KB
774 bytes

Status: Needs review » Needs work

The last submitted patch, 17: claro-simplify_css_dir-3088805-17.patch, failed testing. View results

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
99.16 KB
1.11 KB
lauriii’s picture

+++ /dev/null
@@ -1,8 +0,0 @@
diff --git a/core/themes/claro/images/core/cccccc/clock.svg b/core/themes/claro/images/icons/cccccc/clock.svg

similarity index 100%
rename from core/themes/claro/images/core/cccccc/clock.svg

rename from core/themes/claro/images/core/cccccc/clock.svg
rename to core/themes/claro/images/icons/cccccc/clock.svg

--- /dev/null
+++ b/core/themes/claro/images/icons/cccccc/d8-logo.svg

+++ b/core/themes/claro/images/icons/cccccc/d8-logo.svg
@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="47.411" height="53.531" viewBox="0 0 47.411 53.531">
+  <circle fill="#CCC" cx="22.308" cy="41.593" r="8.449"/>
+  <path fill="#CCC" d="M32.813 31.532c2.503 2.614 4.044 6.156 4.044 10.06 0 4.945-2.47 9.31-6.24 11.94 6.97-2.15 12.733-7.388 15.314-13.73 3.57-8.776.247-15.38-5.33-21.37.17.776.264 1.58.264 2.406 0 5.078-3.405 9.36-8.05 10.694z"/>
+  <circle fill="#CCC" cx="29.735" cy="20.838" r="6.463"/>
+  <path fill="#CCC" d="M11.178 50.96c-2.134-2.53-3.42-5.798-3.42-9.368 0-7.448 5.598-13.584 12.814-14.442-1.238-1.794-1.965-3.968-1.965-6.312 0-6.145 4.982-11.128 11.13-11.128.507 0 1.006.037 1.495.103C27.59 6.67 23.957 3.483 21.09 0 22.553 15.257 7.192 9.713 1.514 23.773c-3.81 9.433-.376 21.095 9.663 27.188z"/>
+</svg>

--- /dev/null
+++ b/core/themes/claro/images/icons/cccccc/database.svg

+++ b/core/themes/claro/images/icons/cccccc/database.svg
@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="38.847" height="44.262" viewBox="0 0 38.847 44.262">
+  <path fill="#CCC" d="M19.745 0c5.74.123 12.272.953 16.9 4.668 1.865 1.498 2.786 3.91 1.597 6.126-1.255 2.34-4.13 3.733-6.518 4.6-5.63 2.04-12.113 2.38-18.014 1.573-2.92-.4-5.91-1.103-8.58-2.374C2.94 13.553.39 11.788.037 9.19c-.532-3.925 4.23-6.23 7.264-7.3C11.287.482 15.54.037 19.745 0c4.302.092-3.334.03 0 0z"/>
+  <path fill="#CCC" d="M6.76 17.5c3.702 1.427 7.65 1.972 11.6 2.09 4.058.12 8.107-.424 12.023-1.523 4.227-1.186 7.227-3.624 8.463-6.145v5.965c-.076.524-.197 1.028-.384 1.5-.718 1.81-2.594 2.974-4.235 3.848-4.293 2.286-9.5 3.04-14.31 3.083-4.803.043-9.902-.542-14.3-2.575-1.906-.88-3.9-2.02-4.988-3.887-.66-1.135-.626-2.21-.626-3.486v-4.38c1.232 2.64 3.94 4.422 6.757 5.51z"/>
+  <path fill="#CCC" d="M6.76 26.436c3.702 1.428 7.65 1.973 11.6 2.09 4.058.12 8.107-.423 12.023-1.522 4.227-1.186 7.227-3.624 8.463-6.145v5.964c-.076.524-.197 1.028-.384 1.5-.718 1.81-2.594 2.974-4.235 3.848-4.293 2.286-9.5 3.04-14.31 3.083-4.803.043-9.902-.542-14.3-2.575-1.906-.88-3.9-2.02-4.988-3.887-.66-1.135-.626-2.21-.626-3.486v-4.38c1.232 2.64 3.94 4.422 6.757 5.51z"/>
+  <path fill="#CCC" d="M6.76 35.374c3.702 1.428 7.65 1.973 11.6 2.09 4.058.12 8.107-.423 12.023-1.522 4.227-1.186 7.227-3.624 8.463-6.145v5.965c-.076.524-.197 1.028-.384 1.5-.718 1.81-2.594 2.974-4.235 3.848-4.293 2.286-9.5 3.04-14.31 3.083-4.803.043-9.902-.542-14.3-2.575-1.906-.88-3.9-2.02-4.988-3.887-.66-1.134-.626-2.21-.626-3.485v-4.38c1.232 2.64 3.94 4.422 6.757 5.51z"/>
+</svg>

--- /dev/null
+++ b/core/themes/claro/images/icons/cccccc/php-logo.svg

+++ b/core/themes/claro/images/icons/cccccc/php-logo.svg
@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="66" height="33.447" viewBox="0 0 66 33.447">
+  <g fill="#CCC">
+    <path d="M49.5 12.255h-2.7l-1.473 7h2.4c1.59 0 2.773-.342 3.55-.94.78-.6 1.304-1.62 1.577-3.023.26-1.345.142-1.975-.357-2.528-.5-.553-1.498-.51-2.996-.51z"/>
+    <path d="M33 0C14.775 0 0 7.488 0 16.724s14.775 16.724 33 16.724 33-7.488 33-16.724S51.225 0 33 0zm-9.328 19.982c-.787.737-1.662 1.376-2.625 1.69-.963.313-2.19.583-3.68.583H13.99l-.935 5H9.11l3.52-18h7.584c2.28 0 3.946.34 4.992 1.537 1.046 1.197 1.36 2.74.944 4.885-.172.884-.462 1.628-.87 2.36-.413.732-.947 1.338-1.608 1.945zm11.51 2.273l1.558-8.124c.177-.91.112-1.29-.196-1.62-.308-.33-.962-.255-1.963-.255h-3.126l-2.016 10h-3.913l3.52-18h3.912l-.935 5h3.486c2.193 0 3.706.124 4.54.888.832.765 1.08 1.99.748 3.703l-1.637 8.41h-3.977zm21.747-6.708c-.173.884-.463 1.692-.872 2.424-.41.734-.944 1.404-1.605 2.01-.787.738-1.662 1.377-2.625 1.69-.963.314-2.19.584-3.68.584h-3.377l-.934 5h-3.944l3.518-18h7.584c2.282 0 3.946.34 4.992 1.537 1.046 1.2 1.36 2.61.943 4.757z"/>
+    <path d="M18.72 12.255h-2.703l-1.473 7h2.4c1.59 0 2.773-.342 3.552-.94.778-.6 1.303-1.62 1.576-3.023.26-1.345.142-1.975-.357-2.528-.5-.553-1.497-.51-2.996-.51z"/>
+  </g>
+</svg>
diff --git a/core/themes/claro/images/core/cccccc/server.svg b/core/themes/claro/images/icons/cccccc/server.svg

similarity index 100%
rename from core/themes/claro/images/core/cccccc/server.svg

rename from core/themes/claro/images/core/cccccc/server.svg
rename to core/themes/claro/images/icons/cccccc/server.svg

What are these additions? Shouldn't we be using icons from core for these?

huzooka’s picture

Re #21:
These are only available in Seven theme.

huzooka’s picture

xjm’s picture

@lauriii and I discussed separating the SVG improvements from simply moving the files around since these two changes require different review contexts.

We also discussed that this issue is probably OK to backport to 8.8.x during the beta phase since Claro is internal code; however, it's probably worth finishing before 8.8.0-rc1 if we're going to make the change.

huzooka’s picture

I definitely think that this is finished.

huzooka’s picture

Assigned: Unassigned » huzooka
Status: Needs review » Needs work

From @lauriii on slack:

I talked about the simplify Claro folder structure with Jess (xjm)
she recommended that we split the icons clean up to a separate issue

huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
79.83 KB
14.32 KB
huzooka’s picture

lauriii’s picture

Status: Needs review » Needs work
+++ b/core/themes/claro/css/components/details.css
@@ -699,7 +699,7 @@ rgba(0, 0, 0, 0.1);
-  background-image: url(../../../images/core/ee0000/required.svg);
+  background-image: url(../../../../misc/icons/ee0000/required.svg);

+++ b/core/themes/claro/css/components/details.pcss.css
@@ -562,7 +562,7 @@
-  background-image: url(../../../images/core/ee0000/required.svg);
+  background-image: url(../../../../misc/icons/ee0000/required.svg);

+++ b/core/themes/claro/css/components/dialog.css
@@ -73,7 +73,7 @@
-  background: url(../../../images/core/ffffff/ex.svg) 0 0 no-repeat;
+  background: url(../../../../misc/icons/ffffff/ex.svg) 0 0 no-repeat;

+++ b/core/themes/claro/css/components/dialog.pcss.css
@@ -57,7 +57,7 @@
-  background: url(../../../images/core/ffffff/ex.svg) 0 0 no-repeat;
+  background: url(../../../../misc/icons/ffffff/ex.svg) 0 0 no-repeat;

+++ b/core/themes/claro/css/components/form.css
@@ -160,7 +160,7 @@ tr .form-item,
-  background-image: url(../../../images/core/ee0000/required.svg);
+  background-image: url(../../../../misc/icons/ee0000/required.svg);

+++ b/core/themes/claro/css/components/form.pcss.css
@@ -77,7 +77,7 @@ tr .form-item,
-  background-image: url(../../../images/core/ee0000/required.svg);
+  background-image: url(../../../../misc/icons/ee0000/required.svg);

+++ b/core/themes/claro/css/components/system-admin--modules.css
@@ -140,7 +140,7 @@
-  background: url(../../../images/core/787878/questionmark-disc.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/questionmark-disc.svg) 0 50% no-repeat; /* LTR */

@@ -148,7 +148,7 @@
-  background: url(../../../images/core/787878/key.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/key.svg) 0 50% no-repeat; /* LTR */

@@ -156,7 +156,7 @@
-  background: url(../../../images/core/787878/cog.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/cog.svg) 0 50% no-repeat; /* LTR */

+++ b/core/themes/claro/css/components/system-admin--modules.pcss.css
@@ -106,19 +106,19 @@
-  background: url(../../../images/core/787878/questionmark-disc.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/questionmark-disc.svg) 0 50% no-repeat; /* LTR */
...
-  background: url(../../../images/core/787878/key.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/key.svg) 0 50% no-repeat; /* LTR */
...
-  background: url(../../../images/core/787878/cog.svg) 0 50% no-repeat; /* LTR */
+  background: url(../../../../misc/icons/787878/cog.svg) 0 50% no-repeat; /* LTR */

+++ b/core/themes/claro/css/components/system-admin--status-report.css
@@ -50,11 +50,11 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);

+++ b/core/themes/claro/css/components/system-admin--status-report.pcss.css
@@ -37,10 +37,10 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);

+++ b/core/themes/claro/css/components/system-status-counter.css
@@ -53,15 +53,15 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);
...
-  background-image: url(../../../images/core/73b355/check.svg);
+  background-image: url(../../../../misc/icons/73b355/check.svg);

+++ b/core/themes/claro/css/components/system-status-counter.pcss.css
@@ -39,13 +39,13 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);
...
-  background-image: url(../../../images/core/73b355/check.svg);
+  background-image: url(../../../../misc/icons/73b355/check.svg);

+++ b/core/themes/claro/css/components/system-status-report.css
@@ -120,12 +120,12 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);

+++ b/core/themes/claro/css/components/system-status-report.pcss.css
@@ -93,11 +93,11 @@
-  background-image: url(../../../images/core/e32700/error.svg);
+  background-image: url(../../../../misc/icons/e32700/error.svg);
...
-  background-image: url(../../../images/core/e29700/warning.svg);
+  background-image: url(../../../../misc/icons/e29700/warning.svg);

+++ b/core/themes/claro/css/theme/ckeditor-dialog.css
@@ -115,7 +115,7 @@
-  background: url(../../../images/core/ffffff/ex.svg) 0 0 no-repeat;
+  background: url(../../../../misc/icons/ffffff/ex.svg) 0 0 no-repeat;

@@ -191,7 +191,7 @@
-  background: #fcfcfa url(../../../images/core/333333/caret-down.svg) no-repeat 99% 63%;
+  background: #fcfcfa url(../../../../misc/icons/333333/caret-down.svg) no-repeat 99% 63%;

+++ b/core/themes/claro/css/theme/ckeditor-dialog.pcss.css
@@ -58,7 +58,7 @@
-  background: url(../../../images/core/ffffff/ex.svg) 0 0 no-repeat;
+  background: url(../../../../misc/icons/ffffff/ex.svg) 0 0 no-repeat;

@@ -116,7 +116,7 @@
-  background: #fcfcfa url(../../../images/core/333333/caret-down.svg) no-repeat 99% 63%;
+  background: #fcfcfa url(../../../../misc/icons/333333/caret-down.svg) no-repeat 99% 63%;

Thank you, this is already much easier to review! Let's move these changes to #3092984: Clean up Claro's unused images as well.

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
79.39 KB
19.08 KB

Broken image references:

  1. In core/themes/claro/css/components/jquery.ui/theme.pcss.css: the background image of 
.ui-progressbar .ui-progressbar-value was broken (and I cannot find the originally referenced asset)
  2. In core/themes/claro/css/components/menus-and-lists.pcss.css: the background of the collapsed/expanded menu list items were also broken and the collapsed state misses the RTL style
lauriii’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed that we're only making changes to the directory structure by using git diff --color-words=.. I also grepped all url(../ instances and manually confirmed they are pointing to a valid path. Everything looks good now! 👏

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed f1e9b8a and pushed to 9.0.x and cherry-picked it to 8.9.x and 8.8.x as @xjm's +1 on backport. Thanks!

  • alexpott committed f1e9b8a on 9.0.x
    Issue #3088805 by huzooka, martin107, lauriii, xjm: Simplify Claro's CSS...

  • alexpott committed ea9d7ae on 8.9.x
    Issue #3088805 by huzooka, martin107, lauriii, xjm: Simplify Claro's CSS...

  • alexpott committed ecf8e5f on 8.8.x
    Issue #3088805 by huzooka, martin107, lauriii, xjm: Simplify Claro's CSS...
xjm’s picture

Issue tags: +8.8.0 release notes

This should go in the RC release notes since it's a semi-disruptive change from beta1. (It doesn't need to go in the 8.8.0 notes since the theme didn't exist in 8.7.)

xjm’s picture

Issue summary: View changes

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.