From 6227f747c18f48e989d6f04cf3a260ff1c035c97 Mon Sep 17 00:00:00 2001
From: Deimos <deimos@tildes.net>
Date: Mon, 15 Jun 2020 17:27:22 -0600
Subject: [PATCH] Use intercooler for comment reply form

Previously, the comment reply form was being created entirely
client-side by cloning and modifying a <template>. This was nice because
it meant that a network request wasn't necessary to display the form,
but it also had downsides.

For example, if a topic was locked after a user had already loaded the
page (or their notifications page with a comment from that topic), they
would still be able to click Reply and type in a comment, and wouldn't
know that replying wasn't possible until they actually tried to submit
the comment.

By switching to using intercooler for this form, we can do server-side
validation to check permissions before showing the form, and it also
simplifies some other aspects, such as the warning about replying to an
old comment, which previously needed a data-js-old-warning-age attribute
in the HTML, but is now just part of generating the reply form template
server-side.
---
 tildes/scss/modules/_comment.scss             |   6 +-
 .../js/behaviors/comment-reply-button.js      | 102 ------------------
 .../static/js/behaviors/comment-reply-form.js |  46 ++++++++
 tildes/tildes/routes.py                       |   1 +
 .../intercooler/comment_reply.jinja2          |  35 ++++++
 .../tildes/templates/macros/comments.jinja2   |  42 +++-----
 .../templates/notifications_unread.jinja2     |   3 +-
 tildes/tildes/templates/topic.jinja2          |   3 +-
 tildes/tildes/templates/user.jinja2           |   3 +-
 tildes/tildes/views/api/web/comment.py        |  11 ++
 10 files changed, 113 insertions(+), 139 deletions(-)
 delete mode 100644 tildes/static/js/behaviors/comment-reply-button.js
 create mode 100644 tildes/static/js/behaviors/comment-reply-form.js
 create mode 100644 tildes/tildes/templates/intercooler/comment_reply.jinja2

diff --git a/tildes/scss/modules/_comment.scss b/tildes/scss/modules/_comment.scss
index 0526e0e..88a2003 100644
--- a/tildes/scss/modules/_comment.scss
+++ b/tildes/scss/modules/_comment.scss
@@ -100,7 +100,6 @@
 
 .comment-tree {
   margin: 0;
-  padding-top: 0.4rem;
   list-style-type: none;
 }
 
