Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/diffs/components/diff_discussion_reply.vue

+<script>
+import { mapGetters } from 'vuex';
+import NoteSignedOutWidget from '~/notes/components/note_signed_out_widget.vue';
+import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
+import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
+
+export default {
+  name: 'DiffDiscussionReply',
+  components: {
+    NoteSignedOutWidget,
+    ReplyPlaceholder,
+    UserAvatarLink,
+  },
+  props: {
+    hasForm: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    renderReplyPlaceholder: {
+      type: Boolean,
+      required: true,
+    },
+  },
+  computed: {
+    ...mapGetters({
+      currentUser: 'getUserData',
+      userCanReply: 'userCanReply',
+    }),
+  },
+};
+</script>
+
+<template>
+  <div class="discussion-reply-holder d-flex clearfix">
+    <template v-if="userCanReply">
+      <slot v-if="hasForm" name="form"></slot>
+      <template v-else-if="renderReplyPlaceholder">
+        <user-avatar-link
+          :link-href="currentUser.path"
+          :img-src="currentUser.avatar_url"
+          :img-alt="currentUser.name"
+          :img-size="40"
+          class="d-none d-sm-block"
+        />
+        <reply-placeholder
+          class="qa-discussion-reply"
+          :button-text="__('Start a new discussion...')"
+          @onClick="$emit('showNewDiscussionForm')"
+        />
+      </template>
+    </template>
+    <note-signed-out-widget v-else />
+  </div>
+</template>

app/assets/javascripts/diffs/components/diff_discussions.vue

