messages: clean up messages and errors containers

Refactor the messages container to make use of message.tags [1] which
allows for more customization for each level (e.g. success, warning,
error, etc.) of a message through CSS selectors. As basic proof of
concept, customize the text color of each existing message level. Also,
this addition resolves a TODO by stephenfin in the previous code.

Move the errors container after the messages container in the DOM in the
base.html template so that every template can share the same errors
container. Also, add a background color to the errors container so that
both containers blend in as a uniform block. Add bullet points to each
error item in the list of errors.

Change both the messages and errors containers to always exist in
the DOM. With this, the addition of update and error messages is simpler
because it eliminates the need to create the containers if they don't
exist. These changes will be useful in a following patch that introduces
an internal JS module to make client-side requests to the REST API.

[1] https://docs.djangoproject.com/en/3.2/ref/contrib/messages/#message-tags

Signed-off-by: Raxel Gutierrez <raxel@google.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
[dja: converted HTML comment to django template comment, thanks sfin]
Signed-off-by: Daniel Axtens <dja@axtens.net>

diff --git a/htdocs/css/style.css b/htdocs/css/style.css
index 46a91ee84601e550f209813a0ab7fb7b0b756409..f30988e0bcaa2ee9f5997023024df3b8ab984bad 100644 (file)
--- a/htdocs/css/style.css
+++ b/htdocs/css/style.css
@@ -1,3 +1,9 @@
+:root {
+    --success-color:rgb(92, 184, 92);
+    --warning-color:rgb(240, 173, 78);
+    --danger-color:rgb(217, 83, 79);
+}
+
 h2 {
     font-size: 25px;
     margin: 18px 0 18px 0;
@@ -78,14 +84,27 @@ dl dt {
 }
 
 /* messages */
-#messages {
+.messages {
     background: #e0e0f0;
-    margin: 0.5em 1em 0.0em 0.5em;
+    margin: 0.5em 1em 0.0em 1em;
     padding: 0.3em;
+    list-style-type: none;
+}
+
+.messages:empty {
+    display: none;
+}
+
+.messages .success {
+    color: var(--success-color);
+}
+
+.messages .warning {
+    color: var(--warning-color);
 }
 
-#messages .message {
-    color: green;
+.messages .error {
+    color: var(--danger-color);
 }
 
 .filters {
@@ -421,13 +440,17 @@ table.loginform {
 }
 
 /* form errors */
-.errorlist {
-    color: red;
-    list-style-type: none;
-    padding-left: 0.2em;
-    margin: 0em;
+#errors {
+    background: #e0e0f0;
+    margin: 0em 1em 0.5em 1em;
+    padding: 0.3em;
 }
-.error {
+
+#errors:empty {
+    display: none;
+}
+
+.error-list, .errorlist {
     color: red;
 }
 

diff --git a/patchwork/templates/patchwork/list.html b/patchwork/templates/patchwork/list.html
index 5d3d82aa198e1d8cb0170af5b9307eb1bfbb4fae..6efbed2617afabd0b8a94191ab21a69821f823eb 100644 (file)
--- a/patchwork/templates/patchwork/list.html
+++ b/patchwork/templates/patchwork/list.html
@@ -8,16 +8,6 @@
 
 {% block body %}
 
-{% if errors %}
-<p>The following error{{ errors|length|pluralize:" was,s were" }} encountered
-while updating patches:</p>
-<ul class="errorlist">
-{% for error in errors %}
- <li>{{ error }}</li>
-{% endfor %}
-</ul>
-{% endif %}
-
 {% include "patchwork/partials/patch-list.html" %}
 
 {% endblock %}

diff --git a/patchwork/templates/patchwork/user-link-confirm.html b/patchwork/templates/patchwork/user-link-confirm.html
index 79678f64aa71f84dc73d279cef26965e15877e09..a6d671f3266fcc118a3a344142548402ba6ee0d0 100644 (file)
--- a/patchwork/templates/patchwork/user-link-confirm.html
+++ b/patchwork/templates/patchwork/user-link-confirm.html
@@ -5,12 +5,9 @@
 
 {% block body %}
 
-{% if errors %}
-<p>{{ errors }}</p>
-{% else %}
+{% if not errors %}
  <p>You have successfully linked the email address {{ person.email }} to
   your Patchwork account</p>
-
 {% endif %}
 <p>Back to <a href="{% url 'user-profile' %}">your
  profile</a>.</p>

diff --git a/patchwork/templates/patchwork/user-link.html b/patchwork/templates/patchwork/user-link.html
index bf331520f3a4e2f5af125a0e418538f82418ab1d..c0595bdca1b467fc5efcc198160e38009019eb5c 100644 (file)
--- a/patchwork/templates/patchwork/user-link.html
+++ b/patchwork/templates/patchwork/user-link.html
@@ -9,12 +9,12 @@
 on the link provided in the email to confirm that this address belongs to
 you.</p>
 {% else %}
+   <p>There was an error submitting your link request:</p>
    {% if form.errors %}
-   <p>There was an error submitting your link request.</p>
     {{ form.non_field_errors }}
    {% endif %}
    {% if error %}
-    <ul class="errorlist"><li>{{error}}</li></ul>
+    <ul class="error-list"><li>{{error}}</li></ul>
    {% endif %}
 
    <form action="{% url 'user-link' %}" method="post">

diff --git a/templates/base.html b/templates/base.html
index e57e2d5b94e9aaee65d85c63368550bce1b63fb8..de76595619067b291388e5591763f0d6efad200e 100644 (file)
--- a/templates/base.html
+++ b/templates/base.html
@@ -105,15 +105,28 @@
     </div>
    </div>
   </nav>
-{% if messages %}
-  <div id="messages">
-  {% for message in messages %}
-  {# TODO(stephenfin): Make use of message.tags when completely #}
-  {# converted to django.contrib.messages #}
-   <div class="message">{{ message }}</div>
-  {% endfor %}
+  {% comment %}
+    spaceless tag is used to remove automatically added whitespace so that the container
+    is truly considered empty by the `:empty` pseudo-class that is used for styling
+  {% endcomment %}{% spaceless %}
+  <ul class="messages">
+  {% if messages %}
+    {% for message in messages %}
+    <li{% if message.tags %} class="{{ message.tags }}"{% endif %}>{{ message }}</li>
+    {% endfor %}
+  {% endif %}
+  </ul>
+  <div id="errors">
+    {% if errors %}
+        <p>The following error{{ errors|length|pluralize:" was,s were" }} encountered:</p>
+        <ul class="error-list">
+        {% for error in errors %}
+            <li>{{ error }}</li>
+        {% endfor %}
+        </ul>
+    {% endif %}
   </div>
-{% endif %}
+  {% endspaceless %}
   <div id="main-content" class="container-fluid">
 {% block body %}
 {% endblock %}