@@ -114,8 +113,11 @@
 
 .comment-tree-item {
   margin: 0;
-  padding: 0;
   max-width: none;
+
+  &:first-child {
+    margin-top: 0.4rem;
+  }
 }
 
 .comment-labels {
diff --git a/tildes/static/js/behaviors/comment-reply-button.js b/tildes/static/js/behaviors/comment-reply-button.js
deleted file mode 100644
index 32214dc..0000000
--- a/tildes/static/js/behaviors/comment-reply-button.js
+++ /dev/null
@@ -1,102 +0,0 @@
-// Copyright (c) 2018 Tildes contributors <code@tildes.net>
-// SPDX-License-Identifier: AGPL-3.0-or-later
-
-$.onmount("[data-js-comment-reply-button]", function() {
-    $(this).click(function(event) {
-        event.preventDefault();
-
-        // disable click/hover events on the reply button
-        $(this).css("pointer-events", "none");
-
-        var $comment = $(this)
-            .parents(".comment")
-            .first();
-
-        // get the replies list, or create one if it doesn't already exist
-        var $replies = $comment.children(".comment-tree-replies");
-        if (!$replies.length) {
-            var repliesList = document.createElement("ol");
-            repliesList.setAttribute("class", "comment-tree comment-tree-replies");
-            $comment.append(repliesList);
-            $replies = $(repliesList);
-        }
-
-        var $parentComment = $(this).parents("article.comment:first");
-        var parentCommentID = $parentComment.attr("data-comment-id36");
-        var postURL = "/api/web/comments/" + parentCommentID + "/replies";
-        var markdownID = "markdown-reply-" + parentCommentID;
-        var previewID = markdownID + "-preview";
-
-        if ($("#" + markdownID).length > 0) {
-            $("#" + markdownID).focus();
-            return;
-        }
-
-        // clone and populate the 'comment-reply' template
-        var template = document.getElementById("comment-reply");
-        var clone = document.importNode(template.content, true);
-
-        clone.querySelector("form").setAttribute("data-ic-post-to", postURL);
-        var textarea = clone.querySelector("textarea");
-        textarea.setAttribute("id", markdownID);
-
-        var preview = clone.querySelector(".form-markdown-preview");
-        preview.setAttribute("id", previewID);
-
-        clone
-            .querySelector("[data-js-markdown-preview-tab] .btn")
-            .setAttribute("data-ic-target", "#" + previewID);
-
-        var cancelButton = clone.querySelector("[data-js-cancel-button]");
-        $(cancelButton).on("click", function() {
-            // re-enable click/hover events on the reply button
-            $(this)
-                .parents(".comment")
-                .first()
-                .find(".btn-post-action[name=reply]")
-                .first()
-                .css("pointer-events", "auto");
-        });
-
-        // If the user has text selected inside a comment when they click the reply
-        // button, start the comment form off with that text inside a blockquote
-        if (window.getSelection) {
-            var selectedText = "";
-
-            // only capture the selected text if it's all from the same comment
-            var selection = window.getSelection();
-            var $start = $(selection.anchorNode).closest(".comment-text");
-            var $end = $(selection.focusNode).closest(".comment-text");
-            if ($start.is($end)) {
-                selectedText = selection.toString();
-            }
-
-            if (selectedText.length > 0) {
-                selectedText = selectedText.replace(/\s+$/g, "");
-                selectedText = selectedText.replace(/^/gm, "> ");
-                textarea.value = selectedText + "\n\n";
-                textarea.scrollTop = textarea.scrollHeight;
-            }
-        }
-
-        // add a warning about the comment's age, if necessary (determined by backend)
-        var warningAge = $(this).attr("data-js-old-warning-age");
-        if (warningAge) {
-            var warningDiv = document.createElement("div");
-            warningDiv.classList.add("warning-old-reply");
-            warningDiv.innerHTML =
-                '<p class="text-warning text-small">The comment you\'re replying to ' +
-                "is " +
-                warningAge +
-                " old. Replying to old comments is fine as long as you're " +
-                "contributing to the discussion.</p>";
-            clone.querySelector("form").prepend(warningDiv);
-        }
-
-        // update Intercooler so it knows about this new form
-        Intercooler.processNodes(clone);
-
-        $replies.prepend(clone);
-        $.onmount();
-    });
-});
diff --git a/tildes/static/js/behaviors/comment-reply-form.js b/tildes/static/js/behaviors/comment-reply-form.js
new file mode 100644
index 0000000..dc0fd33
--- /dev/null
+++ b/tildes/static/js/behaviors/comment-reply-form.js
@@ -0,0 +1,46 @@
+// Copyright (c) 2020 Tildes contributors <code@tildes.net>
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+$.onmount("[data-js-comment-reply-form]", function() {
+    var $this = $(this);
+
+    // the parent comment's Reply button (that was clicked to create this form)
+    var $replyButton = $this
+        .closest(".comment")
+        .find(".btn-post-action[name=reply]")
+        .first();
+
+    // disable click/hover events on the reply button to prevent opening multiple forms
+    $replyButton.css("pointer-events", "none");
+
+    // have the Cancel button re-enable click/hover events on the reply button
+    $this.find("[data-js-cancel-button]").click(function() {
+        $replyButton.css("pointer-events", "auto");
+    });
+
+    var $textarea = $this.find("textarea").first();
+
+    // If the user has text selected inside a comment when the reply form is created,
+    // populate the textbox with that text inside a blockquote
+    if (window.getSelection) {
+        var selectedText = "";
+
+        // only capture the selected text if it's all from the same comment
+        var selection = window.getSelection();
+        var $start = $(selection.anchorNode).closest(".comment-text");
+        var $end = $(selection.focusNode).closest(".comment-text");
+        if ($start.is($end)) {
+            selectedText = selection.toString();
+        }
+
+        if (selectedText.length > 0) {
+            selectedText = selectedText.replace(/\s+$/g, "");
+            selectedText = selectedText.replace(/^/gm, "> ");
+
+            $textarea.val(selectedText + "\n\n");
+            $textarea.scrollTop($textarea.prop("scrollHeight"));
+        }
+    }
+
+    $textarea.focus();
+});
diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py
index cb55137..e0a197c 100644
--- a/tildes/tildes/routes.py
+++ b/tildes/tildes/routes.py
@@ -154,6 +154,7 @@ def add_intercooler_routes(config: Configurator) -> None:
     with config.route_prefix_context("/comments/{comment_id36}"):
         add_ic_route("comment_remove", "/remove", factory=comment_by_id36)
         add_ic_route("comment_replies", "/replies", factory=comment_by_id36)
+        add_ic_route("comment_reply", "/reply", factory=comment_by_id36)
         add_ic_route("comment_vote", "/vote", factory=comment_by_id36)
         add_ic_route("comment_label", "/labels/{name}", factory=comment_by_id36)
         add_ic_route("comment_bookmark", "/bookmark", factory=comment_by_id36)
diff --git a/tildes/tildes/templates/intercooler/comment_reply.jinja2 b/tildes/tildes/templates/intercooler/comment_reply.jinja2
new file mode 100644
index 0000000..599fd2a
--- /dev/null
+++ b/tildes/tildes/templates/intercooler/comment_reply.jinja2
@@ -0,0 +1,35 @@
+{# Copyright (c) 2020 Tildes contributors <code@tildes.net> #}
+{# SPDX-License-Identifier: AGPL-3.0-or-later #}
+
+{% from 'macros/forms.jinja2' import markdown_textarea %}
+
+<li class="comment-tree-item">
+
+<form
+  method="post"
+  autocomplete="off"
+  data-ic-post-to="{{ request.route_url(
+    "ic_comment_replies",
+    comment_id36=parent_comment.comment_id36,
+  ) }}"
+  data-ic-replace-target="true"
+  data-js-confirm-cancel="Discard your reply?"
+  data-js-prevent-double-submit
+  data-js-confirm-leave-page-unsaved
+  data-js-comment-reply-form
+>
+  {% if parent_comment.age.days >= 7 %}
+  <div class="warning-old-reply">
+    <p class="text-warning text-small">The comment you're replying to is {{ parent_comment.age|vague_timedelta_description }} old. Replying to old comments is fine as long as you're contributing to the discussion.</p>
+  </div>
+  {% endif %}
+
+  {{ markdown_textarea(id="markdown-reply-%s" % parent_comment.comment_id36) }}
+
+  <div class="form-buttons">
+    <button type="submit" class="btn btn-primary">Post comment</button>
+    <button type="button" class="btn btn-link" data-js-cancel-button>Cancel</button>
+  </div>
+</form>
+
+</li>
diff --git a/tildes/tildes/templates/macros/comments.jinja2 b/tildes/tildes/templates/macros/comments.jinja2
index 2d4972e..f4f60d6 100644
--- a/tildes/tildes/templates/macros/comments.jinja2
+++ b/tildes/tildes/templates/macros/comments.jinja2
@@ -3,7 +3,6 @@
 
 {% from 'buttons.jinja2' import post_action_toggle_button with context %}
 {% from 'datetime.jinja2' import adaptive_date_responsive %}
