From 8e6df0b1e38c1e8f06373572db96d0ec20bf9c2f Mon Sep 17 00:00:00 2001
From: cmrd Senya <senya@riseup.net>
Date: Wed, 16 Sep 2015 22:26:58 +0300
Subject: [PATCH] Comments expansion refactoring

If you look at comment expand process with javascript debugger, you
will notice that at first comments get added to existing stream ("add"
event handler of model.comments gets launched because of model.comments.fetch).
Then the comment stream gets empty and then filled by the postRenderTemplate
handler.

This patch removes comments rerendering on the expansion. The fetching process is
changed so that older comments are added to the correct place, so the order
looks nice without rerendering. Thus, unnecessary job of rerendering is avoided.
---
 .../app/views/comment_stream_view.js          | 49 ++++++++++++-------
 .../templates/comment-stream_tpl.jst.hbs      | 14 +++---
 .../app/views/comment_stream_view_spec.js     | 32 ++++++------
 3 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/app/assets/javascripts/app/views/comment_stream_view.js b/app/assets/javascripts/app/views/comment_stream_view.js
index 8cc3d238e..6878eec1a 100644
--- a/app/assets/javascripts/app/views/comment_stream_view.js
+++ b/app/assets/javascripts/app/views/comment_stream_view.js
@@ -21,16 +21,10 @@ app.views.CommentStream = app.views.Base.extend({
 
   setupBindings: function() {
     this.model.comments.bind('add', this.appendComment, this);
-    this.model.bind("commentsExpanded", this.storeTextareaValue, this);
-    this.model.bind("commentsExpanded", this.render, this);
   },
 
   postRenderTemplate : function() {
     this.model.comments.each(this.appendComment, this);
-
-    // add autoexpanders to new comment textarea
-    this.$("textarea").val(this.textareaValue);
-    autosize.update(this.$("textarea"));
   },
 
   presenter: function(){
@@ -62,34 +56,55 @@ app.views.CommentStream = app.views.Base.extend({
     }
   },
 
+  _insertPoint: 0, // An index of the comment added in the last call of this.appendComment
+
+  // This adjusts this._insertPoint according to timestamp value
+  _moveInsertPoint: function(timestamp, commentBlocks) {
+    if (commentBlocks.length === 0) {
+      this._insertPoint = 0;
+      return;
+    }
+
+    if (this._insertPoint > commentBlocks.length) {
+      this._insertPoint = commentBlocks.length;
+    }
+
+    while (this._insertPoint > 0 && timestamp < commentBlocks.eq(this._insertPoint - 1).find("time").attr("datetime")) {
+      this._insertPoint--;
+    }
+    while (this._insertPoint < commentBlocks.length &&
+        timestamp > commentBlocks.eq(this._insertPoint).find("time").attr("datetime")) {
+      this._insertPoint++;
+    }
+  },
+
   appendComment: function(comment) {
     // Set the post as the comment's parent, so we can check
     // on post ownership in the Comment view.
     comment.set({parent : this.model.toJSON()});
 
-    this.$(".comments").append(new app.views.Comment({
-      model: comment
-    }).render().el);
+    var commentHtml = new app.views.Comment({model: comment}).render().el;
+    var commentBlocks = this.$(".comments div.comment.media");
+    this._moveInsertPoint(comment.get("created_at"), commentBlocks);
+    if (this._insertPoint === commentBlocks.length) {
+      this.$(".comments").append(commentHtml);
+    } else {
+      commentBlocks.eq(this._insertPoint).before(commentHtml);
+    }
+    this._insertPoint++;
   },
 
   commentTextareaFocused: function(){
     this.$("form").removeClass('hidden').addClass("open");
   },
 
-  storeTextareaValue: function(){
-    this.textareaValue = this.$('textarea').val();
-  },
-
   expandComments: function(evt){
     if(evt){ evt.preventDefault(); }
     var self = this;
 
     this.model.comments.fetch({
       success : function(resp){
-        self.model.set({
-          comments : resp.models,
-          all_comments_loaded : true
-        });
+        self.$("div.comment.show_comments").addClass("hidden");
 
         self.model.trigger("commentsExpanded", self);
       }
diff --git a/app/assets/templates/comment-stream_tpl.jst.hbs b/app/assets/templates/comment-stream_tpl.jst.hbs
index 1bbf98bba..0b6678a10 100644
--- a/app/assets/templates/comment-stream_tpl.jst.hbs
+++ b/app/assets/templates/comment-stream_tpl.jst.hbs
@@ -1,12 +1,10 @@
-{{#unless all_comments_loaded}}
-  <div class="show_comments comment {{#unless showExpandCommentsLink}} hidden {{/unless}}">
-    <div class="media">
-      <a href="/posts/{{id}}#comments" class="toggle_post_comments">
-        {{t "stream.more_comments" count=moreCommentsCount}}
-      </a>
-    </div>
+<div class="show_comments comment {{#unless showExpandCommentsLink}} hidden {{/unless}}">
+  <div class="media">
+    <a href="/posts/{{id}}#comments" class="toggle_post_comments">
+      {{t "stream.more_comments" count=moreCommentsCount}}
+    </a>
   </div>
-{{/unless}}
+</div>
 
 <div class="comments"> </div>
 
diff --git a/spec/javascripts/app/views/comment_stream_view_spec.js b/spec/javascripts/app/views/comment_stream_view_spec.js
index 9157aab5f..c0f3f20fb 100644
--- a/spec/javascripts/app/views/comment_stream_view_spec.js
+++ b/spec/javascripts/app/views/comment_stream_view_spec.js
@@ -5,20 +5,11 @@ describe("app.views.CommentStream", function(){
   });
 
   describe("binds", function() {
-    it("re-renders on a commentsExpanded trigger", function(){
-      spyOn(this.view, "render");
+    it("calls appendComment on insertion to the comments collection", function() {
+      spyOn(this.view, "appendComment");
       this.view.setupBindings();
-      this.view.model.trigger("commentsExpanded");
-      expect(this.view.render).toHaveBeenCalled();
-    });
-  });
-
-  describe("postRenderTemplate", function(){
-    it("autoResizes the new comment textarea", function(){
-      spyOn(window.autosize, "update");
-      this.view.postRenderTemplate();
-      expect(window.autosize.update).toHaveBeenCalled();
-      expect(window.autosize.update.calls.mostRecent().args[0].selector).toBe("textarea");
+      this.view.model.comments.push(factory.comment());
+      expect(this.view.appendComment).toHaveBeenCalled();
     });
   });
 
@@ -79,10 +70,23 @@ describe("app.views.CommentStream", function(){
       this.view.appendComment(comment);
       expect(comment.set).toHaveBeenCalled();
     });
+
+    it("sorts comments in the right order", function() {
+      this.view.render();
+      this.view.appendComment(factory.comment({"created_at": new Date(2000).toJSON(), "text": "2"}));
+      this.view.appendComment(factory.comment({"created_at": new Date(4000).toJSON(), "text": "4"}));
+      this.view.appendComment(factory.comment({"created_at": new Date(5000).toJSON(), "text": "5"}));
+      this.view.appendComment(factory.comment({"created_at": new Date(6000).toJSON(), "text": "6"}));
+      this.view.appendComment(factory.comment({"created_at": new Date(1000).toJSON(), "text": "1"}));
+      this.view.appendComment(factory.comment({"created_at": new Date(3000).toJSON(), "text": "3"}));
+
+      expect(this.view.$(".comments div.comment.media").length).toEqual(6);
+      expect(this.view.$(".comments div.comment.media div.comment-content p").text()).toEqual("123456");
+    });
   });
 
   describe("expandComments", function() {
-    it("refills the comment textbox on success", function() {
+    it("doesn't drop the comment textbox value on success", function() {
       this.view.render();
       this.view.$("textarea").val("great post!");
       this.view.expandComments();