From 36b63a872327ae13f09b94255e1b8a56207a889d Mon Sep 17 00:00:00 2001
From: Deimos <deimos@tildes.net>
Date: Wed, 11 Sep 2019 14:28:35 -0600
Subject: [PATCH] Show errors from Intercooler <button> requests

Previously, error messages were only shown inside <form> elements. If a
button hit an error (such as a 403 when trying to vote on a comment that
was deleted after page load), the button simply wouldn't work with no
indication of why.

This adds the error message into the <menu> containing the buttons, and
involved some reworking of the relevant JS and CSS.
---
 tildes/scss/modules/_btn.scss                 |  5 ++
 tildes/scss/modules/_form.scss                |  6 +-
 tildes/scss/modules/_text.scss                |  4 ++
 tildes/scss/themes/_theme_base.scss           |  8 +--
 tildes/static/js/behaviors/theme-selector.js  |  2 +-
 tildes/static/js/scripts.js                   | 63 ++++++++++---------
 .../templates/includes/new_topic_form.jinja2  |  2 +-
 7 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/tildes/scss/modules/_btn.scss b/tildes/scss/modules/_btn.scss
index 340ec81..43baaff 100644
--- a/tildes/scss/modules/_btn.scss
+++ b/tildes/scss/modules/_btn.scss
@@ -99,6 +99,11 @@
   form {
     min-width: 100%;
   }
+
+  // Status message (errors) should be on their own line as well
+  > .text-status-message {
+    min-width: 100%;
+  }
 }
 
 .btn-post-action {
diff --git a/tildes/scss/modules/_form.scss b/tildes/scss/modules/_form.scss
index 7d08a87..d66a81d 100644
--- a/tildes/scss/modules/_form.scss
+++ b/tildes/scss/modules/_form.scss
@@ -55,6 +55,7 @@ select.form-select:not([multiple]) {
   display: flex;
   flex-direction: row-reverse;
   justify-content: flex-start;
+  align-items: center;
 
   margin: 0.2rem 0;
   max-width: 40rem;
@@ -72,11 +73,6 @@ textarea.form-input {
   transition: none;
 }
 
-.form-status {
-  margin: auto 0;
-  font-size: 0.6rem;
-}
-
 .form-input {
   max-width: 40rem;
 }
diff --git a/tildes/scss/modules/_text.scss b/tildes/scss/modules/_text.scss
index a2c246e..b277d59 100644
--- a/tildes/scss/modules/_text.scss
+++ b/tildes/scss/modules/_text.scss
@@ -10,6 +10,10 @@
   line-height: 0.9rem;
 }
 
+.text-status-message {
+  font-size: 0.6rem;
+}
+
 // special formatting rules for wiki pages
 .text-wiki {
   h1,
diff --git a/tildes/scss/themes/_theme_base.scss b/tildes/scss/themes/_theme_base.scss
index 805bb4d..ad23b84 100644
--- a/tildes/scss/themes/_theme_base.scss
+++ b/tildes/scss/themes/_theme_base.scss
@@ -368,10 +368,6 @@
     }
   }
 
-  .form-status-error {
-    color: map-get($theme, "error");
-  }
-
   .input-group-addon {
     background-color: map-get($theme, "background-secondary");
     color: map-get($theme, "foreground-highlight");
@@ -478,6 +474,10 @@
     }
   }
 
+  .text-error {
+    color: map-get($theme, "error");
+  }
+
   .text-secondary {
     color: map-get($theme, "foreground-secondary");
   }
