Merge pull request #1797 from codidact/0valt/1040/in-page-follow

Follow/unfollow post threads in place if JS is enabled

Author: Oaphi

app/assets/javascripts/comments.js

@@ -339,4 +339,41 @@ $(() => {
       $tgt.find('.js-text').text('copy link');
     }, 1000);
   });
+
+  QPixel.DOM.addSelectorListener('click', '.js-follow-comments', async (ev) => {
+    ev.preventDefault();
+
+    const { target } = ev;
+
+    if (!QPixel.DOM.isHTMLElement(target)) {
+      return;
+    }
+
+    const { postId, action } = target.dataset;
+
+    if (!postId || !action) {
+      return;
+    }
+
+    const shouldFollow = action === 'follow';
+
+    const data = shouldFollow ?
+                  await QPixel.followComments(postId) :
+                  await QPixel.unfollowComments(postId);
+
+    QPixel.handleJSONResponse(data, () => {
+      target.dataset.action = shouldFollow ? 'unfollow' : 'follow';
+
+      const icon = document.createElement('i');
+      icon.classList.add('fas', 'fa-fw', shouldFollow ? 'fa-bell-slash' : 'fa-bell');
+      const text = document.createTextNode(` ${shouldFollow ? 'Unfollow' : 'Follow'} new`);
+      target.replaceChildren(icon, text);
+
+      const form = target.closest('form');
+
+      if (form) {
+        form.action = `/comments/post/${postId}/${shouldFollow ? 'unfollow' : 'follow'}`;
+      }
+    });
+  });
 });

app/assets/javascripts/qpixel_api.js

@@ -471,6 +471,16 @@ window.QPixel = {
     return data;
   },
 
