Merge branch 'diff-comments-avatars' into 'master'

Added discussion comments avatars to diff

Closes #17237

See merge request !7357

app/assets/javascripts/diff_notes/components/diff_note_avatars.js

+/* global CommentsStore Cookies notes */
+import Vue from 'vue';
+import collapseIcon from '../icons/collapse_icon.svg';
+
+(() => {
+  const DiffNoteAvatars = Vue.extend({
+    props: ['discussionId'],
+    data() {
+      return {
+        isVisible: false,
+        lineType: '',
+        storeState: CommentsStore.state,
+        shownAvatars: 3,
+        collapseIcon,
+      };
+    },
+    template: `
+      <div class="diff-comment-avatar-holders"
+        v-show="notesCount !== 0">
+        <div v-if="!isVisible">
+          <img v-for="note in notesSubset"
+            class="avatar diff-comment-avatar has-tooltip js-diff-comment-avatar"
+            width="19"
+            height="19"
+            role="button"
+            data-container="body"
+            data-placement="top"
+            :data-line-type="lineType"
+            :title="note.authorName + ': ' + note.noteTruncated"
+            :src="note.authorAvatar"
+            @click="clickedAvatar($event)" />
+          <span v-if="notesCount > shownAvatars"
+            class="diff-comments-more-count has-tooltip js-diff-comment-avatar"
+            data-container="body"
+            data-placement="top"
+            ref="extraComments"
+            role="button"
+            :data-line-type="lineType"
+            :title="extraNotesTitle"
+            @click="clickedAvatar($event)">{{ moreText }}</span>
+        </div>
+        <button class="diff-notes-collapse js-diff-comment-avatar"
+          type="button"
+          aria-label="Show comments"
+          :data-line-type="lineType"
+          @click="clickedAvatar($event)"
+          v-if="isVisible"
+          v-html="collapseIcon">
+        </button>
+      </div>
+    `,
+    mounted() {
+      this.$nextTick(() => {
+        this.addNoCommentClass();
+        this.setDiscussionVisible();
+
+        this.lineType = $(this.$el).closest('.diff-line-num').hasClass('old_line') ? 'old' : 'new';
+      });
+
+      $(document).on('toggle.comments', () => {
+        this.$nextTick(() => {
+          this.setDiscussionVisible();
+        });
+      });
+    },
+    destroyed() {
+      $(document).off('toggle.comments');
+    },
+    watch: {
+      storeState: {
+        handler() {
+          this.$nextTick(() => {
+            $('.has-tooltip', this.$el).tooltip('fixTitle');
+
+            // We need to add/remove a class to an element that is outside the Vue instance
+            this.addNoCommentClass();
+          });
+        },
+        deep: true,
+      },
+    },
+    computed: {
+      notesSubset() {
+        let notes = [];
+
+        if (this.discussion) {
+          notes = Object.keys(this.discussion.notes)
+            .slice(0, this.shownAvatars)
+            .map(noteId => this.discussion.notes[noteId]);
+        }
+
+        return notes;
+      },
+      extraNotesTitle() {
+        if (this.discussion) {
+          const extra = this.discussion.notesCount() - this.shownAvatars;
+
+          return `${extra} more comment${extra > 1 ? 's' : ''}`;
+        }
+
+        return '';
+      },
+      discussion() {
+        return this.storeState[this.discussionId];
+      },
+      notesCount() {
+        if (this.discussion) {
+          return this.discussion.notesCount();
+        }
+
+        return 0;
+      },
+      moreText() {
+        const plusSign = this.notesCount < 100 ? '+' : '';
+
+        return `${plusSign}${this.notesCount - this.shownAvatars}`;
+      },
+    },
+    methods: {
+      clickedAvatar(e) {
+        notes.addDiffNote(e);
+
+        // Toggle the active state of the toggle all button
+        this.toggleDiscussionsToggleState();
+
+        this.$nextTick(() => {
+          this.setDiscussionVisible();
+
+          $('.has-tooltip', this.$el).tooltip('fixTitle');
+          $('.has-tooltip', this.$el).tooltip('hide');
+        });
+      },
+      addNoCommentClass() {
+        const notesCount = this.notesCount;
+
+        $(this.$el).closest('.js-avatar-container')
+          .toggleClass('js-no-comment-btn', notesCount > 0)
+          .nextUntil('.js-avatar-container')
+          .toggleClass('js-no-comment-btn', notesCount > 0);
+      },
+      toggleDiscussionsToggleState() {
+        const $notesHolders = $(this.$el).closest('.code').find('.notes_holder');
+        const $visibleNotesHolders = $notesHolders.filter(':visible');
+        const $toggleDiffCommentsBtn = $(this.$el).closest('.diff-file').find('.js-toggle-diff-comments');
+
+        $toggleDiffCommentsBtn.toggleClass('active', $notesHolders.length === $visibleNotesHolders.length);
+      },
+      setDiscussionVisible() {
+        this.isVisible = $(`.diffs .notes[data-discussion-id="${this.discussion.id}"]`).is(':visible');
+      },
+    },
+  });
+
+  Vue.component('diff-note-avatars', DiffNoteAvatars);
+})();

