Create reusable color classes

Lets implement our:

Yellow
Green
Red

Color scheme

I have added colors.css to Seven for the helper classes.

As a proof of concept I have amended the Status report table to now use the new color classes.
I also noticed that the info class does not have any css associated with it and we have deprecated the ok green styling so I removed those two from the array. Also as a result I needed to add an if statement to the twig file to print 'class' when there is one present for the row.

Rerolled the patch in comment #5

Fixed wrong filename (color.css -> colors.css)

Hope to get patch names right this time.

It passes tests on my local machine, requeing.

Fixed css comments.

Fixed the test.

It still needs screenshots.

Putting back for the screenshots :)

Now it's fixed. Icons and colors appear correctly.

BEFORE

AFTER

Tested this manually and fixed a very minor bug on the code. I will RTBC this because its relatively simple change. I also updated the Issue summary to match the current solution.

1. +++ b/core/modules/system/system.admin.inc
   @@ -170,10 +170,12 @@ function template_preprocess_status_report(&$variables) {
   +      'class' => 'color-warning',
   ...
   +      'class' => 'color-error',
   

   I thought we were trying to not put classes in code.

2. +++ b/core/modules/system/system.admin.inc
   @@ -193,6 +195,7 @@ function template_preprocess_status_report(&$variables) {
   +    $variables['requirements'][$i]['severity_class'] = isset($severity['class']) ? $severity['class'] : NULL;
   
   +++ b/core/modules/system/templates/status-report.html.twig
   @@ -25,20 +25,24 @@
   -    <tr class="{{ requirement.severity_status }}">
   -      <td class="status-icon">
   -        <div title="{{ requirement.severity_title }}">
   -          <span class="visually-hidden">{{ requirement.severity_title }}</span>
   -        </div>
   -      </td>
   -      <td class="status-title">{{ requirement.title }}</td>
   -      <td class="status-value">
   -        {{ requirement.value }}
   -        {% if requirement.description %}
   -          <div class="description">{{ requirement.description }}</div>
   -        {% endif %}
   -      </td>
   -    </tr>
   +    {% if requirement.severity_class %}
   +      <tr class="{{ requirement.severity_class }}">
   +    {% else %}
   +      <tr>
   +    {% endif %}
   +        <td class="status-icon">
   +          <div title="{{ requirement.severity_title }}">
   +            <span class="visually-hidden">{{ requirement.severity_title }}</span>
   +          </div>
   +        </td>
   +        <td class="status-title">{{ requirement.title }}</td>
   +        <td class="status-value">
   +          {{ requirement.value }}
   +          {% if requirement.description %}
   +            <div class="description">{{ requirement.description }}</div>
   +          {% endif %}
   +        </td>
   +      </tr>
   

   So I guess this is fixing an out of scope bug. Please open a new issue - this is a normal task and the issue summary just does not cover this.

I guess we need the change 2. because we dont want to print class for all the severities. We could also use Attribute for this case but for me it seems bigger change than this.

Thats a good idea. I did the monkey job!

We still have CSS like this from system.theme.css:

.messages--status,
.ok {
  color: #325e1c;
}

Obviously without something like Sass, we'll have some amount of duplication, but should we include a comment in cases like this pointing to the new colors.css?

How does this change play out with #2227401: Apply the seven style guide to the status report?

Rerolled the patch. Everything looked good. Someone could double check this. This looks like RTBC to me.<

Tested the patch and it works!

On the admin/reports/status I see the color-error and color-warning classes. On div.messages I would expect the same color classes. Why are the color classes not used on div.messages? So you would have "messages messages--status color-success" instead of "messages messages--status"?

Thanks a lot for your time on reviewing & testing this issue @DomoSapiens! We are going to do it in follow-up. In this issue we are only doing this for the admin page just as an example. Are you happy with that? :)

Yep. I think it's a good idea to do it as a follow up, so it's consistent on all placed.

The patch no longer applies, it needs a reroll.

The patch applies cleanly.
When I go to the Status report page and inspect the warnings and error I see the following:

The error table row has the class .color-error and only receives it's colour styles from this selector.

The warning table row has the .color-warning and only receives it's colour styles from this selector.

Setting this to RTBC.

+++ b/core/modules/system/templates/authorize-report.html.twig
@@ -0,0 +1,23 @@
+{#
+/**
+ * @file
+ * Default theme implementation for authorize.php operation report templates.
+ *
+ * This report displays the results of an operation run via authorize.php.
+ *
+ * Available variables:
+ * - report: A list of result messages.
+ * - attributes: HTML attributes for the element.
+ *
+ * @see template_preprocess_authorize_report()
+ *
+ * @ingroup themeable
+ */
+#}
+{% if report %}
+  <div{{ attributes }}>
+    {% for messages in report %}
+      {{ messages }}
+    {% endfor %}
+  </div>
+{% endif %}

This came in #44 not sure why.

My fault again :P

Last patch might be confusing because its not what is should be. Here's the right one

+++ b/core/modules/system/css/system.theme.css
@@ -530,8 +530,10 @@ ul.tabs {
+/* See .color-success in Seven's colors.css */

@@ -541,33 +543,24 @@ ul.tabs {
+/* See .color-warning in Seven's colors.css */
...
+/* See .color-error in Seven's colors.css */

+++ b/core/themes/seven/css/components/tables.css
@@ -37,6 +37,16 @@ tbody tr:hover,
+/* See colors.css */

Not too sure about these comments. If anything they should be /* @see colors.css*/. But referencing a theme from system's css seems the wrong direction.

Could we use /* @see colors.css*/ in the files and reference Seven styling guide on the colors.css? I think that simpler comment full fills most of the use cases on such comments.

Setting back to RTBC after very minor comment changes.

The @see format is not quite right. https://www.drupal.org/coding-standards/docs#see

Looking at https://www.drupal.org/coding-standards/docs#see should it be...

   * @see color-success
   * in Seven's colors.css

So just the classname after the @see as it uses that as a link and the rest of the instructions on the next line?

I don't know how it should be but it cannot be that because coding standards says:

Each @see reference is on its own line, with no additional text beyond the item being referenced. To provide more explanation, just use a regular documentation paragraph.

So I have no idea.

Yeah this sucks. I was wrong to worry about this - sorry. Let's go back to #58. Can someone re-upload so it is the latest patch on the issue.

CSS change is not frozen in beta. Committed 6c991e8 and pushed to 8.0.x. Thanks!