+  followComments: async (postId) => {
+    const resp = await QPixel.fetchJSON(`/comments/post/${postId}/follow`, {}, {
+      headers: { 'Accept': 'application/json' }
+    });
+
+    const data = await resp.json();
+
+    return data;
+  },
+
   undeleteComment: async (id) => {
     const resp = await QPixel.fetchJSON(`/comments/${id}/delete`, {}, {
       headers: { 'Accept': 'application/json' },
@@ -482,6 +492,16 @@ window.QPixel = {
     return data;
   },
 
+  unfollowComments: async (postId) => {
+    const resp = await QPixel.fetchJSON(`/comments/post/${postId}/unfollow`, {}, {
+      headers: { 'Accept': 'application/json' }
+    });
+
+    const data = await resp.json();
+
+    return data;
+  },
+
   lockThread: async (id) => {
     const resp = await QPixel.fetchJSON(`/comments/thread/${id}/restrict`, {
       type: 'lock'

app/assets/javascripts/qpixel_dom.js

@@ -4,13 +4,6 @@ QPixel.DOM = {
   _delegatedListeners: [],
   _eventListeners: {},
 
-  /**
-   * Add a delegated event listener. Use when an event listener is required that will fire for elements added to the
-   * DOM dynamically after the delegated listener is added.
-   * @param {string} event An event name to listen for.
-   * @param {string} selector A CSS selector representing elements on which to apply the listener.
-   * @param {EventCallback} callback A callback function to pass to the event listener.
-   */
   addDelegatedListener: (event, selector, callback) => {
     if (!QPixel.DOM._eventListeners[event]) {
       const listener = (ev) => {
@@ -26,23 +19,12 @@ QPixel.DOM = {
     QPixel.DOM._delegatedListeners.push({ event, selector, callback });
   },
 
-  /**
-   * Convenience method. Add an event listener to _all_ elements that currently match a selector.
-   * @param {string} event An event name to listen for.
-   * @param {string} selector A CSS selector representing elements on which to apply the listener.
-   * @param {EventCallback} callback A callback function to pass to the event listener.
-   */
   addSelectorListener: (event, selector, callback) => {
     document.querySelectorAll(selector).forEach((el) => {
       el.addEventListener(event, callback);
     });
   },
 
-  /**
-   * Smoothly fade an element out of view, then remove it.
-   * @param {HTMLElement} element The element to fade out.
-   * @param {number} duration A duration for the effect in milliseconds.
-   */
   fadeOut: (element, duration) => {
     element.style.transition = `${duration}ms`;
     element.style.opacity = '0';
@@ -51,12 +33,10 @@ QPixel.DOM = {
     }, duration);
   },
 
-  /**
-   * Helper to set the visibility of an element or array of elements. Uses display: none so should work with screen
-   * readers.
-   * @param {HTMLElement|HTMLElement[]} elements An element or array of elements to set visibility for.
-   * @param {boolean} visible Whether or not the elements should be visible.
-   */
+  isHTMLElement: (node) => {
+    return node instanceof HTMLElement;
+  },
+
   setVisible: (elements, visible) => {
     if (!Array.isArray(elements)) {
       elements = [elements];

app/controllers/comments_controller.rb

@@ -4,7 +4,7 @@ class CommentsController < ApplicationController
   before_action :authenticate_user!, except: [:post, :show, :thread, :thread_content]
 
   before_action :set_comment, only: [:update, :destroy, :undelete, :show]
-  before_action :set_post, only: [:create_thread]
+  before_action :set_post, only: [:create_thread, :post_follow, :post_unfollow]
   before_action :set_thread,
                 only: [:create, :thread, :thread_content, :thread_rename, :thread_restrict, :thread_unrestrict,
                        :thread_followers]
@@ -264,29 +264,33 @@ def thread_unrestrict
 
   def post
     @post = Post.find(params[:post_id])
-    @comment_threads = if current_user&.at_least_moderator? || current_user&.post_privilege?('flag_curate', @post)
-                         CommentThread
-                       else
-                         CommentThread.undeleted
-                       end.where(post: @post).order(deleted: :asc, archived: :asc, reply_count: :desc)
+    @comment_threads = CommentThread.accessible_to(current_user, @post)
+                                    .where(post: @post)
+                                    .order(deleted: :asc, archived: :asc, reply_count: :desc)
     respond_to do |format|
       format.html { render layout: false }
       format.json { render json: @comment_threads }
     end
   end
 
   def post_follow
-    @post = Post.find(params[:post_id])
     if ThreadFollower.where(post: @post, user: current_user).none?
       ThreadFollower.create(post: @post, user: current_user)
     end
-    redirect_to post_path(@post)
+
+    respond_to do |format|
+      format.html { redirect_to post_path(@post) }
+      format.json { render json: { status: 'success' } }
+    end
   end
 
   def post_unfollow
-    @post = Post.find(params[:post_id])
     ThreadFollower.where(post: @post, user: current_user).destroy_all
-    redirect_to post_path(@post)
+
+    respond_to do |format|
+      format.html { redirect_to post_path(@post) }
+      format.json { render json: { status: 'success' } }
+    end
   end
 
   def pingable
@@ -302,7 +306,7 @@ def comment_params
   end
 
   def set_comment
-    @comment = Comment.unscoped.find params[:id]
+    @comment = Comment.unscoped.find(params[:id])
   end
 
   def set_post

app/models/comment_thread.rb

@@ -16,6 +16,18 @@ class CommentThread < ApplicationRecord
 
   after_create :create_follower
 
+  # Gets threads appropriately scoped for a given user & post
+  # @param user [User, nil] user to check
+  # @para post [Post] post to check
+  # @return [ActiveRecord::Relation<CommentThread>]
+  def self.accessible_to(user, post)
+    if user&.at_least_moderator? || user&.post_privilege?('flag_curate', post)
+      CommentThread
+    else
+      CommentThread.undeleted
+    end
+  end
+
   # Is the thread read-only (can't be edited)?
   # @return [Boolean] check result
   def read_only?

app/views/posts/_expanded.html.erb

@@ -297,14 +297,16 @@
               <% unless post.locked? %>
                 <% if !post.deleted %>
                   <%= link_to delete_post_path(post), method: :post,
-                              data: { confirm: 'Are you sure you want to delete this post?' }, class: "tools--item is-danger",
+                              data: { confirm: 'Are you sure you want to delete this post?' },
+                              class: "tools--item is-danger",
                               role: 'button', 'aria-label': 'Delete this post' do %>
                     <i class="fa fa-trash"></i>
                     Delete
                   <% end %>
                 <% else %>
                   <%= link_to restore_post_path(post), method: :post,
-                              data: { confirm: 'Restore this post, making it visible to regular users?' }, class: "tools--item is-danger is-filled",
+                              data: { confirm: 'Restore this post, making it visible to regular users?' },
+                              class: "tools--item is-danger is-filled",
                               role: 'button', 'aria-label': 'Restore this post' do %>
                     <i class="fa fa-undo"></i>
                     Restore
@@ -313,7 +315,11 @@
               <% end %>
             <% end %>
             <% if check_your_privilege('flag_curate') %>
-              <a href="javascript:void(0);" data-modal="#mod-tools-<%= post.id %>" class="tools--item" role="button" aria-label="Open Moderator Tools">
+              <a href="javascript:void(0)"
+                 data-modal="#mod-tools-<%= post.id %>"
+                 class="tools--item"
+                 role="button"
+                 aria-label="Open Moderator Tools">
                 <i class="fa fa-wrench"></i>
                 Tools
               </a>
@@ -429,7 +435,9 @@
               <input class="form-element js-flag-comment" id="flag-post-<%= post.id %>">
             </div>
             <div class="widget--footer">
-              <button class="flag-link button is-filled is-muted" data-post-type="<%= is_question ? 'Question' : 'Answer' %>" data-post-id="<%= post.id %>">
+              <button class="flag-link button is-filled is-muted"
+                      data-post-type="<%= is_question ? 'Question' : 'Answer' %>"
+                      data-post-id="<%= post.id %>">
                 Flag for attention
               </button>
             </div>
@@ -501,7 +509,10 @@
 
         <% if is_top_level && post.children.undeleted.length >= SiteSetting["TableOfContentsThreshold"] && SiteSetting["TableOfContentsThreshold"] != -1 %>
           <div class="toc has-margin-left-4" id="toc-toggle">
-            <button class="toc--header" data-toggle="#toc-toggle" data-toggle-property="class" data-toggle-value="is-active">Table of Contents</button>
+            <button class="toc--header"
+                    data-toggle="#toc-toggle"
+                    data-toggle-property="class"
+                    data-toggle-value="is-active">Table of Contents</button>
             <% sorted_answers = post.children.sort_by { |answer| answer.score }.reverse! %>
             <% sorted_answers.each do |answer| %>
               <% next if answer.deleted? && !at_least_moderator? %>
@@ -532,23 +543,7 @@
             <%= pluralize(public_count, 'comment thread') %>
           </h4>
           <% if user_signed_in? %>
-            <% if post.followed_by?(current_user) %>
-              <%= link_to unfollow_post_comments_path(post_id: post.id), method: :post,
-                          class: "button is-muted is-outlined is-small",
-                          title: 'Don\'t follow new comment threads on this post',
-                          role: 'button',
-                          'aria-label': 'Unfollow new comment threads on this post' do %>
-                  <i class="fas fa-fw fa-bell-slash"></i> Unfollow new
-              <% end %>
-            <% else %>
-              <%= link_to follow_post_comments_path(post_id: post.id), method: :post,
-                          class: "button is-muted is-outlined is-small",
-                          title: 'Follow all new comment threads on this post',
-                          role: 'button',
-                          'aria-label': 'Follow all new comment threads on this post' do %>
-                <i class="fas fa-fw fa-bell"></i> Follow new
-              <% end %>
-            <% end %>
+            <%= render 'posts/follow_comments_link', post: post, user: current_user %>
           <% end %>
           </div>
           <div class="post--comments-container" role="list">
@@ -558,7 +553,10 @@
           </div>
           <div class="post--comments-links has-margin-top-1">
             <% if available_count > [comment_threads.count, 5].min %>
-              <a href="#" class="js-more-comments button is-muted is-small" data-post-id="<%= post.id %>" role="button" aria-label="Show more comment threads">
+              <a href="#" class="js-more-comments button is-muted is-small"
+                          data-post-id="<%= post.id %>"
+                          role="button"
+                          aria-label="Show more comment threads">
                 Show more
               </a>
             <% end %>

app/views/posts/_follow_comments_link.html.erb

@@ -0,0 +1,26 @@
+<%#
+   "Helper for rendering the follow/unfollow comments buttons
+
+    Variables:
+      post : Post to create the link for
+      user : User to check the following status for
+"%>
+
+<%
+  is_followed = post.followed_by?(user)
+  action_path = is_followed ?
+                  unfollow_post_comments_path(post_id: post.id) :
+                  follow_post_comments_path(post_id: post.id)
+  text = "#{is_followed ? 'Unfollow' : 'Follow'} new"
+  title = 'Switch following new comment threads on this post'
+%>
+
+<%= form_tag action_path, method: :post do %>
+  <%= button_tag type: :submit,
+                 class: "button is-muted is-outlined is-small js-follow-comments",
+                 data: { post_id: post.id, action: is_followed ? 'unfollow' : 'follow' },
+                 aria: { label: title },
+                 title: title do %>
+    <i class="fas fa-fw <%= is_followed ? 'fa-bell-slash' : 'fa-bell' %>"></i> <%= text %>
+  <% end %>
+<% end %>