app/assets/javascripts/diff_notes/components/resolve_btn.js

@@ -11,7 +11,10 @@ const Vue = require('vue');
       discussionId: String,
       resolved: Boolean,
       canResolve: Boolean,
-      resolvedBy: String
+      resolvedBy: String,
+      authorName: String,
+      authorAvatar: String,
+      noteTruncated: String,
     },
     data: function () {
       return {
@@ -98,7 +101,16 @@ const Vue = require('vue');
       CommentsStore.delete(this.discussionId, this.noteId);
     },
     created: function () {
-      CommentsStore.create(this.discussionId, this.noteId, this.canResolve, this.resolved, this.resolvedBy);
+      CommentsStore.create({
+        discussionId: this.discussionId,
+        noteId: this.noteId,
+        canResolve: this.canResolve,
+        resolved: this.resolved,
+        resolvedBy: this.resolvedBy,
+        authorName: this.authorName,
+        authorAvatar: this.authorAvatar,
+        noteTruncated: this.noteTruncated,
+      });
 
       this.note = this.discussion.getNote(this.noteId);
     }

app/assets/javascripts/diff_notes/diff_notes_bundle.js

@@ -13,6 +13,7 @@ require('./components/jump_to_discussion');
 require('./components/resolve_btn');
 require('./components/resolve_count');
 require('./components/resolve_discussion_btn');
+require('./components/diff_note_avatars');
 
 $(() => {
   const projectPath = document.querySelector('.merge-request').dataset.projectPath;
@@ -24,6 +25,15 @@ $(() => {
   window.ResolveService = new gl.DiffNotesResolveServiceClass(projectPath);
 
   gl.diffNotesCompileComponents = () => {
+    $('diff-note-avatars').each(function () {
+      const tmp = Vue.extend({
+        template: $(this).get(0).outerHTML
+      });
+      const tmpApp = new tmp().$mount();
+
+      $(this).replaceWith(tmpApp.$el);
+    });
+
     const $components = $(COMPONENT_SELECTOR).filter(function () {
       return $(this).closest('resolve-count').length !== 1;
     });

app/assets/javascripts/diff_notes/icons/collapse_icon.svg

+<svg width="11" height="11" viewBox="0 0 9 13"><path d="M2.57568253,6.49866948 C2.50548852,6.57199715 2.44637866,6.59708255 2.39835118,6.57392645 C2.3503237,6.55077034 2.32631032,6.48902165 2.32631032,6.38867852 L2.32631032,-2.13272614 C2.32631032,-2.23306927 2.3503237,-2.29481796 2.39835118,-2.31797406 C2.44637866,-2.34113017 2.50548852,-2.31604477 2.57568253,-2.24271709 L6.51022184,1.86747129 C6.53977721,1.8983461 6.56379059,1.93500939 6.5822627,1.97746225 L6.5822627,2.27849013 C6.56379059,2.31708364 6.53977721,2.35374693 6.51022184,2.38848109 L2.57568253,6.49866948 Z" transform="translate(4.454287, 2.127976) rotate(90.000000) translate(-4.454287, -2.127976) "></path><path d="M3.74312342,2.09553332 C3.74312342,1.99519019 3.77821989,1.9083561 3.8484139,1.83502843 C3.91860791,1.76170075 4.00173115,1.72503747 4.09778611,1.72503747 L4.80711151,1.72503747 C4.90316647,1.72503747 4.98628971,1.76170075 5.05648372,1.83502843 C5.12667773,1.9083561 5.16177421,1.99519019 5.16177421,2.09553332 L5.16177421,10.2464421 C5.16177421,10.3467853 5.12667773,10.4336194 5.05648372,10.506947 C4.98628971,10.5802747 4.90316647,10.616938 4.80711151,10.616938 L4.09778611,10.616938 C4.00173115,10.616938 3.91860791,10.5802747 3.8484139,10.506947 C3.77821989,10.4336194 3.74312342,10.3467853 3.74312342,10.2464421 L3.74312342,2.09553332 Z" transform="translate(4.452449, 6.170988) rotate(-90.000000) translate(-4.452449, -6.170988) "></path><path d="M2.57568253,14.6236695 C2.50548852,14.6969971 2.44637866,14.7220826 2.39835118,14.6989264 C2.3503237,14.6757703 2.32631032,14.6140216 2.32631032,14.5136785 L2.32631032,5.99227386 C2.32631032,5.89193073 2.3503237,5.83018204 2.39835118,5.80702594 C2.44637866,5.78386983 2.50548852,5.80895523 2.57568253,5.88228291 L6.51022184,9.99247129 C6.53977721,10.0233461 6.56379059,10.0600094 6.5822627,10.1024622 L6.5822627,10.4034901 C6.56379059,10.4420836 6.53977721,10.4787469 6.51022184,10.5134811 L2.57568253,14.6236695 Z" transform="translate(4.454287, 10.252976) scale(1, -1) rotate(90.000000) translate(-4.454287, -10.252976) "></path></svg>

app/assets/javascripts/diff_notes/models/discussion.js

@@ -10,8 +10,8 @@ class DiscussionModel {
     this.canResolve = false;
   }
 
-  createNote (noteId, canResolve, resolved, resolved_by) {
-    Vue.set(this.notes, noteId, new NoteModel(this.id, noteId, canResolve, resolved, resolved_by));
+  createNote (noteObj) {
+    Vue.set(this.notes, noteObj.noteId, new NoteModel(this.id, noteObj));
   }
 
   deleteNote (noteId) {

app/assets/javascripts/diff_notes/models/note.js

 /* eslint-disable camelcase, no-unused-vars */
 
 class NoteModel {
-  constructor(discussionId, noteId, canResolve, resolved, resolved_by) {
+  constructor(discussionId, noteObj) {
     this.discussionId = discussionId;
-    this.id = noteId;
-    this.canResolve = canResolve;
-    this.resolved = resolved;
-    this.resolved_by = resolved_by;
+    this.id = noteObj.noteId;
+    this.canResolve = noteObj.canResolve;
+    this.resolved = noteObj.resolved;
+    this.resolved_by = noteObj.resolvedBy;
+    this.authorName = noteObj.authorName;
+    this.authorAvatar = noteObj.authorAvatar;
+    this.noteTruncated = noteObj.noteTruncated;
   }
 }
 

app/assets/javascripts/diff_notes/stores/comments.js

@@ -21,10 +21,10 @@
 
       return discussion;
     },
-    create: function (discussionId, noteId, canResolve, resolved, resolved_by) {
-      const discussion = this.createDiscussion(discussionId);
+    create: function (noteObj) {
+      const discussion = this.createDiscussion(noteObj.discussionId);
 
-      discussion.createNote(noteId, canResolve, resolved, resolved_by);
+      discussion.createNote(noteObj);
     },
     update: function (discussionId, noteId, resolved, resolved_by) {
       const discussion = this.state[discussionId];

app/assets/javascripts/files_comment_button.js

@@ -38,6 +38,9 @@
     FilesCommentButton.prototype.render = function(e) {
       var $currentTarget, buttonParentElement, lineContentElement, textFileElement, $button;
       $currentTarget = $(e.currentTarget);
+
+      if ($currentTarget.hasClass('js-no-comment-btn')) return;
+
       lineContentElement = this.getLineContent($currentTarget);
       buttonParentElement = this.getButtonParent($currentTarget);
 

app/assets/javascripts/main.js

@@ -342,11 +342,11 @@ require('./zen_mode');
       var notesHolders = $this.closest('.diff-file').find('.notes_holder');
       $this.toggleClass('active');
       if ($this.hasClass('active')) {
-        notesHolders.show().find('.hide').show();
+        notesHolders.show().find('.hide, .content').show();
       } else {
-        notesHolders.hide();
+        notesHolders.hide().find('.content').hide();
       }
-      $this.trigger('blur');
+      $(document).trigger('toggle.comments');
       return e.preventDefault();
     });
     $document.off('click', '.js-confirm-danger');

app/assets/javascripts/notes.js

@@ -312,7 +312,7 @@ require('./task_list');
      */
 
     Notes.prototype.renderDiscussionNote = function(note) {
-      var discussionContainer, form, note_html, row;
+      var discussionContainer, form, note_html, row, lineType, diffAvatarContainer;
       if (!this.isNewNote(note)) {
         return;
       }
@@ -322,6 +322,8 @@ require('./task_list');
         form = $("#new-discussion-note-form-" + note.original_discussion_id);
       }
       row = form.closest("tr");
+      lineType = this.isParallelView() ? form.find('#line_type').val() : 'old';
+      diffAvatarContainer = row.prevAll('.line_holder').first().find('.js-avatar-container.' + lineType + '_line');
       note_html = $(note.html);
       note_html.renderGFM();
       // is this the first note of discussion?
@@ -330,10 +332,26 @@ require('./task_list');
         discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']");
       }
       if (discussionContainer.length === 0) {
-        // insert the note and the reply button after the temp row
-        row.after(note.diff_discussion_html);
-        // remove the note (will be added again below)
-        row.next().find(".note").remove();
+        if (!this.isParallelView() || row.hasClass('js-temp-notes-holder')) {
+          // insert the note and the reply button after the temp row
+          row.after(note.diff_discussion_html);
+
+          // remove the note (will be added again below)
+          row.next().find(".note").remove();
+        } else {
+          // Merge new discussion HTML in
+          var $discussion = $(note.diff_discussion_html);
+          var $notes = $discussion.find('.notes[data-discussion-id="' + note.discussion_id + '"]');
+          var contentContainerClass = '.' + $notes.closest('.notes_content')
+            .attr('class')
+            .split(' ')
+            .join('.');
+
+          // remove the note (will be added again below)
+          $notes.find('.note').remove();
+
+          row.find(contentContainerClass + ' .content').append($notes.closest('.content').children());
+        }
         // Before that, the container didn't exist
         discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']");
         // Add note to 'Changes' page discussions
@@ -347,14 +365,40 @@ require('./task_list');
         discussionContainer.append(note_html);
       }
 
-      if (typeof gl.diffNotesCompileComponents !== 'undefined') {
+      if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_id) {
         gl.diffNotesCompileComponents();
+        this.renderDiscussionAvatar(diffAvatarContainer, note);
       }
 
       gl.utils.localTimeAgo($('.js-timeago'), false);
       return this.updateNotesCount(1);
     };
 
+    Notes.prototype.getLineHolder = function(changesDiscussionContainer) {
+      return $(changesDiscussionContainer).closest('.notes_holder')
+        .prevAll('.line_holder')
+        .first()
+        .get(0);
+    };
+
+    Notes.prototype.renderDiscussionAvatar = function(diffAvatarContainer, note) {
+      var commentButton = diffAvatarContainer.find('.js-add-diff-note-button');
+      var avatarHolder = diffAvatarContainer.find('.diff-comment-avatar-holders');
+
+      if (!avatarHolder.length) {
+        avatarHolder = document.createElement('diff-note-avatars');
+        avatarHolder.setAttribute('discussion-id', note.discussion_id);
+
+        diffAvatarContainer.append(avatarHolder);
+
+        gl.diffNotesCompileComponents();
+      }
+
+      if (commentButton.length) {
+        commentButton.remove();
+      }
+    };
+
     /*
     Called in response the main target form has been successfully submitted.
 
@@ -592,9 +636,14 @@ require('./task_list');
      */
 
     Notes.prototype.removeNote = function(e) {
-      var noteId;
-      noteId = $(e.currentTarget).closest(".note").attr("id");
-      $(".note[id='" + noteId + "']").each((function(_this) {
+      var noteElId, noteId, dataNoteId, $note, lineHolder;
+      $note = $(e.currentTarget).closest('.note');
+      noteElId = $note.attr('id');
+      noteId = $note.attr('data-note-id');
+      lineHolder = $(e.currentTarget).closest('.notes[data-discussion-id]')
+        .closest('.notes_holder')
+        .prev('.line_holder');
+      $(".note[id='" + noteElId + "']").each((function(_this) {
         // A same note appears in the "Discussion" and in the "Changes" tab, we have
         // to remove all. Using $(".note[id='noteId']") ensure we get all the notes,
         // where $("#noteId") would return only one.
@@ -604,17 +653,26 @@ require('./task_list');
           notes = note.closest(".notes");
 
           if (typeof gl.diffNotesCompileComponents !== 'undefined') {
-            if (gl.diffNoteApps[noteId]) {
-              gl.diffNoteApps[noteId].$destroy();
+            if (gl.diffNoteApps[noteElId]) {
+              gl.diffNoteApps[noteElId].$destroy();
             }
           }
 
+          note.remove();
+
           // check if this is the last note for this line
-          if (notes.find(".note").length === 1) {
+          if (notes.find(".note").length === 0) {
+            var notesTr = notes.closest("tr");
+
             // "Discussions" tab
             notes.closest(".timeline-entry").remove();
-            // "Changes" tab / commit view
-            notes.closest("tr").remove();
+
+            if (!_this.isParallelView() || notesTr.find('.note').length === 0) {
+              // "Changes" tab / commit view
+              notesTr.remove();
+            } else {
+              notes.closest('.content').empty();
+            }
           }
           return note.remove();
         };
@@ -707,15 +765,16 @@ require('./task_list');
      */
 
     Notes.prototype.addDiffNote = function(e) {
-      var $link, addForm, hasNotes, lineType, newForm, nextRow, noteForm, notesContent, notesContentSelector, replyButton, row, rowCssToAdd, targetContent;
+      var $link, addForm, hasNotes, lineType, newForm, nextRow, noteForm, notesContent, notesContentSelector, replyButton, row, rowCssToAdd, targetContent, isDiffCommentAvatar;
       e.preventDefault();
-      $link = $(e.currentTarget);
+      $link = $(e.currentTarget || e.target);
       row = $link.closest("tr");
       nextRow = row.next();
       hasNotes = nextRow.is(".notes_holder");
       addForm = false;
       notesContentSelector = ".notes_content";
       rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"><div class=\"content\"></div></td></tr>";
+      isDiffCommentAvatar = $link.hasClass('js-diff-comment-avatar');
       // In parallel view, look inside the correct left/right pane
       if (this.isParallelView()) {
         lineType = $link.data("lineType");
@@ -723,7 +782,9 @@ require('./task_list');
         rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line old\"></td><td class=\"notes_content parallel old\"><div class=\"content\"></div></td><td class=\"notes_line new\"></td><td class=\"notes_content parallel new\"><div class=\"content\"></div></td></tr>";
       }
       notesContentSelector += " .content";
-      if (hasNotes) {
+      notesContent = nextRow.find(notesContentSelector);
+
+      if (hasNotes && !isDiffCommentAvatar) {
         nextRow.show();
         notesContent = nextRow.find(notesContentSelector);
         if (notesContent.length) {
@@ -740,13 +801,21 @@ require('./task_list');
             }
           }
         }
-      } else {
+      } else if (!isDiffCommentAvatar) {
         // add a notes row and insert the form
         row.after(rowCssToAdd);
         nextRow = row.next();
         notesContent = nextRow.find(notesContentSelector);
         addForm = true;
+      } else {
+        nextRow.show();
+        notesContent.toggle(!notesContent.is(':visible'));
+
+        if (!nextRow.find('.content:not(:empty)').is(':visible')) {
+          nextRow.hide();
+        }
       }
+
       if (addForm) {
         newForm = this.formClone.clone();
         newForm.appendTo(notesContent);

app/assets/stylesheets/pages/diff.scss

@@ -113,6 +113,10 @@
       td.line_content.parallel {
         width: 46%;
       }
+
+      .add-diff-note {
+        margin-left: -55px;
+      }
     }
 
     .old_line,
@@ -490,3 +494,103 @@
     }
   }
 }
+
+.diff-comment-avatar-holders {
+  position: absolute;
+  height: 19px;
+  width: 19px;
+  margin-left: -15px;
+
+  &:hover {
+    .diff-comment-avatar,
+    .diff-comments-more-count {
+      @for $i from 1 through 4 {
+        $x-pos: 14px;
+
+        &:nth-child(#{$i}) {
+          @if $i == 4 {
+            $x-pos: 14.5px;
+          }
+
+          transform: translateX((($i * $x-pos) - $x-pos));
+
+          &:hover {
+            transform: translateX((($i * $x-pos) - $x-pos)) scale(1.2);
+          }
+        }
+      }
+    }
+
+    .diff-comments-more-count {
+      padding-left: 2px;
+      padding-right: 2px;
+      width: auto;
+    }
+  }
+}
+
+.diff-comment-avatar,
+.diff-comments-more-count {
+  position: absolute;
+  left: 0;
+  width: 19px;
+  height: 19px;
+  margin-right: 0;
+  border-color: $white-light;
+  cursor: pointer;
+  transition: all .1s ease-out;
+
+  @for $i from 1 through 4 {
+    &:nth-child(#{$i}) {
+      z-index: (4 - $i);
+    }
+  }
+}
+
+.diff-comments-more-count {
+  width: 19px;
+  min-width: 19px;
+  padding-left: 0;
+  padding-right: 0;
+  overflow: hidden;
+}
+
+.diff-comments-more-count,
+.diff-notes-collapse {
+  background-color: $gray-darkest;
+  color: $white-light;
+  border: 1px solid $white-light;
+  border-radius: 1em;
+  font-family: $regular_font;
+  font-size: 9px;
+  line-height: 17px;
+  text-align: center;
+}
+
+.diff-notes-collapse {
+  position: relative;
+  width: 19px;
+  height: 19px;
+  padding: 0;
+  transition: transform .1s ease-out;
+
+  svg {
+    position: absolute;
+    left: 50%;
+    top: 50%;
+    margin-left: -5.5px;
+    margin-top: -5.5px;
+  }
+
+  path {
+    fill: $white-light;
+  }
+
+  &:hover {
+    transform: scale(1.2);
+  }
+
+  &:focus {
+    outline: 0;
+  }
+}

app/views/discussions/_diff_discussion.html.haml

@@ -2,5 +2,5 @@
 %tr.notes_holder{ class: ('hide' unless expanded) }
   %td.notes_line{ colspan: 2 }
   %td.notes_content
-    .content
+    .content{ class: ('hide' unless expanded) }
       = render "discussions/notes", discussion: discussion

app/views/projects/diffs/_line.html.haml

 - email = local_assigns.fetch(:email, false)
 - plain = local_assigns.fetch(:plain, false)
+- discussions = local_assigns.fetch(:discussions, nil)
 - type = line.type
 - line_code = diff_file.line_code(line)
-%tr.line_holder{ plain ? { class: type} : { class: type, id: line_code } }
+- if discussions && !line.meta?
+  - discussion = discussions[line_code]
+%tr.line_holder{ class: type, id: (line_code unless plain) }
   - case type
   - when 'match'
     = diff_match_line line.old_pos, line.new_pos, text: line.text
@@ -11,12 +14,14 @@
     %td.new_line.diff-line-num
     %td.line_content.match= line.text
   - else
-    %td.old_line.diff-line-num{ class: type, data: { linenumber: line.old_pos } }
+    %td.old_line.diff-line-num.js-avatar-container{ class: type, data: { linenumber: line.old_pos } }
       - link_text = type == "new" ? " " : line.old_pos
       - if plain
         = link_text
       - else
         %a{ href: "##{line_code}", data: { linenumber: link_text } }
+      - if discussion && !plain
+        %diff-note-avatars{ "discussion-id" => discussion.id }
     %td.new_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } }
       - link_text = type == "old" ? " " : line.new_pos
       - if plain
@@ -29,9 +34,6 @@
       - else
         = diff_line_content(line.text)
 
-- discussions = local_assigns.fetch(:discussions, nil)
-- if discussions && !line.meta?
-  - discussion = discussions[line_code]
-  - if discussion
-    - discussion_expanded = local_assigns.fetch(:discussion_expanded, discussion.expanded?)
-    = render "discussions/diff_discussion", discussion: discussion, expanded: discussion_expanded
+- if discussion
+  - discussion_expanded = local_assigns.fetch(:discussion_expanded, discussion.expanded?)
+  = render "discussions/diff_discussion", discussion: discussion, expanded: discussion_expanded

app/views/projects/diffs/_parallel_view.html.haml

@@ -4,6 +4,9 @@
     - diff_file.parallel_diff_lines.each do |line|
       - left = line[:left]
       - right = line[:right]
+      - last_line = right.new_pos if right
+      - unless @diff_notes_disabled
+        - discussion_left, discussion_right = parallel_diff_discussions(left, right, diff_file)
       %tr.line_holder.parallel
         - if left
           - case left.type
@@ -15,8 +18,10 @@
           - else
             - left_line_code = diff_file.line_code(left)
             - left_position = diff_file.position(left)
-            %td.old_line.diff-line-num{ id: left_line_code, class: left.type, data: { linenumber: left.old_pos } }
+            %td.old_line.diff-line-num.js-avatar-container{ id: left_line_code, class: left.type, data: { linenumber: left.old_pos } }
               %a{ href: "##{left_line_code}", data: { linenumber: left.old_pos } }
+              - if discussion_left
+                %diff-note-avatars{ "discussion-id" => discussion_left.id }
             %td.line_content.parallel.noteable_line{ class: left.type, data: diff_view_line_data(left_line_code, left_position, 'old') }= diff_line_content(left.text)
         - else
           %td.old_line.diff-line-num.empty-cell
@@ -32,17 +37,17 @@
           - else
             - right_line_code = diff_file.line_code(right)
             - right_position = diff_file.position(right)
-            %td.new_line.diff-line-num{ id: right_line_code, class: right.type, data: { linenumber: right.new_pos } }
+            %td.new_line.diff-line-num.js-avatar-container{ id: right_line_code, class: right.type, data: { linenumber: right.new_pos } }
               %a{ href: "##{right_line_code}", data: { linenumber: right.new_pos } }
+              - if discussion_right
+                %diff-note-avatars{ "discussion-id" => discussion_right.id }
             %td.line_content.parallel.noteable_line{ class: right.type, data: diff_view_line_data(right_line_code, right_position, 'new') }= diff_line_content(right.text)
         - else
           %td.old_line.diff-line-num.empty-cell
           %td.line_content.parallel
 
-      - unless @diff_notes_disabled
-        - discussion_left, discussion_right = parallel_diff_discussions(left, right, diff_file)
-        - if discussion_left || discussion_right
-          = render "discussions/parallel_diff_discussion", discussion_left: discussion_left, discussion_right: discussion_right
+      - if discussion_left || discussion_right
+        = render "discussions/parallel_diff_discussion", discussion_left: discussion_left, discussion_right: discussion_right
     - if !diff_file.new_file && !diff_file.deleted_file && diff_file.diff_lines.any?
       - last_line = diff_file.diff_lines.last
       - if last_line.new_pos < total_lines

app/views/projects/notes/_note.html.haml

@@ -2,7 +2,7 @@
 - return if note.cross_reference_not_visible_for?(current_user)
 
 - note_editable = note_editable?(note)
-%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable} }
+%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable, note_id: note.id} }
   .timeline-entry-inner
     .timeline-icon
       %a{ href: user_path(note.author) }
@@ -30,11 +30,15 @@
 
             - if note.resolvable?
               - can_resolve = can?(current_user, :resolve_note, note)
-              %resolve-btn{ "discussion-id" => "#{note.discussion_id}",
+              %resolve-btn{ "project-path" => project_path(note.project),
+                  "discussion-id" => note.discussion_id,
                   ":note-id" => note.id,
                   ":resolved" => note.resolved?,
                   ":can-resolve" => can_resolve,
-                  "resolved-by" => "#{note.resolved_by.try(:name)}",
+                  ":author-name" => "'#{j(note.author.name)}'",
+                  "author-avatar" => note.author.avatar_url,
+                  ":note-truncated" => "'#{truncate(note.note, length: 17)}'",
+                  ":resolved-by" => "'#{j(note.resolved_by.try(:name))}'",
                   "v-show" => "#{can_resolve || note.resolved?}",
                   "inline-template" => true,
                   "ref" => "note_#{note.id}" }

app/views/shared/icons/_collapse.svg.erb

+<svg width="<%= size %>" height="<%= size %>" viewBox="0 0 9 13"><path d="M2.57568253,6.49866948 C2.50548852,6.57199715 2.44637866,6.59708255 2.39835118,6.57392645 C2.3503237,6.55077034 2.32631032,6.48902165 2.32631032,6.38867852 L2.32631032,-2.13272614 C2.32631032,-2.23306927 2.3503237,-2.29481796 2.39835118,-2.31797406 C2.44637866,-2.34113017 2.50548852,-2.31604477 2.57568253,-2.24271709 L6.51022184,1.86747129 C6.53977721,1.8983461 6.56379059,1.93500939 6.5822627,1.97746225 L6.5822627,2.27849013 C6.56379059,2.31708364 6.53977721,2.35374693 6.51022184,2.38848109 L2.57568253,6.49866948 Z" transform="translate(4.454287, 2.127976) rotate(90.000000) translate(-4.454287, -2.127976) "></path><path d="M3.74312342,2.09553332 C3.74312342,1.99519019 3.77821989,1.9083561 3.8484139,1.83502843 C3.91860791,1.76170075 4.00173115,1.72503747 4.09778611,1.72503747 L4.80711151,1.72503747 C4.90316647,1.72503747 4.98628971,1.76170075 5.05648372,1.83502843 C5.12667773,1.9083561 5.16177421,1.99519019 5.16177421,2.09553332 L5.16177421,10.2464421 C5.16177421,10.3467853 5.12667773,10.4336194 5.05648372,10.506947 C4.98628971,10.5802747 4.90316647,10.616938 4.80711151,10.616938 L4.09778611,10.616938 C4.00173115,10.616938 3.91860791,10.5802747 3.8484139,10.506947 C3.77821989,10.4336194 3.74312342,10.3467853 3.74312342,10.2464421 L3.74312342,2.09553332 Z" transform="translate(4.452449, 6.170988) rotate(-90.000000) translate(-4.452449, -6.170988) "></path><path d="M2.57568253,14.6236695 C2.50548852,14.6969971 2.44637866,14.7220826 2.39835118,14.6989264 C2.3503237,14.6757703 2.32631032,14.6140216 2.32631032,14.5136785 L2.32631032,5.99227386 C2.32631032,5.89193073 2.3503237,5.83018204 2.39835118,5.80702594 C2.44637866,5.78386983 2.50548852,5.80895523 2.57568253,5.88228291 L6.51022184,9.99247129 C6.53977721,10.0233461 6.56379059,10.0600094 6.5822627,10.1024622 L6.5822627,10.4034901 C6.56379059,10.4420836 6.53977721,10.4787469 6.51022184,10.5134811 L2.57568253,14.6236695 Z" transform="translate(4.454287, 10.252976) scale(1, -1) rotate(90.000000) translate(-4.454287, -10.252976) "></path></svg>

spec/features/merge_requests/diff_notes_avatars_spec.rb

+require 'spec_helper'
+
+feature 'Diff note avatars', feature: true, js: true do
+  include WaitForAjax
+
+  let(:user)          { create(:user) }
+  let(:project)       { create(:project, :public) }
+  let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") }
+  let(:path)          { "files/ruby/popen.rb" }
+  let(:position) do
+    Gitlab::Diff::Position.new(
+      old_path: path,
+      new_path: path,
+      old_line: nil,
+      new_line: 9,
+      diff_refs: merge_request.diff_refs
+    )
+  end
+  let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) }
+
+  before do
+    project.team << [user, :master]
+    login_as user
+  end
+
+  %w(inline parallel).each do |view|
+    context "#{view} view" do
+      before do
+        visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: view)
+
+        wait_for_ajax
+      end
+
+      it 'shows note avatar' do
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          find('.diff-notes-collapse').click
+
+          expect(page).to have_selector('img.js-diff-comment-avatar', count: 1)
+        end
+      end
+
+      it 'shows comment on note avatar' do
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          find('.diff-notes-collapse').click
+
+          expect(first('img.js-diff-comment-avatar')["title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}")
+        end
+      end
+
+      it 'toggles comments when clicking avatar' do
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          find('.diff-notes-collapse').click
+        end
+
+        expect(page).to have_selector('.notes_holder', visible: false)
+
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          first('img.js-diff-comment-avatar').click
+        end
+
+        expect(page).to have_selector('.notes_holder')
+      end
+
+      it 'removes avatar when note is deleted' do
+        page.within find(".note-row-#{note.id}") do
+          find('.js-note-delete').click
+        end
+
+        wait_for_ajax
+
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          expect(page).not_to have_selector('img.js-diff-comment-avatar')
+        end
+      end
+
+      it 'adds avatar when commenting' do
+        click_button 'Reply...'
+
+        page.within '.js-discussion-note-form' do
+          find('.js-note-text').native.send_keys('Test')
+
+          click_button 'Comment'
+
+          wait_for_ajax
+        end
+
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          find('.diff-notes-collapse').click
+
+          expect(page).to have_selector('img.js-diff-comment-avatar', count: 2)
+        end
+      end
+
+      it 'adds multiple comments' do
+        3.times do
+          click_button 'Reply...'
+
+          page.within '.js-discussion-note-form' do
+            find('.js-note-text').native.send_keys('Test')
+
+            find('.js-comment-button').trigger 'click'
+
+            wait_for_ajax
+          end
+        end
+
+        page.within find("[id='#{position.line_code(project.repository)}']") do
+          find('.diff-notes-collapse').click
+
+          expect(page).to have_selector('img.js-diff-comment-avatar', count: 3)
+          expect(find('.diff-comments-more-count')).to have_content '+1'
+        end
+      end
+
+      context 'multiple comments' do
+        before do
+          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
+          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
+          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
+
+          visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: view)
+
+          wait_for_ajax
+        end
+
+        it 'shows extra comment count' do
+          page.within find("[id='#{position.line_code(project.repository)}']") do
+            find('.diff-notes-collapse').click
+
+            expect(find('.diff-comments-more-count')).to have_content '+1'
+          end
+        end
+      end
+    end
+  end
+end

spec/javascripts/diff_comments_store_spec.js

@@ -7,7 +7,16 @@ require('~/diff_notes/stores/comments');
 
 (() => {
   function createDiscussion(noteId = 1, resolved = true) {
-    CommentsStore.create('a', noteId, true, resolved, 'test');
+    CommentsStore.create({
+      discussionId: 'a',
+      noteId,
+      canResolve: true,
+      resolved,
+      resolvedBy: 'test',
+      authorName: 'test',
+      authorAvatar: 'test',
+      noteTruncated: 'test...',
+    });
   }
 
   beforeEach(() => {