-{% from 'forms.jinja2' import markdown_textarea %}
 {% from 'links.jinja2' import link_to_user with context %}
 {% from 'utils.jinja2' import pluralize %}
 
@@ -30,12 +29,13 @@
     >
       {{ render_comment_contents(comment, is_individual_comment) }}
 
-      {# Recursively display reply comments, unless we hit a "removed marker" #}
-      {% if comment.replies and
-           (request.has_permission("view", comment) or not comment.removed_marker) %}
-        <ol class="comment-tree comment-tree-replies">
+      {% if request.has_permission("view", comment) or not comment.removed_marker %}
+      <ol class="comment-tree comment-tree-replies">
+        {# Recursively display reply comments, unless we hit a "removed marker" #}
+        {% if comment.replies %}
           {{ loop(comment.replies) }}
-        </ol>
+        {% endif %}
+      </ol>
       {% endif %}
     </article>
     {% if not is_individual_comment %}</li>{% endif %}
@@ -226,10 +226,13 @@
             <button
               class="btn-post-action"
               name="reply"
-              data-js-comment-reply-button
-              {% if comment.age.days >= 7 %}
-              data-js-old-warning-age="{{ comment.age|vague_timedelta_description }}"
-              {% endif %}
+              data-ic-get-from="{{ request.route_url(
+                "ic_comment_reply",
+                comment_id36=comment.comment_id36,
+              ) }}"
+              data-ic-target="#comment-{{ comment.comment_id36 }} > .comment-tree-replies:first"
+              data-ic-scroll-to-target="true"
+              data-ic-swap-style="prepend"
             >Reply</button>
           </li>
         {% endif %}
@@ -282,22 +285,3 @@
     </menu>
   </template>
 {% endmacro %}
