Add renderNote animation and added spec

8 files changed, 196 insertions, 32 deletions

diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 15f7a813626..974fb0d83da 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -308,8 +308,10 @@ require('./task_list');
 
       if (this.isNewNote(note)) {
         this.note_ids.push(note.id);
-        $notesList = $('ul.main-notes-list');
-        $notesList.append(note.html).syntaxHighlight();
+
+        $notesList = window.$('ul.main-notes-list');
+        Notes.animateAppendNote(note.html, $notesList);
+
         // Update datetime format on the recent note
         gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false);
         this.collapseLongCommitList();
@@ -348,7 +350,7 @@ require('./task_list');
       lineType = this.isParallelView() ? form.find('#line_type').val() : 'old';
       diffAvatarContainer = row.prevAll('.line_holder').first().find('.js-avatar-container.' + lineType + '_line');
       // is this the first note of discussion?
-      discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']");
+      discussionContainer = window.$(`.notes[data-discussion-id="${note.discussion_id}"]`);
       if (!discussionContainer.length) {
         discussionContainer = form.closest('.discussion').find('.notes');
       }
@@ -370,14 +372,13 @@ require('./task_list');
             row.find(contentContainerClass + ' .content').append($notes.closest('.content').children());
           }
         }
-
         // Init discussion on 'Discussion' page if it is merge request page
-        if ($('body').attr('data-page').indexOf('projects:merge_request') === 0 || !note.diff_discussion_html) {
-          $('ul.main-notes-list').append($(note.discussion_html).renderGFM());
+        if (window.$('body').attr('data-page').indexOf('projects:merge_request') === 0 || !note.diff_discussion_html) {
+          Notes.animateAppendNote(note.discussion_html, window.$('ul.main-notes-list'));
         }
       } else {
         // append new note to all matching discussions
-        discussionContainer.append($(note.html).renderGFM());
+        Notes.animateAppendNote(note.html, discussionContainer);
       }
 
       if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_resolvable) {
@@ -1063,6 +1064,13 @@ require('./task_list');
       return $form;
     };
 
+    Notes.animateAppendNote = function(noteHTML, $notesList) {
+      const $note = window.$(noteHTML);
+
+      $note.addClass('fade-in').renderGFM();
+      $notesList.append($note);
+    };
+
     return Notes;
   })();
 }).call(window);
diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss
index 90935b9616b..7c50b80fd2b 100644
--- a/app/assets/stylesheets/framework/animations.scss
+++ b/app/assets/stylesheets/framework/animations.scss
@@ -145,3 +145,17 @@ a {
 .dropdown-menu-nav a {
   transition: none;
 }
+
+@keyframes fadeIn {
+  0% {
+    opacity: 0;
+  }
+
+  100% {
+    opacity: 1;
+  }
+}
+
+.fade-in {
+  animation: fadeIn $fade-in-duration 1;
+}
diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss
index 20ef9a774e4..3ef6ec3f912 100644
--- a/app/assets/stylesheets/framework/variables.scss
+++ b/app/assets/stylesheets/framework/variables.scss
@@ -458,6 +458,11 @@ $label-remove-border: rgba(0, 0, 0, .1);
 $label-border-radius: 100px;
 
 /*
+* Animation
+*/
+$fade-in-duration: 200ms;
+
+/*
 * Lint
 */
 $lint-incorrect-color: $red-500;
diff --git a/changelogs/unreleased/29977-style-comments-and-system-notes-real-time-updates.yml b/changelogs/unreleased/29977-style-comments-and-system-notes-real-time-updates.yml
new file mode 100644
index 00000000000..c1640777e12
--- /dev/null
+++ b/changelogs/unreleased/29977-style-comments-and-system-notes-real-time-updates.yml
@@ -0,0 +1,4 @@
+---
+title: Added quick-update (fade-in) animation to newly rendered notes
+merge_request: 10623
+author:
diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature
index 27fa67c1843..4dee0cd23dc 100644
--- a/features/project/issues/issues.feature
+++ b/features/project/issues/issues.feature
@@ -177,9 +177,3 @@ Feature: Project Issues
     And I should not see labels field
     And I submit new issue "500 error on profile"
     Then I should see issue "500 error on profile"
-
-  @javascript
-  Scenario: Another user adds a comment to issue I'm currently viewing
-    Given I visit issue page "Release 0.4"
-    And another user adds a comment with text "Yay!" to issue "Release 0.4"
-    Then I should see a new comment with text "Yay!"
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index c0dc48f1bb2..637e6568267 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -345,17 +345,6 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
     end
   end
 
-  step 'another user adds a comment with text "Yay!" to issue "Release 0.4"' do
-    issue = Issue.find_by!(title: 'Release 0.4')
-    create(:note_on_issue, noteable: issue, project: project, note: 'Yay!')
-  end
-
-  step 'I should see a new comment with text "Yay!"' do
-    page.within '#notes' do
-      expect(page).to have_content('Yay!')
-    end
-  end
-
   def filter_issue(text)
     fill_in 'issuable_search', with: text
   end
diff --git a/spec/features/issues/note_polling_spec.rb b/spec/features/issues/note_polling_spec.rb
index f5cfe2d666e..378f6de1a78 100644
--- a/spec/features/issues/note_polling_spec.rb
+++ b/spec/features/issues/note_polling_spec.rb
@@ -1,17 +1,15 @@
 require 'spec_helper'
 
-feature 'Issue notes polling' do
-  let!(:project) { create(:project, :public) }
-  let!(:issue) { create(:issue, project: project) }
+feature 'Issue notes polling', :feature, :js do
+  let(:project) { create(:empty_project, :public) }
+  let(:issue) { create(:issue, project: project) }
 
-  background do
+  before do
     visit namespace_project_issue_path(project.namespace, project, issue)
   end
 
-  scenario 'Another user adds a comment to an issue', js: true do
-    note = create(:note, noteable: issue, project: project,
-                         note: 'Looks good!')
-
+  it 'should display the new comment' do
+    note = create(:note, noteable: issue, project: project, note: 'Looks good!')
     page.execute_script('notes.refresh();')
 
     expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!')
diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index d81a5bbb6a5..ca8ee04d955 100644
--- a/spec/javascripts/notes_spec.js
+++ b/spec/javascripts/notes_spec.js
@@ -72,5 +72,157 @@ require('~/lib/utils/text_utility');
         expect(this.autoSizeSpy).toHaveBeenTriggered();
       });
     });
