Merge branch '29977-style-comments-and-system-notes-real-time-updates' into 'master'

Add renderNote animation and added spec

Closes #29977

See merge request !10623

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);

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;
+}

app/assets/stylesheets/framework/variables.scss

@@ -457,6 +457,11 @@ $label-inverse-bg: #333;
 $label-remove-border: rgba(0, 0, 0, .1);
 $label-border-radius: 100px;
 
+/*
+* Animation
+*/
+$fade-in-duration: 200ms;
+
 /*
 * Lint
 */

changelogs/unreleased/29977-style-comments-and-system-notes-real-time-updates.yml

+---
+title: Added quick-update (fade-in) animation to newly rendered notes
+merge_request: 10623
+author:

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!"

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

spec/features/issues/note_polling_spec.rb

 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!')

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);