diff --git a/tildes/static/js/behaviors/theme-selector.js b/tildes/static/js/behaviors/theme-selector.js
index ae8798b..0d96d63 100644
--- a/tildes/static/js/behaviors/theme-selector.js
+++ b/tildes/static/js/behaviors/theme-selector.js
@@ -8,7 +8,7 @@ $.onmount("[data-js-theme-selector]", function() {
         // hide any IC change message
         $(this)
             .parent()
-            .find(".form-status")
+            .find(".text-status-message")
             .hide();
 
         var new_theme = $(this).val();
diff --git a/tildes/static/js/scripts.js b/tildes/static/js/scripts.js
index af1ee64..d11040a 100644
--- a/tildes/static/js/scripts.js
+++ b/tildes/static/js/scripts.js
@@ -26,45 +26,48 @@ $(function() {
         $.onmount();
     });
 
-    // Called whenever an Intercooler request completes; used for <form> elements
-    // to display the error or a success message.
-    // If the triggering element already contains an element with class
-    // "form-status", it will be removed, then a new one is added inside the
-    // .form-buttons element if possible, otherwise it will be appended to the
-    // triggering element itself. The status div will then have its class set based
-    // on whether the response was an error or not, and the text set to either the
-    // error message or a generic success message.
+    // Called whenever an Intercooler request completes; used to display an error or
+    // success message in an appropriate place near supported elements.
     /* eslint-disable-next-line no-unused-vars */
     $(document).on("complete.ic", function(evt, elt, data, status, xhr, requestId) {
-        // only do anything for <form> elements
-        if (elt[0].tagName !== "FORM") {
+        var $container = null;
+
+        // Only display these messages if the triggering element was <form> or <button>
+        if (elt[0].tagName === "FORM") {
+            $container = $(elt);
+
+            // add the message inside .form-buttons if possible
+            var $buttonElement = $container.find(".form-buttons").first();
+            if ($buttonElement.length) {
+                $container = $buttonElement;
+            }
+        } else if (elt[0].tagName === "BUTTON") {
+            // for buttons, we only want to display error messages
+            if (status !== "error") {
+                return;
+            }
+
+            // choose the containing <menu> as the place to display the message
+            // if the button isn't in a menu, nothing will be displayed
+            $container = $(elt).closest("menu");
+        }
+
+        // exit if it's an unsupported element or no appropriate container was found
+        if (!$container || !$container.length) {
             return;
         }
 
         // see if a status element already exists and remove it
-        $(elt)
-            .find(".form-status")
-            .remove();
-
-        // add a new one (inside .form-buttons if possible)
-        var statusDiv = '<div class="form-status"></div>';
+        $container.find(".text-status-message").remove();
 
-        var $buttonElement = $(elt)
-            .find(".form-buttons")
-            .first();
-        if ($buttonElement.length) {
-            $buttonElement.append(statusDiv);
-        } else {
-            $(elt).append(statusDiv);
-        }
-        var $statusElement = $(elt)
-            .find(".form-status")
-            .first();
+        // add the new one
+        $container.append('<div class="text-status-message"></div>');
+        var $statusElement = $container.find(".text-status-message").first();
 
-        // set the class and text, then fade in
+        // set the text (and class for errors), then fade in
         $statusElement.hide();
         if (status === "success") {
-            $statusElement.addClass("form-status-success").text("Saved successfully");
+            $statusElement.text("Saved successfully");
         } else {
             var errorText = xhr.responseText;
             if (xhr.status === 413) {
@@ -76,7 +79,7 @@ $(function() {
                 errorText = "Unknown error";
             }
 
-            $statusElement.addClass("form-status-error").text(errorText);
+            $statusElement.addClass("text-error").text(errorText);
         }
         $statusElement.fadeIn("slow");
     });
diff --git a/tildes/tildes/templates/includes/new_topic_form.jinja2 b/tildes/tildes/templates/includes/new_topic_form.jinja2
index d139bdc..f5d75a4 100644
--- a/tildes/tildes/templates/includes/new_topic_form.jinja2
+++ b/tildes/tildes/templates/includes/new_topic_form.jinja2
@@ -51,7 +51,7 @@
   <div class="form-buttons">
     <button type="submit" class="btn btn-primary">Post topic</button>
     {% if previous_topics %}
-      <div class="form-status form-status-error">Link has been posted before, see below</div>
+      <div class="text-status-message text-error">Link has been posted before, see below</div>
     {% endif %}
   </div>