+
+    describe('renderNote', () => {
+      let notes;
+      let note;
+      let $notesList;
+
+      beforeEach(() => {
+        note = {
+          discussion_html: null,
+          valid: true,
+          html: '<div></div>',
+        };
+        $notesList = jasmine.createSpyObj('$notesList', ['find']);
+
+        notes = jasmine.createSpyObj('notes', [
+          'refresh',
+          'isNewNote',
+          'collapseLongCommitList',
+          'updateNotesCount',
+        ]);
+        notes.taskList = jasmine.createSpyObj('tasklist', ['init']);
+        notes.note_ids = [];
+
+        spyOn(window, '$').and.returnValue($notesList);
+        spyOn(gl.utils, 'localTimeAgo');
+        spyOn(Notes, 'animateAppendNote');
+        notes.isNewNote.and.returnValue(true);
+
+        Notes.prototype.renderNote.call(notes, note);
+      });
+
+      it('should query for the notes list', () => {
+        expect(window.$).toHaveBeenCalledWith('ul.main-notes-list');
+      });
+
+      it('should call .animateAppendNote', () => {
+        expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList);
+      });
+    });
+
+    describe('renderDiscussionNote', () => {
+      let discussionContainer;
+      let note;
+      let notes;
+      let $form;
+      let row;
+
+      beforeEach(() => {
+        note = {
+          html: '<li></li>',
+          discussion_html: '<div></div>',
+          discussion_id: 1,
+          discussion_resolvable: false,
+          diff_discussion_html: false,
+        };
+        $form = jasmine.createSpyObj('$form', ['closest', 'find']);
+        row = jasmine.createSpyObj('row', ['prevAll', 'first', 'find']);
+
+        notes = jasmine.createSpyObj('notes', [
+          'isNewNote',
+          'isParallelView',
+          'updateNotesCount',
+        ]);
+        notes.note_ids = [];
+
+        spyOn(gl.utils, 'localTimeAgo');
+        spyOn(Notes, 'animateAppendNote');
+        notes.isNewNote.and.returnValue(true);
+        notes.isParallelView.and.returnValue(false);
+        row.prevAll.and.returnValue(row);
+        row.first.and.returnValue(row);
+        row.find.and.returnValue(row);
+      });
+
+      describe('Discussion root note', () => {
+        let $notesList;
+        let body;
+
+        beforeEach(() => {
+          body = jasmine.createSpyObj('body', ['attr']);
+          discussionContainer = { length: 0 };
+
+          spyOn(window, '$').and.returnValues(discussionContainer, body, $notesList);
+          $form.closest.and.returnValues(row, $form);
+          $form.find.and.returnValues(discussionContainer);
+          body.attr.and.returnValue('');
+
+          Notes.prototype.renderDiscussionNote.call(notes, note, $form);
+        });
+
+        it('should query for the notes list', () => {
+          expect(window.$.calls.argsFor(2)).toEqual(['ul.main-notes-list']);
+        });
+
+        it('should call Notes.animateAppendNote', () => {
+          expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $notesList);
+        });
+      });
+
+      describe('Discussion sub note', () => {
+        beforeEach(() => {
+          discussionContainer = { length: 1 };
+
+          spyOn(window, '$').and.returnValues(discussionContainer);
+          $form.closest.and.returnValues(row);
+
+          Notes.prototype.renderDiscussionNote.call(notes, note, $form);
+        });
+
+        it('should query foor the discussion container', () => {
+          expect(window.$).toHaveBeenCalledWith(`.notes[data-discussion-id="${note.discussion_id}"]`);
+        });
+
+        it('should call Notes.animateAppendNote', () => {
+          expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, discussionContainer);
+        });
+      });
+    });
+
+    describe('animateAppendNote', () => {
+      let noteHTML;
+      let $note;
+      let $notesList;
+
+      beforeEach(() => {
+        noteHTML = '<div></div>';
+        $note = jasmine.createSpyObj('$note', ['addClass', 'renderGFM', 'removeClass']);
+        $notesList = jasmine.createSpyObj('$notesList', ['append']);
+
+        spyOn(window, '$').and.returnValue($note);
+        spyOn(window, 'setTimeout').and.callThrough();
+        $note.addClass.and.returnValue($note);
+        $note.renderGFM.and.returnValue($note);
+
+        Notes.animateAppendNote(noteHTML, $notesList);
+      });
+
+      it('should init the note jquery object', () => {
+        expect(window.$).toHaveBeenCalledWith(noteHTML);
+      });
+
+      it('should call addClass', () => {
+        expect($note.addClass).toHaveBeenCalledWith('fade-in');
+      });
+      it('should call renderGFM', () => {
+        expect($note.renderGFM).toHaveBeenCalledWith();
+      });
+
+      it('should append note to the notes list', () => {
+        expect($notesList.append).toHaveBeenCalledWith($note);
+      });
+    });
   });
 }).call(window);