-
-{% macro comment_reply_template() %}
-<template id="comment-reply">
-  <form
-    method="post"
-    autocomplete="off"
-    data-js-confirm-cancel="Discard your reply?"
-    data-js-prevent-double-submit
-    data-js-confirm-leave-page-unsaved
-    data-ic-replace-target="true"
-  >
-    {{ markdown_textarea(auto_focus=True) }}
-    <div class="form-buttons">
-      <button type="submit" class="btn btn-primary">Post comment</button>
-      <button type="button" class="btn btn-link" data-js-cancel-button>Cancel</button>
-    </div>
-  </form>
-</template>
-{% endmacro %}
diff --git a/tildes/tildes/templates/notifications_unread.jinja2 b/tildes/tildes/templates/notifications_unread.jinja2
index 0258c07..6cab6a8 100644
--- a/tildes/tildes/templates/notifications_unread.jinja2
+++ b/tildes/tildes/templates/notifications_unread.jinja2
@@ -3,7 +3,7 @@
 
 {% extends 'base_user_menu.jinja2' %}
 
-{% from 'macros/comments.jinja2' import comment_label_options_template, comment_reply_template, render_single_comment with context %}
+{% from 'macros/comments.jinja2' import comment_label_options_template, render_single_comment with context %}
 {% from 'macros/links.jinja2' import link_to_group with context %}
 
 {% block title %}Unread notifications{% endblock %}
@@ -80,5 +80,4 @@
 
 {% block templates %}
   {{ comment_label_options_template(comment_label_options) }}
-  {{ comment_reply_template() }}
 {% endblock %}
diff --git a/tildes/tildes/templates/topic.jinja2 b/tildes/tildes/templates/topic.jinja2
index fab73fb..ee8e36c 100644
--- a/tildes/tildes/templates/topic.jinja2
+++ b/tildes/tildes/templates/topic.jinja2
@@ -4,7 +4,7 @@
 {% extends 'base.jinja2' %}
 
 {% from 'macros/buttons.jinja2' import post_action_toggle_button with context %}
-{% from 'macros/comments.jinja2' import comment_label_options_template, comment_reply_template, render_comment_tree with context %}
+{% from 'macros/comments.jinja2' import comment_label_options_template, render_comment_tree with context %}
 {% from 'macros/datetime.jinja2' import adaptive_date_responsive, time_ago %}
 {% from 'macros/forms.jinja2' import markdown_textarea %}
 {% from 'macros/groups.jinja2' import group_segmented_link %}
@@ -21,7 +21,6 @@
 
 {% block templates %}
   {% if request.user %}
-    {{ comment_reply_template() }}
     {{ comment_label_options_template(comment_label_options) }}
   {% endif %}
 {% endblock %}
diff --git a/tildes/tildes/templates/user.jinja2 b/tildes/tildes/templates/user.jinja2
index 9faf384..6d19357 100644
--- a/tildes/tildes/templates/user.jinja2
+++ b/tildes/tildes/templates/user.jinja2
@@ -3,7 +3,7 @@
 
 {% extends 'base_user_menu.jinja2' %}
 
-{% from 'macros/comments.jinja2' import comment_label_options_template, comment_reply_template, render_single_comment with context %}
+{% from 'macros/comments.jinja2' import comment_label_options_template, render_single_comment with context %}
 {% from 'macros/links.jinja2' import link_to_group with context %}
 {% from 'macros/topics.jinja2' import render_topic_for_listing with context %}
 
@@ -11,7 +11,6 @@
 
 {% block templates %}
   {% if request.user %}
-    {{ comment_reply_template() }}
     {{ comment_label_options_template(comment_label_options) }}
   {% endif %}
 {% endblock %}
diff --git a/tildes/tildes/views/api/web/comment.py b/tildes/tildes/views/api/web/comment.py
index 71fc941..7b4dead 100644
--- a/tildes/tildes/views/api/web/comment.py
+++ b/tildes/tildes/views/api/web/comment.py
@@ -130,6 +130,17 @@ def get_comment_contents(request: Request) -> dict:
     return {"comment": request.context}
 
 
+@ic_view_config(
+    route_name="comment_reply",
+    request_method="GET",
+    renderer="comment_reply.jinja2",
+    permission="reply",
+)
+def get_comment_reply(request: Request) -> dict:
+    """Get the reply form for a comment with Intercooler."""
+    return {"parent_comment": request.context}
+
+
 @ic_view_config(
     route_name="comment",
     request_method="GET",