@@ -80,7 +80,6 @@ export default {
           v-show="isExpanded(discussion)"
           :discussion="discussion"
           :render-diff-file="false"
-          :always-expanded="true"
           :discussions-by-diff-order="true"
           :line="line"
           :help-page-path="helpPagePath"

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -151,7 +151,11 @@ export default {
     stickyMonitor(this.$refs.header, contentTop() - fileHeaderHeight - 1, false);
   },
   methods: {
-    ...mapActions('diffs', ['toggleFileDiscussions', 'toggleFullDiff']),
+    ...mapActions('diffs', [
+      'toggleFileDiscussions',
+      'toggleFileDiscussionWrappers',
+      'toggleFullDiff',
+    ]),
     handleToggleFile(e, checkTarget) {
       if (
         !checkTarget ||
@@ -165,7 +169,7 @@ export default {
       this.$emit('showForkMessage');
     },
     handleToggleDiscussions() {
-      this.toggleFileDiscussions(this.diffFile);
+      this.toggleFileDiscussionWrappers(this.diffFile);
     },
     handleFileNameClick(e) {
       const isLinkToOtherPage =

app/assets/javascripts/diffs/components/diff_gutter_avatars.vue

 <script>
-import { mapActions } from 'vuex';
 import Icon from '~/vue_shared/components/icon.vue';
 import { pluralize, truncate } from '~/lib/utils/text_utility';
 import UserAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue';
@@ -19,11 +18,13 @@ export default {
       type: Array,
       required: true,
     },
+    discussionsExpanded: {
+      type: Boolean,
+      required: false,
+      default: false,
     },
-  computed: {
-    discussionsExpanded() {
-      return this.discussions.every(discussion => discussion.expanded);
   },
+  computed: {
     allDiscussions() {
       return this.discussions.reduce((acc, note) => acc.concat(note.notes), []);
     },
@@ -45,26 +46,14 @@ export default {
     },
   },
   methods: {
-    ...mapActions(['toggleDiscussion']),
     getTooltipText(noteData) {
       let { note } = noteData;
-
       if (note.length > LENGTH_OF_AVATAR_TOOLTIP) {
         note = truncate(note, LENGTH_OF_AVATAR_TOOLTIP);
       }
 
       return `${noteData.author.name}: ${note}`;
     },
-    toggleDiscussions() {
-      const forceExpanded = this.discussions.some(discussion => !discussion.expanded);
-
-      this.discussions.forEach(discussion => {
-        this.toggleDiscussion({
-          discussionId: discussion.id,
-          forceExpanded,
-        });
-      });
-    },
   },
 };
 </script>
@@ -76,7 +65,7 @@ export default {
       type="button"
       :aria-label="__('Show comments')"
       class="diff-notes-collapse js-diff-comment-avatar js-diff-comment-button"
-      @click="toggleDiscussions"
+      @click="$emit('toggleLineDiscussions')"
     >
       <icon :size="12" name="collapse" />
     </button>
@@ -87,7 +76,7 @@ export default {
         :img-src="note.author.avatar_url"
         :tooltip-text="getTooltipText(note)"
         class="diff-comment-avatar js-diff-comment-avatar"
-        @click.native="toggleDiscussions"
+        @click.native="$emit('toggleLineDiscussions')"
       />
       <span
         v-if="moreText"
@@ -97,7 +86,7 @@ export default {
         data-container="body"
         data-placement="top"
         role="button"
-        @click="toggleDiscussions"
+        @click="$emit('toggleLineDiscussions')"
         >+{{ moreCount }}</span
       >
     </template>

app/assets/javascripts/diffs/components/diff_line_gutter_content.vue

@@ -105,7 +105,13 @@ export default {
     },
   },
   methods: {
-    ...mapActions('diffs', ['loadMoreLines', 'showCommentForm', 'setHighlightedRow']),
+    ...mapActions('diffs', [
+      'loadMoreLines',
+      'showCommentForm',
+      'setHighlightedRow',
+      'toggleLineDiscussions',
+      'toggleLineDiscussionWrappers',
+    ]),
     handleCommentButton() {
       this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash });
     },
@@ -184,7 +190,14 @@ export default {
         @click="setHighlightedRow(lineCode)"
       >
       </a>
-      <diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" :discussions="line.discussions" />
+      <diff-gutter-avatars
+        v-if="shouldShowAvatarsOnGutter"
+        :discussions="line.discussions"
+        :discussions-expanded="line.discussionsExpanded"
+        @toggleLineDiscussions="
+          toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
+        "
+      />
     </template>
   </div>
 </template>

app/assets/javascripts/diffs/components/inline_diff_comment_row.vue

 <script>
-import diffDiscussions from './diff_discussions.vue';
-import diffLineNoteForm from './diff_line_note_form.vue';
+import { mapActions } from 'vuex';
+import DiffDiscussions from './diff_discussions.vue';
+import DiffLineNoteForm from './diff_line_note_form.vue';
+import DiffDiscussionReply from './diff_discussion_reply.vue';
 
 export default {
   components: {
-    diffDiscussions,
-    diffLineNoteForm,
+    DiffDiscussions,
+    DiffLineNoteForm,
+    DiffDiscussionReply,
   },
   props: {
     line: {
@@ -32,9 +35,11 @@ export default {
       if (!this.line.discussions || !this.line.discussions.length) {
         return false;
       }
-
-      return this.line.discussions.every(discussion => discussion.expanded);
+      return this.line.discussionsExpanded;
+    },
   },
+  methods: {
+    ...mapActions('diffs', ['showCommentForm']),
   },
 };
 </script>
@@ -49,13 +54,22 @@ export default {
           :discussions="line.discussions"
           :help-page-path="helpPagePath"
         />
+        <diff-discussion-reply
+          :has-form="line.hasForm"
+          :render-reply-placeholder="Boolean(line.discussions.length)"
+          @showNewDiscussionForm="
+            showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash })
+          "
+        >
+          <template #form>
             <diff-line-note-form
-          v-if="line.hasForm"
               :diff-file-hash="diffFileHash"
               :line="line"
               :note-target-line="line"
               :help-page-path="helpPagePath"
             />
+          </template>
+        </diff-discussion-reply>
       </div>
     </td>
   </tr>

app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue

 <script>
-import diffDiscussions from './diff_discussions.vue';
-import diffLineNoteForm from './diff_line_note_form.vue';
+import { mapActions } from 'vuex';
+import DiffDiscussions from './diff_discussions.vue';
+import DiffLineNoteForm from './diff_line_note_form.vue';
+import DiffDiscussionReply from './diff_discussion_reply.vue';
 
 export default {
   components: {
-    diffDiscussions,
-    diffLineNoteForm,
+    DiffDiscussions,
+    DiffLineNoteForm,
+    DiffDiscussionReply,
   },
   props: {
     line: {
@@ -29,24 +32,30 @@ export default {
   computed: {
     hasExpandedDiscussionOnLeft() {
       return this.line.left && this.line.left.discussions.length
-        ? this.line.left.discussions.every(discussion => discussion.expanded)
+        ? this.line.left.discussionsExpanded
         : false;
     },
     hasExpandedDiscussionOnRight() {
       return this.line.right && this.line.right.discussions.length
-        ? this.line.right.discussions.every(discussion => discussion.expanded)
+        ? this.line.right.discussionsExpanded
         : false;
     },
     hasAnyExpandedDiscussion() {
       return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
     },
     shouldRenderDiscussionsOnLeft() {
-      return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft;
+      return (
+        this.line.left &&
+        this.line.left.discussions &&
+        this.line.left.discussions.length &&
+        this.hasExpandedDiscussionOnLeft
+      );
     },
     shouldRenderDiscussionsOnRight() {
       return (
         this.line.right &&
         this.line.right.discussions &&
+        this.line.right.discussions.length &&
         this.hasExpandedDiscussionOnRight &&
         this.line.right.type
       );
@@ -81,6 +90,22 @@ export default {
 
       return hasCommentFormOnLeft || hasCommentFormOnRight;
     },
+    shouldRenderReplyPlaceholderOnLeft() {
+      return Boolean(
+        this.line.left && this.line.left.discussions && this.line.left.discussions.length,
+      );
+    },
+    shouldRenderReplyPlaceholderOnRight() {
+      return Boolean(
+        this.line.right && this.line.right.discussions && this.line.right.discussions.length,
+      );
+    },
+  },
+  methods: {
+    ...mapActions('diffs', ['showCommentForm']),
+    showNewDiscussionForm() {
+      this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.diffFileHash });
+    },
   },
 };
 </script>
@@ -90,37 +115,49 @@ export default {
     <td class="notes-content parallel old" colspan="2">
       <div v-if="shouldRenderDiscussionsOnLeft" class="content">
         <diff-discussions
-          v-if="line.left.discussions.length"
           :discussions="line.left.discussions"
           :line="line.left"
           :help-page-path="helpPagePath"
         />
       </div>
+      <diff-discussion-reply
+        :has-form="showLeftSideCommentForm"
+        :render-reply-placeholder="shouldRenderReplyPlaceholderOnLeft"
+        @showNewDiscussionForm="showNewDiscussionForm"
+      >
+        <template #form>
           <diff-line-note-form
-        v-if="showLeftSideCommentForm"
             :diff-file-hash="diffFileHash"
             :line="line.left"
             :note-target-line="line.left"
             :help-page-path="helpPagePath"
             line-position="left"
           />
+        </template>
+      </diff-discussion-reply>
     </td>
     <td class="notes-content parallel new" colspan="2">
       <div v-if="shouldRenderDiscussionsOnRight" class="content">
         <diff-discussions
-          v-if="line.right.discussions.length"
           :discussions="line.right.discussions"
           :line="line.right"
           :help-page-path="helpPagePath"
         />
       </div>
+      <diff-discussion-reply
+        :has-form="showRightSideCommentForm"
+        :render-reply-placeholder="shouldRenderReplyPlaceholderOnRight"
+        @showNewDiscussionForm="showNewDiscussionForm"
+      >
+        <template #form>
           <diff-line-note-form
-        v-if="showRightSideCommentForm"
             :diff-file-hash="diffFileHash"
             :line="line.right"
             :note-target-line="line.right"
             line-position="right"
           />
+        </template>
+      </diff-discussion-reply>
     </td>
   </tr>
 </template>

app/assets/javascripts/diffs/store/actions.js

@@ -12,6 +12,7 @@ import {
   getNoteFormData,
   convertExpandLines,
   idleCallback,
+  allDiscussionWrappersExpanded,
 } from './utils';
 import * as types from './mutation_types';
 import {
@@ -79,6 +80,7 @@ export const assignDiscussionsToDiff = (
   discussions = rootState.notes.discussions,
 ) => {
   const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles);
+  const hash = getLocationHash();
 
   discussions
     .filter(discussion => discussion.diff_discussion)
@@ -86,6 +88,7 @@ export const assignDiscussionsToDiff = (
       commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, {
         discussion,
         diffPositionByLineCode,
+        hash,
       });
     });
 
@@ -99,6 +102,10 @@ export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => {
   commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash: file_hash, lineCode: line_code, id });
 };
 
+export const toggleLineDiscussions = ({ commit }, options) => {
+  commit(types.TOGGLE_LINE_DISCUSSIONS, options);
+};
+
 export const renderFileForDiscussionId = ({ commit, rootState, state }, discussionId) => {
   const discussion = rootState.notes.discussions.find(d => d.id === discussionId);
 
@@ -257,6 +264,31 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => {
   });
 };
 
+export const toggleFileDiscussionWrappers = ({ commit }, diff) => {
+  const discussionWrappersExpanded = allDiscussionWrappersExpanded(diff);
+  let linesWithDiscussions;
+  if (diff.highlighted_diff_lines) {
+    linesWithDiscussions = diff.highlighted_diff_lines.filter(line => line.discussions.length);
+  }
+  if (diff.parallel_diff_lines) {
+    linesWithDiscussions = diff.parallel_diff_lines.filter(
+      line =>
+        (line.left && line.left.discussions.length) ||
+        (line.right && line.right.discussions.length),
+    );
+  }
+
+  if (linesWithDiscussions.length) {
+    linesWithDiscussions.forEach(line => {
+      commit(types.TOGGLE_LINE_DISCUSSIONS, {
+        fileHash: diff.file_hash,
+        lineCode: line.line_code,
+        expanded: !discussionWrappersExpanded,
+      });
+    });
+  }
+};
+
 export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => {
   const postData = getNoteFormData({
     commit: state.commit,

app/assets/javascripts/diffs/store/mutation_types.js

@@ -35,3 +35,5 @@ export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINE
 export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE';
 
 export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER';
+
+export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS';

app/assets/javascripts/diffs/store/mutations.js

@@ -6,6 +6,7 @@ import {
   addContextLines,
   prepareDiffData,
   isDiscussionApplicableToLine,
+  updateLineInFile,
 } from './utils';
 import * as types from './mutation_types';
 
@@ -109,7 +110,7 @@ export default {
     }));
   },
 
-  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode }) {
+  [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) {
     const { latestDiff } = state;
 
     const discussionLineCode = discussion.line_code;
@@ -130,13 +131,27 @@ export default {
         : [],
     });
 
+    const setDiscussionsExpanded = line => {
+      const isLineNoteTargeted = line.discussions.some(
+        disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`),
+      );
+
+      return {
+        ...line,
+        discussionsExpanded:
+          line.discussions && line.discussions.length
+            ? line.discussions.some(disc => !disc.resolved) || isLineNoteTargeted
+            : false,
+      };
+    };
+
     state.diffFiles = state.diffFiles.map(diffFile => {
       if (diffFile.file_hash === fileHash) {
         const file = { ...diffFile };
 
         if (file.highlighted_diff_lines) {
           file.highlighted_diff_lines = file.highlighted_diff_lines.map(line =>
-            lineCheck(line) ? mapDiscussions(line) : line,
+            setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line),
           );
         }
 
@@ -148,8 +163,10 @@ export default {
             if (left || right) {
               return {
                 ...line,
-                left: line.left ? mapDiscussions(line.left) : null,
-                right: line.right ? mapDiscussions(line.right, () => !left) : null,
+                left: line.left ? setDiscussionsExpanded(mapDiscussions(line.left)) : null,
+                right: line.right
+                  ? setDiscussionsExpanded(mapDiscussions(line.right, () => !left))
+                  : null,
               };
             }
 
@@ -173,32 +190,11 @@ export default {
   [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) {
     const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash);
     if (selectedFile) {
-      if (selectedFile.parallel_diff_lines) {
-        const targetLine = selectedFile.parallel_diff_lines.find(
-          line =>
-            (line.left && line.left.line_code === lineCode) ||
-            (line.right && line.right.line_code === lineCode),
+      updateLineInFile(selectedFile, lineCode, line =>
+        Object.assign(line, {
+          discussions: line.discussions.filter(discussion => discussion.notes.length),
+        }),
       );
-        if (targetLine) {
-          const side = targetLine.left && targetLine.left.line_code === lineCode ? 'left' : 'right';
-
-          Object.assign(targetLine[side], {
-            discussions: targetLine[side].discussions.filter(discussion => discussion.notes.length),
-          });
-        }
-      }
-
-      if (selectedFile.highlighted_diff_lines) {
-        const targetInlineLine = selectedFile.highlighted_diff_lines.find(
-          line => line.line_code === lineCode,
-        );
-
-        if (targetInlineLine) {
-          Object.assign(targetInlineLine, {
-            discussions: targetInlineLine.discussions.filter(discussion => discussion.notes.length),
-          });
-        }
-      }
 
       if (selectedFile.discussions && selectedFile.discussions.length) {
         selectedFile.discussions = selectedFile.discussions.filter(
@@ -207,6 +203,15 @@ export default {
       }
     }
   },
+
+  [types.TOGGLE_LINE_DISCUSSIONS](state, { fileHash, lineCode, expanded }) {
+    const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash);
+
+    updateLineInFile(selectedFile, lineCode, line =>
+      Object.assign(line, { discussionsExpanded: expanded }),
+    );
+  },
+
   [types.TOGGLE_FOLDER_OPEN](state, path) {
     state.treeEntries[path].opened = !state.treeEntries[path].opened;
   },

app/assets/javascripts/diffs/store/utils.js

@@ -454,3 +454,48 @@ export const convertExpandLines = ({
 };
 
 export const idleCallback = cb => requestIdleCallback(cb);
+
+export const updateLineInFile = (selectedFile, lineCode, updateFn) => {
+  if (selectedFile.parallel_diff_lines) {
+    const targetLine = selectedFile.parallel_diff_lines.find(
+      line =>
+        (line.left && line.left.line_code === lineCode) ||
+        (line.right && line.right.line_code === lineCode),
+    );
+    if (targetLine) {
+      const side = targetLine.left && targetLine.left.line_code === lineCode ? 'left' : 'right';
+
+      updateFn(targetLine[side]);
+    }
+  }
+  if (selectedFile.highlighted_diff_lines) {
+    const targetInlineLine = selectedFile.highlighted_diff_lines.find(
+      line => line.line_code === lineCode,
+    );
+
+    if (targetInlineLine) {
+      updateFn(targetInlineLine);
+    }
+  }
+};
+
+export const allDiscussionWrappersExpanded = diff => {
+  const discussionsExpandedArray = [];
+  if (diff.parallel_diff_lines) {
+    diff.parallel_diff_lines.forEach(line => {
+      if (line.left && line.left.discussions.length) {
+        discussionsExpandedArray.push(line.left.discussionsExpanded);
+      }
+      if (line.right && line.right.discussions.length) {
+        discussionsExpandedArray.push(line.right.discussionsExpanded);
+      }
+    });
+  } else if (diff.highlighted_diff_lines) {
+    diff.parallel_diff_lines.forEach(line => {
+      if (line.discussions.length) {
+        discussionsExpandedArray.push(line.discussionsExpanded);
+      }
+    });
+  }
+  return discussionsExpandedArray.every(el => el);
+};

app/assets/javascripts/notes/components/discussion_actions.vue

@@ -39,16 +39,24 @@ export default {
 </script>
 
 <template>
-  <div class="discussion-with-resolve-btn clearfix">
-    <reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" />
-
-    <div class="btn-group discussion-actions" role="group">
+  <div class="discussion-with-resolve-btn">
+    <reply-placeholder
+      :button-text="s__('MergeRequests|Reply...')"
+      class="qa-discussion-reply"
+      @onClick="$emit('showReplyForm')"
+    />
     <resolve-discussion-button
       v-if="discussion.resolvable"
       :is-resolving="isResolving"
       :button-title="resolveButtonTitle"
       @onClick="$emit('resolve')"
     />
+    <div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group">
+      <resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" />
+      <jump-to-next-discussion-button
+        v-if="shouldShowJumpToNextDiscussion"
+        @onClick="$emit('jumpToNextDiscussion')"
+      />
       <resolve-with-issue-button
         v-if="discussion.resolvable && resolveWithIssuePath"
         :url="resolveWithIssuePath"

app/assets/javascripts/notes/components/discussion_notes.vue

 <script>
-import { mapGetters } from 'vuex';
+import { mapGetters, mapActions } from 'vuex';
 import { SYSTEM_NOTE } from '../constants';
 import { __ } from '~/locale';
-import NoteableNote from './noteable_note.vue';
-import PlaceholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
-import PlaceholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
+import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue';
+import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue';
 import SystemNote from '~/vue_shared/components/notes/system_note.vue';
+import NoteableNote from './noteable_note.vue';
 import ToggleRepliesWidget from './toggle_replies_widget.vue';
 import NoteEditedText from './note_edited_text.vue';
 
@@ -72,6 +72,7 @@ export default {
     },
   },
   methods: {
+    ...mapActions(['toggleDiscussion']),
     componentName(note) {
       if (note.isPlaceholderNote) {
         if (note.placeholderType === SYSTEM_NOTE) {
@@ -101,7 +102,7 @@ export default {
         <component
           :is="componentName(firstNote)"
           :note="componentData(firstNote)"
-          :line="line"
+          :line="line || diffLine"
           :commit="commit"
           :help-page-path="helpPagePath"
           :show-reply-button="userCanReply"
@@ -118,11 +119,15 @@ export default {
           />
           <slot slot="avatar-badge" name="avatar-badge"></slot>
         </component>
+        <div
+          :class="discussion.diff_discussion ? 'discussion-collapsible bordered-box clearfix' : ''"
+        >
           <toggle-replies-widget
             v-if="hasReplies"
             :collapsed="!isExpanded"
             :replies="replies"
-          @toggle="$emit('toggleDiscussion')"
+            :class="{ 'discussion-toggle-replies': discussion.diff_discussion }"
+            @toggle="toggleDiscussion({ discussionId: discussion.id })"
           />
           <template v-if="isExpanded">
             <component
@@ -135,6 +140,8 @@ export default {
               @handleDeleteNote="$emit('deleteNote')"
             />
           </template>
+          <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot>
+        </div>
       </template>
       <template v-else>
         <component
@@ -148,8 +155,8 @@ export default {
         >
           <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot>
         </component>
+        <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot>
       </template>
     </ul>
-    <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot>
   </div>
 </template>

app/assets/javascripts/notes/components/discussion_reply_placeholder.vue

 <script>
 export default {
   name: 'ReplyPlaceholder',
+  props: {
+    buttonText: {
+      type: String,
+      required: true,
+    },
+  },
 };
 </script>
 
@@ -12,6 +18,6 @@ export default {
     :title="s__('MergeRequests|Add a reply')"
     @click="$emit('onClick')"
   >
-    {{ s__('MergeRequests|Reply...') }}
+    {{ buttonText }}
   </button>
 </template>

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -132,7 +132,7 @@ export default {
       return this.discussion.diff_discussion && this.renderDiffFile;
     },
     shouldGroupReplies() {
-      return !this.shouldRenderDiffs && !this.discussion.diff_discussion;
+      return !this.shouldRenderDiffs;
     },
     wrapperComponent() {
       return this.shouldRenderDiffs ? diffWithNote : 'div';
@@ -250,6 +250,11 @@ export default {
       clearDraft(this.autosaveKey);
     },
     saveReply(noteText, form, callback) {
+      if (!noteText) {
+        this.cancelReplyForm();
+        callback();
+        return;
+      }
       const postData = {
         in_reply_to_discussion_id: this.discussion.reply_id,
         target_type: this.getNoteableData.targetType,
@@ -363,7 +368,6 @@ Please check your network connection and try again.`;
               :line="line"
               :should-group-replies="shouldGroupReplies"
               @startReplying="showReplyForm"
-              @toggleDiscussion="toggleDiscussionHandler"
               @deleteNote="deleteNoteHandler"
             >
               <slot slot="avatar-badge" name="avatar-badge"></slot>
@@ -376,7 +380,7 @@ Please check your network connection and try again.`;
                 <div
                   v-else-if="showReplies"
                   :class="{ 'is-replying': isReplying }"
-                  class="discussion-reply-holder"
+                  class="discussion-reply-holder clearfix"
                 >
                   <user-avatar-link
                     v-if="!isReplying && userCanReply"

app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue

@@ -39,7 +39,7 @@ export default {
 </script>
 
 <template>
-  <timeline-entry-item class="note being-posted fade-in-half">
+  <timeline-entry-item class="note note-wrapper being-posted fade-in-half">
     <div class="timeline-icon">
       <user-avatar-link
         :link-href="getUserData.path"

app/assets/stylesheets/pages/diff.scss

@@ -1093,6 +1093,17 @@ table.code {
   line-height: 0;
 }
 
+.discussion-collapsible {
+  margin: 0 $gl-padding $gl-padding 71px;
+}
+
+.parallel {
+  .discussion-collapsible {
+    margin: $gl-padding;
+    margin-top: 0;
+  }
+}
+
 @media (max-width: map-get($grid-breakpoints, md)-1) {
   .diffs .files {
     @include fixed-width-container;
@@ -1110,6 +1121,11 @@ table.code {
       padding-right: 0;
     }
   }
+
+  .discussion-collapsible {
+    margin: $gl-padding;
+    margin-top: 0;
+  }
 }
 
 .image-diff-overlay,

app/assets/stylesheets/pages/notes.scss

@@ -134,6 +134,16 @@ $note-form-margin-left: 72px;
     }
   }
 
+  .discussion-toggle-replies {
+    border-top: 0;
+    border-radius: 4px 4px 0 0;
+
+    &.collapsed {
+      border: 0;
+      border-radius: 4px;
+    }
+  }
+
   .note-created-ago,
   .note-updated-at {
     white-space: normal;
@@ -462,6 +472,14 @@ $note-form-margin-left: 72px;
     position: relative;
   }
 
+  .notes-content .discussion-notes.diff-discussions {
+    border-bottom: 1px solid $border-color;
+
+    &:nth-last-child(1) {
+      border-bottom: 0;
+    }
+  }
+
   .notes_holder {
     font-family: $regular-font;
 
@@ -517,6 +535,17 @@ $note-form-margin-left: 72px;
   .discussion-reply-holder {
     border-radius: 0 0 $border-radius-default $border-radius-default;
     position: relative;
+
+    .discussion-form {
+      width: 100%;
+      background-color: $gray-light;
+      padding: 0;
+    }
+
+    .disabled-comment {
+      padding: $gl-vert-padding 0;
+      width: 100%;
+    }
   }
 }
 

changelogs/unreleased/32452-multiple-discussions.yml

+---
+title: Resolve Multiple discussions per line in merge request diffs
+merge_request: 28748
+author:
+type: added

changelogs/unreleased/62124-new-threaded-discussion-design.yml

+---
+title: Implement borderless discussion design with new reply field
+merge_request: 28580
+author:
+type: added

spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb

@@ -54,7 +54,7 @@ describe 'Resolve an open discussion in a merge request by creating an issue', :
 
     context 'creating the issue' do
       before do
-        find(resolve_discussion_selector).click
+        find(resolve_discussion_selector, match: :first).click
       end
 
       it 'has a hidden field for the discussion' do

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -368,8 +368,8 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
         all_discussion_replies = page.all('.discussion-reply-holder')
 
         expect(all_discussion_replies.count).to eq(2)
-        expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1)
-        expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1)
+        expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(2)
+        expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(2)
       end
 
       it 'displays next discussion even if hidden' do

spec/frontend/diffs/components/diff_discussion_reply_spec.js

+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import DiffDiscussionReply from '~/diffs/components/diff_discussion_reply.vue';
+import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
+import NoteSignedOutWidget from '~/notes/components/note_signed_out_widget.vue';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+describe('DiffDiscussionReply', () => {
+  let wrapper;
+  let getters;
+  let store;
+
+  const createComponent = (props = {}, slots = {}) => {
+    wrapper = shallowMount(DiffDiscussionReply, {
+      store,
+      localVue,
+      sync: false,
+      propsData: {
+        ...props,
+      },
+      slots: {
+        ...slots,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('if user can reply', () => {
+    beforeEach(() => {
+      getters = {
+        userCanReply: () => true,
+        getUserData: () => ({
+          path: 'test-path',
+          avatar_url: 'avatar_url',
+          name: 'John Doe',
+        }),
+      };
+
+      store = new Vuex.Store({
+        getters,
+      });
+    });
+
+    it('should render a form if component has form', () => {
+      createComponent(
+        {
+          renderReplyPlaceholder: false,
+          hasForm: true,
+        },
+        {
+          form: `<div id="test-form"></div>`,
+        },
+      );
+
+      expect(wrapper.find('#test-form').exists()).toBe(true);
+    });
+
+    it('should render a reply placeholder if there is no form', () => {
+      createComponent({
+        renderReplyPlaceholder: true,
+        hasForm: false,
+      });
+
+      expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true);
+    });
+  });
+
+  it('renders a signed out widget when user is not logged in', () => {
+    getters = {
+      userCanReply: () => false,
+      getUserData: () => null,
+    };
+
+    store = new Vuex.Store({
+      getters,
+    });
+
+    createComponent({
+      renderReplyPlaceholder: false,
+      hasForm: false,
+    });
+
+    expect(wrapper.find(NoteSignedOutWidget).exists()).toBe(true);
+  });
+});

spec/frontend/diffs/components/diff_gutter_avatars_spec.js

+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue';
+import discussionsMockData from '../mock_data/diff_discussions';
+
+const localVue = createLocalVue();
+const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
+
+describe('DiffGutterAvatars', () => {
+  let wrapper;
+
+  const findCollapseButton = () => wrapper.find('.diff-notes-collapse');
+  const findMoreCount = () => wrapper.find('.diff-comments-more-count');
+  const findUserAvatars = () => wrapper.findAll('.diff-comment-avatar');
+
+  const createComponent = (props = {}) => {
+    wrapper = shallowMount(DiffGutterAvatars, {
+      localVue,
+      sync: false,
+      propsData: {
+        ...props,
+      },
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('when expanded', () => {
+    beforeEach(() => {
+      createComponent({
+        discussions: getDiscussionsMockData(),
+        discussionsExpanded: true,
+      });
+    });
+
+    it('renders a collapse button when discussions are expanded', () => {
+      expect(findCollapseButton().exists()).toBe(true);
+    });
+
+    it('should emit toggleDiscussions event on button click', () => {
+      findCollapseButton().trigger('click');
+
+      expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy();
+    });
+  });
+
+  describe('when collapsed', () => {
+    beforeEach(() => {
+      createComponent({
+        discussions: getDiscussionsMockData(),
+        discussionsExpanded: false,
+      });
+    });
+
+    it('renders user avatars and moreCount text', () => {
+      expect(findUserAvatars().exists()).toBe(true);
+      expect(findMoreCount().exists()).toBe(true);
+    });
+
+    it('renders correct amount of user avatars', () => {
+      expect(findUserAvatars().length).toBe(3);
+    });
+
+    it('renders correct moreCount number', () => {
+      expect(findMoreCount().text()).toBe('+2');
+    });
+
+    it('should emit toggleDiscussions event on avatars click', () => {
+      findUserAvatars()
+        .at(0)
+        .trigger('click');
+
+      expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy();
+    });
+
+    it('should emit toggleDiscussions event on more count text click', () => {
+      findMoreCount().trigger('click');
+
+      expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy();
+    });
+  });
+
+  it('renders an empty more count string if there are no discussions', () => {
+    createComponent({
+      discussions: [],
+      discussionsExpanded: false,
+    });
+
+    expect(findMoreCount().exists()).toBe(false);
+  });
+
+  describe('tooltip text', () => {
+    beforeEach(() => {
+      createComponent({
+        discussions: getDiscussionsMockData(),
+        discussionsExpanded: false,
+      });
+    });
+
+    it('returns original comment if it is shorter than max length', () => {
+      const note = wrapper.vm.discussions[0].notes[0];
+
+      expect(wrapper.vm.getTooltipText(note)).toEqual('Administrator: comment 1');
+    });
+
+    it('returns truncated version of comment if it is longer than max length', () => {
+      const note = wrapper.vm.discussions[0].notes[1];
+
+      expect(wrapper.vm.getTooltipText(note)).toEqual('Fatih Acet: comment 2 is r...');
+    });
+  });
+});

spec/frontend/notes/components/discussion_notes_spec.js

@@ -87,7 +87,7 @@ describe('DiscussionNotes', () => {
         discussion.notes[0],
       ];
       discussion.notes = notesData;
-      createComponent({ discussion });
+      createComponent({ discussion, shouldRenderDiffs: true });
       const notes = wrapper.findAll('.notes > li');
 
       expect(notes.at(0).is(PlaceholderSystemNote)).toBe(true);

spec/frontend/notes/components/discussion_reply_placeholder_spec.js

@@ -2,13 +2,19 @@ import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vu
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 
 const localVue = createLocalVue();
+const buttonText = 'Test Button Text';
 
 describe('ReplyPlaceholder', () => {
   let wrapper;
 
+  const findButton = () => wrapper.find({ ref: 'button' });
+
   beforeEach(() => {
     wrapper = shallowMount(ReplyPlaceholder, {
       localVue,
+      propsData: {
+        buttonText,
+      },
     });
   });
 
@@ -17,9 +23,7 @@ describe('ReplyPlaceholder', () => {
   });
 
   it('emits onClick even on button click', () => {
-    const button = wrapper.find({ ref: 'button' });
-
-    button.trigger('click');
+    findButton().trigger('click');
 
     expect(wrapper.emitted()).toEqual({
       onClick: [[]],
@@ -27,8 +31,6 @@ describe('ReplyPlaceholder', () => {
   });
 
   it('should render reply button', () => {
-    const button = wrapper.find({ ref: 'button' });
-
-    expect(button.text()).toEqual('Reply...');
+    expect(findButton().text()).toEqual(buttonText);
   });
 });

spec/javascripts/diffs/components/diff_file_header_spec.js

@@ -521,7 +521,7 @@ describe('diff_file_header', () => {
       });
 
       describe('with discussions', () => {
-        it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => {
+        it('dispatches toggleFileDiscussionWrappers when user clicks on toggle discussions button', () => {
           const propsCopy = Object.assign({}, props);
           propsCopy.diffFile.submodule = false;
           propsCopy.diffFile.blob = {
@@ -552,11 +552,11 @@ describe('diff_file_header', () => {
             }),
           });
 
-          spyOn(vm, 'toggleFileDiscussions');
+          spyOn(vm, 'toggleFileDiscussionWrappers');
 
           vm.$el.querySelector('.js-btn-vue-toggle-comments').click();
 
-          expect(vm.toggleFileDiscussions).toHaveBeenCalled();
+          expect(vm.toggleFileDiscussionWrappers).toHaveBeenCalled();
         });
       });
     });

spec/javascripts/diffs/components/diff_gutter_avatars_spec.js

-import Vue from 'vue';
-import DiffGutterAvatarsComponent from '~/diffs/components/diff_gutter_avatars.vue';
-import { COUNT_OF_AVATARS_IN_GUTTER } from '~/diffs/constants';
-import store from '~/mr_notes/stores';
-import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
-import discussionsMockData from '../mock_data/diff_discussions';
-
-describe('DiffGutterAvatars', () => {
-  let component;
-  const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
-
-  beforeEach(() => {
-    component = createComponentWithStore(Vue.extend(DiffGutterAvatarsComponent), store, {
-      discussions: getDiscussionsMockData(),
-    }).$mount();
-  });
-
-  describe('computed', () => {
-    describe('discussionsExpanded', () => {
-      it('should return true when all discussions are expanded', () => {
-        expect(component.discussionsExpanded).toEqual(true);
-      });
-
-      it('should return false when all discussions are not expanded', () => {
-        component.discussions[0].expanded = false;
-
-        expect(component.discussionsExpanded).toEqual(false);
-      });
-    });
-
-    describe('allDiscussions', () => {
-      it('should return an array of notes', () => {
-        expect(component.allDiscussions).toEqual([...component.discussions[0].notes]);
-      });
-    });
-
-    describe('notesInGutter', () => {
-      it('should return a subset of discussions to show in gutter', () => {
-        expect(component.notesInGutter.length).toEqual(COUNT_OF_AVATARS_IN_GUTTER);
-        expect(component.notesInGutter[0]).toEqual({
-          note: component.discussions[0].notes[0].note,
-          author: component.discussions[0].notes[0].author,
-        });
-      });
-    });
-
-    describe('moreCount', () => {
-      it('should return count of remaining discussions from gutter', () => {
-        expect(component.moreCount).toEqual(2);
-      });
-    });
-
-    describe('moreText', () => {
-      it('should return proper text if moreCount > 0', () => {
-        expect(component.moreText).toEqual('2 more comments');
-      });
-
-      it('should return empty string if there is no discussion', () => {
-        component.discussions = [];
-
-        expect(component.moreText).toEqual('');
-      });
-    });
-  });
-
-  describe('methods', () => {
-    describe('getTooltipText', () => {
-      it('should return original comment if it is shorter than max length', () => {
-        const note = component.discussions[0].notes[0];
-
-        expect(component.getTooltipText(note)).toEqual('Administrator: comment 1');
-      });
-
-      it('should return truncated version of comment', () => {
-        const note = component.discussions[0].notes[1];
-
-        expect(component.getTooltipText(note)).toEqual('Fatih Acet: comment 2 is r...');
-      });
-    });
-
-    describe('toggleDiscussions', () => {
-      it('should toggle all discussions', () => {
-        expect(component.discussions[0].expanded).toEqual(true);
-
-        component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
-        component.discussions = component.$store.state.notes.discussions;
-        component.toggleDiscussions();
-
-        expect(component.discussions[0].expanded).toEqual(false);
-        component.$store.dispatch('setInitialNotes', []);
-      });
-
-      it('forces expansion of all discussions', () => {
-        spyOn(component.$store, 'dispatch');
-
-        component.discussions[0].expanded = true;
-        component.discussions.push({
-          ...component.discussions[0],
-          id: '123test',
-          expanded: false,
-        });
-
-        component.toggleDiscussions();
-
-        expect(component.$store.dispatch.calls.argsFor(0)).toEqual([
-          'toggleDiscussion',
-          {
-            discussionId: component.discussions[0].id,
-            forceExpanded: true,
-          },
-        ]);
-
-        expect(component.$store.dispatch.calls.argsFor(1)).toEqual([
-          'toggleDiscussion',
-          {
-            discussionId: component.discussions[1].id,
-            forceExpanded: true,
-          },
-        ]);
-      });
-    });
-  });
-
-  describe('template', () => {
-    const buttonSelector = '.js-diff-comment-button';
-    const svgSelector = `${buttonSelector} svg`;
-    const avatarSelector = '.js-diff-comment-avatar';
-    const plusCountSelector = '.js-diff-comment-plus';
-
-    it('should have button to collapse discussions when the discussions expanded', () => {
-      expect(component.$el.querySelector(buttonSelector)).toBeDefined();
-      expect(component.$el.querySelector(svgSelector)).toBeDefined();
-    });
-
-    it('should have user avatars when discussions collapsed', () => {
-      component.discussions[0].expanded = false;
-
-      Vue.nextTick(() => {
-        expect(component.$el.querySelector(buttonSelector)).toBeNull();
-        expect(component.$el.querySelectorAll(avatarSelector).length).toEqual(4);
-        expect(component.$el.querySelector(plusCountSelector)).toBeDefined();
-        expect(component.$el.querySelector(plusCountSelector).textContent).toEqual('+2');
-      });
-    });
-  });
-});

spec/javascripts/diffs/components/inline_diff_view_spec.js

@@ -36,10 +36,11 @@ describe('InlineDiffView', () => {
     it('should render discussions', done => {
       const el = component.$el;
       component.diffLines[1].discussions = getDiscussionsMockData();
+      component.diffLines[1].discussionsExpanded = true;
 
       Vue.nextTick(() => {
         expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
-        expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5);
+        expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(6);
         expect(el.innerText.indexOf('comment 5')).toBeGreaterThan(-1);
         component.$store.dispatch('setInitialNotes', []);
 

spec/javascripts/diffs/store/actions_spec.js

@@ -206,6 +206,7 @@ describe('DiffsStoreActions', () => {
                   position_type: 'text',
                 },
               },
+              hash: 'diff-content-1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a',
             },
           },
         ],

spec/javascripts/notes/components/noteable_discussion_spec.js

@@ -36,6 +36,12 @@ describe('noteable_discussion component', () => {
   });
 
   it('should render user avatar', () => {
+    const discussion = { ...discussionMock };
+    discussion.diff_file = mockDiffFile;
+    discussion.diff_discussion = true;
+
+    wrapper.setProps({ discussion, renderDiffFile: true });
+
     expect(wrapper.find('.user-avatar-link').exists()).toBe(true);
   });
 

spec/support/features/discussion_comments_shared_example.rb

@@ -176,7 +176,7 @@ shared_examples 'discussion comments' do |resource_name|
 
         if resource_name == 'merge request'
           let(:note_id) { find("#{comments_selector} .note:first-child", match: :first)['data-note-id'] }
-          let(:reply_id) { find("#{comments_selector} .note:last-child", match: :first)['data-note-id'] }
+          let(:reply_id) { find("#{comments_selector} .note:last-of-type", match: :first)['data-note-id'] }
 
           it 'can be replied to after resolving' do
             click_button "Resolve discussion"