Merge branch '_diff-lines-refactor' into 'master'

Componentize diff lines and diff comments

Closes #48297

See merge request gitlab-org/gitlab-ce!19702

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

@@ -4,14 +4,7 @@ import { s__ } from '~/locale';
 import { mapState, mapGetters, mapActions } from 'vuex';
 import Icon from '~/vue_shared/components/icon.vue';
 import DiffGutterAvatars from './diff_gutter_avatars.vue';
-import {
-  MATCH_LINE_TYPE,
-  CONTEXT_LINE_TYPE,
-  OLD_NO_NEW_LINE_TYPE,
-  NEW_NO_NEW_LINE_TYPE,
-  LINE_POSITION_RIGHT,
-  UNFOLD_COUNT,
-} from '../constants';
+import { LINE_POSITION_RIGHT, UNFOLD_COUNT } from '../constants';
 import * as utils from '../store/utils';
 
 export default {
@@ -63,6 +56,21 @@ export default {
       required: false,
       default: false,
     },
+    isMatchLine: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isMetaLine: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isContextLine: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     ...mapState({
@@ -70,15 +78,6 @@ export default {
       diffFiles: state => state.diffs.diffFiles,
     }),
     ...mapGetters(['isLoggedIn', 'discussionsByLineCode']),
-    isMatchLine() {
-      return this.lineType === MATCH_LINE_TYPE;
-    },
-    isContextLine() {
-      return this.lineType === CONTEXT_LINE_TYPE;
-    },
-    isMetaLine() {
-      return this.lineType === OLD_NO_NEW_LINE_TYPE || this.lineType === NEW_NO_NEW_LINE_TYPE;
-    },
     lineHref() {
       return this.lineCode ? `#${this.lineCode}` : '#';
     },
@@ -109,9 +108,9 @@ export default {
     },
   },
   methods: {
-    ...mapActions(['loadMoreLines']),
+    ...mapActions(['loadMoreLines', 'showCommentForm']),
     handleCommentButton() {
-      this.$emit('showCommentForm', { lineCode: this.lineCode });
+      this.showCommentForm({ lineCode: this.lineCode });
     },
     handleLoadMoreLines() {
       if (this.isRequesting) {

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

+<script>
+import { mapGetters } from 'vuex';
+import DiffLineGutterContent from './diff_line_gutter_content.vue';
+import {
+  MATCH_LINE_TYPE,
+  CONTEXT_LINE_TYPE,
+  EMPTY_CELL_TYPE,
+  OLD_LINE_TYPE,
+  OLD_NO_NEW_LINE_TYPE,
+  NEW_NO_NEW_LINE_TYPE,
+  LINE_HOVER_CLASS_NAME,
+  LINE_UNFOLD_CLASS_NAME,
+} from '../constants';
+
+export default {
+  components: {
+    DiffLineGutterContent,
+  },
+  props: {
+    line: {
+      type: Object,
+      required: true,
+    },
+    diffFile: {
+      type: Object,
+      required: true,
+    },
+    showCommentButton: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    linePosition: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    lineType: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    isContentLine: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isBottom: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isHover: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    ...mapGetters(['isLoggedIn', 'isInlineView']),
+    normalizedLine() {
+      if (this.isInlineView) {
+        return this.line;
+      }
+
+      return this.lineType === OLD_LINE_TYPE ? this.line.left : this.line.right;
+    },
+    isMatchLine() {
+      return this.normalizedLine.type === MATCH_LINE_TYPE;
+    },
+    isContextLine() {
+      return this.normalizedLine.type === CONTEXT_LINE_TYPE;
+    },
+    isMetaLine() {
+      return (
+        this.normalizedLine.type === OLD_NO_NEW_LINE_TYPE ||
+        this.normalizedLine.type === NEW_NO_NEW_LINE_TYPE ||
+        this.normalizedLine.type === EMPTY_CELL_TYPE
+      );
+    },
+    classNameMap() {
+      const { type } = this.normalizedLine;
+
+      return {
+        [type]: type,
+        [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
+        [LINE_HOVER_CLASS_NAME]:
+          this.isLoggedIn &&
+          this.isHover &&
+          !this.isMatchLine &&
+          !this.isContextLine &&
+          !this.isMetaLine,
+      };
+    },
+    lineNumber() {
+      const { lineType, normalizedLine } = this;
+
+      return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine;
+    },
+  },
+};
+</script>
+
+<template>
+  <td
+    v-if="isContentLine"
+    :class="lineType"
+    class="line_content"
+    v-html="normalizedLine.richText"
+  >
+  </td>
+  <td
+    v-else
+    :class="classNameMap"
+  >
+    <diff-line-gutter-content
+      :file-hash="diffFile.fileHash"
+      :line-type="normalizedLine.type"
+      :line-code="normalizedLine.lineCode"
+      :line-position="linePosition"
+      :line-number="lineNumber"
+      :meta-data="normalizedLine.metaData"
+      :show-comment-button="showCommentButton"
+      :context-lines-path="diffFile.contextLinesPath"
+      :is-bottom="isBottom"
+      :is-match-line="isMatchLine"
+      :is-context-line="isContentLine"
+      :is-meta-line="isMetaLine"
+    />
+  </td>
+</template>

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

+<script>
+import $ from 'jquery';
+import { mapGetters } from 'vuex';
+import DiffTableCell from './diff_table_cell.vue';
+import {
+  NEW_LINE_TYPE,
+  OLD_LINE_TYPE,
+  CONTEXT_LINE_TYPE,
+  CONTEXT_LINE_CLASS_NAME,
+  OLD_NO_NEW_LINE_TYPE,
+  PARALLEL_DIFF_VIEW_TYPE,
+  NEW_NO_NEW_LINE_TYPE,
+  LINE_POSITION_LEFT,
+  LINE_POSITION_RIGHT,
+} from '../constants';
+
+export default {
+  components: {
+    DiffTableCell,
+  },
+  props: {
+    diffFile: {
+      type: Object,
+      required: true,
+    },
+    line: {
+      type: Object,
+      required: true,
+    },
+    isBottom: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  data() {
+    return {
+      isHover: false,
+      isLeftHover: false,
+      isRightHover: false,
+    };
+  },
+  computed: {
+    ...mapGetters(['isInlineView', 'isParallelView']),
+    isContextLine() {
+      return this.line.left
+        ? this.line.left.type === CONTEXT_LINE_TYPE
+        : this.line.type === CONTEXT_LINE_TYPE;
+    },
+    classNameMap() {
+      return {
+        [this.line.type]: this.line.type,
+        [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
+        [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
+      };
+    },
+    inlineRowId() {
+      const { lineCode, oldLine, newLine } = this.line;
+
+      return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`;
+    },
+    parallelViewLeftLineType() {
+      if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
+        return OLD_NO_NEW_LINE_TYPE;
+      }
+
+      return this.line.left.type;
+    },
+  },
+  created() {
+    this.newLineType = NEW_LINE_TYPE;
+    this.oldLineType = OLD_LINE_TYPE;
+    this.linePositionLeft = LINE_POSITION_LEFT;
+    this.linePositionRight = LINE_POSITION_RIGHT;
+  },
+  methods: {
+    handleMouseMove(e) {
+      const isHover = e.type === 'mouseover';
+
+      if (this.isInlineView) {
+        this.isHover = isHover;
+      } else {
+        const hoveringCell = e.target.closest('td');
+        const allCellsInHoveringRow = Array.from(e.currentTarget.children);
+        const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell);
+
+        if (hoverIndex >= 2) {
+          this.isRightHover = isHover;
+        } else {
+          this.isLeftHover = isHover;
+        }
+      }
+    },
+    // Prevent text selecting on both sides of parallel diff view
+    // Backport of the same code from legacy diff notes.
+    handleParallelLineMouseDown(e) {
+      const line = $(e.currentTarget);
+      const table = line.closest('table');
+
+      table.removeClass('left-side-selected right-side-selected');
+      const [lineClass] = ['left-side', 'right-side'].filter(name => line.hasClass(name));
+
+      if (lineClass) {
+        table.addClass(`${lineClass}-selected`);
+      }
+    },
+  },
+};
+</script>
+
+<template>
+  <tr
+    v-if="isInlineView"
+    :id="inlineRowId"
+    :class="classNameMap"
+    class="line_holder"
+    @mouseover="handleMouseMove"
+    @mouseout="handleMouseMove"
+  >
+    <diff-table-cell
+      :diff-file="diffFile"
+      :line="line"
+      :line-type="oldLineType"
+      :is-bottom="isBottom"
+      :is-hover="isHover"
+      :show-comment-button="true"
+      class="diff-line-num old_line"
+    />
+    <diff-table-cell
+      :diff-file="diffFile"
+      :line="line"
+      :line-type="newLineType"
+      :is-bottom="isBottom"
+      :is-hover="isHover"
+      class="diff-line-num new_line"
+    />
+    <diff-table-cell
+      :class="line.type"
+      :diff-file="diffFile"
+      :line="line"
+      :is-content-line="true"
+    />
+  </tr>
+
+  <tr
+    v-else
+    :class="classNameMap"
+    class="line_holder"
+    @mouseover="handleMouseMove"
+    @mouseout="handleMouseMove"
+  >
+    <diff-table-cell
+      :diff-file="diffFile"
+      :line="line"
+      :line-type="oldLineType"
+      :line-position="linePositionLeft"
+      :is-bottom="isBottom"
+      :is-hover="isLeftHover"
+      :show-comment-button="true"
+      class="diff-line-num old_line"
+    />
+    <diff-table-cell
+      :id="line.left.lineCode"
+      :diff-file="diffFile"
+      :line="line"
+      :is-content-line="true"
+      :line-type="parallelViewLeftLineType"
+      class="line_content parallel left-side"
+      @mousedown.native="handleParallelLineMouseDown"
+    />
+    <diff-table-cell
+      :diff-file="diffFile"
+      :line="line"
+      :line-type="newLineType"
+      :line-position="linePositionRight"
+      :is-bottom="isBottom"
+      :is-hover="isRightHover"
+      :show-comment-button="true"
+      class="diff-line-num new_line"
+    />
+    <diff-table-cell
+      :id="line.right.lineCode"
+      :diff-file="diffFile"
+      :line="line"
+      :is-content-line="true"
+      :line-type="line.right.type"
+      class="line_content parallel right-side"
+      @mousedown.native="handleParallelLineMouseDown"
+    />
+  </tr>
+</template>

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

+<script>
+import { mapState, mapGetters } from 'vuex';
+import diffDiscussions from './diff_discussions.vue';
+import diffLineNoteForm from './diff_line_note_form.vue';
+
+export default {
+  components: {
+    diffDiscussions,
+    diffLineNoteForm,
+  },
+  props: {
+    line: {
+      type: Object,
+      required: true,
+    },
+    diffFile: {
+      type: Object,
+      required: true,
+    },
+    diffLines: {
+      type: Array,
+      required: true,
+    },
+    lineIndex: {
+      type: Number,
+      required: true,
+    },
+  },
+  computed: {
+    ...mapState({
+      diffLineCommentForms: state => state.diffs.diffLineCommentForms,
+    }),
+    ...mapGetters(['discussionsByLineCode']),
+    isDiscussionExpanded() {
+      if (!this.discussions.length) {
+        return false;
+      }
+
+      return this.discussions.every(discussion => discussion.expanded);
+    },
+    hasCommentForm() {
+      return this.diffLineCommentForms[this.line.lineCode];
+    },
+    discussions() {
+      return this.discussionsByLineCode[this.line.lineCode] || [];
+    },
+    shouldRender() {
+      return this.isDiscussionExpanded || this.hasCommentForm;
+    },
+    className() {
+      return this.discussions.length ? '' : 'js-temp-notes-holder';
+    },
+  },
+};
+</script>
+
+<template>
+  <tr
+    v-if="shouldRender"
+    :class="className"
+    class="notes_holder"
+  >
+    <td
+      class="notes_line"
+      colspan="2"
+    ></td>
+    <td class="notes_content">
+      <div class="content">
+        <diff-discussions
+          :discussions="discussions"
+        />
+        <diff-line-note-form
+          v-if="diffLineCommentForms[line.lineCode]"
+          :diff-file="diffFile"
+          :diff-lines="diffLines"
+          :line="line"
+          :note-target-line="diffLines[lineIndex]"
+        />
+      </div>
+    </td>
+  </tr>
+</template>

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

 <script>
 import diffContentMixin from '../mixins/diff_content';
-import {
-  MATCH_LINE_TYPE,
-  CONTEXT_LINE_TYPE,
-  OLD_NO_NEW_LINE_TYPE,
-  NEW_NO_NEW_LINE_TYPE,
-  LINE_HOVER_CLASS_NAME,
-  LINE_UNFOLD_CLASS_NAME,
-} from '../constants';
+import inlineDiffCommentRow from './inline_diff_comment_row.vue';
 
 export default {
-  mixins: [diffContentMixin],
-  methods: {
-    handleMouse(lineCode, isOver) {
-      this.hoveredLineCode = isOver ? lineCode : null;
-    },
-    getLineClass(line) {
-      const isSameLine = this.hoveredLineCode && this.hoveredLineCode === line.lineCode;
-      const isMatchLine = line.type === MATCH_LINE_TYPE;
-      const isContextLine = line.type === CONTEXT_LINE_TYPE;
-      const isMetaLine = line.type === OLD_NO_NEW_LINE_TYPE || line.type === NEW_NO_NEW_LINE_TYPE;
-
-      return {
-        [line.type]: line.type,
-        [LINE_UNFOLD_CLASS_NAME]: isMatchLine,
-        [LINE_HOVER_CLASS_NAME]:
-          this.isLoggedIn && isSameLine && !isMatchLine && !isContextLine && !isMetaLine,
-      };
-    },
+  components: {
+    inlineDiffCommentRow,
   },
+  mixins: [diffContentMixin],
 };
 </script>
 
@@ -41,76 +19,19 @@ export default {
       <template
         v-for="(line, index) in normalizedDiffLines"
       >
-        <tr
-          :id="line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`"
-          :key="line.lineCode"
-          :class="getRowClass(line)"
-          class="line_holder"
-          @mouseover="handleMouse(line.lineCode, true)"
-          @mouseout="handleMouse(line.lineCode, false)"
-        >
-          <td
-            :class="getLineClass(line)"
-            class="diff-line-num old_line"
-          >
-            <diff-line-gutter-content
-              :file-hash="fileHash"
-              :line-type="line.type"
-              :line-code="line.lineCode"
-              :line-number="line.oldLine"
-              :meta-data="line.metaData"
-              :show-comment-button="true"
-              :context-lines-path="diffFile.contextLinesPath"
-              :is-bottom="index + 1 === diffLinesLength"
-              @showCommentForm="handleShowCommentForm"
-            />
-          </td>
-          <td
-            :class="getLineClass(line)"
-            class="diff-line-num new_line"
-          >
-            <diff-line-gutter-content
-              :file-hash="fileHash"
-              :line-type="line.type"
-              :line-code="line.lineCode"
-              :line-number="line.newLine"
-              :meta-data="line.metaData"
+        <diff-table-row
+          :diff-file="diffFile"
+          :line="line"
           :is-bottom="index + 1 === diffLinesLength"
-              :context-lines-path="diffFile.contextLinesPath"
-            />
-          </td>
-          <td
-            :class="line.type"
-            class="line_content"
-            v-html="line.richText"
-          >
-          </td>
-        </tr>
-        <tr
-          v-if="isDiscussionExpanded(line.lineCode) || diffLineCommentForms[line.lineCode]"
-          :key="index"
-          :class="discussionsByLineCode[line.lineCode] ? '' : 'js-temp-notes-holder'"
-          class="notes_holder"
-        >
-          <td
-            class="notes_line"
-            colspan="2"
-          ></td>
-          <td class="notes_content">
-            <div class="content">
-              <diff-discussions
-                :discussions="discussionsByLineCode[line.lineCode] || []"
+          :key="line.lineCode"
         />
-              <diff-line-note-form
-                v-if="diffLineCommentForms[line.lineCode]"
+        <inline-diff-comment-row
           :diff-file="diffFile"
-                :diff-lines="diffLines"
+          :diff-lines="normalizedDiffLines"
           :line="line"
-                :note-target-line="diffLines[index]"
+          :line-index="index"
+          :key="index"
         />
-            </div>
-          </td>
-        </tr>
       </template>
     </tbody>
   </table>

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

+<script>
+import { mapState, mapGetters } from 'vuex';
+import diffDiscussions from './diff_discussions.vue';
+import diffLineNoteForm from './diff_line_note_form.vue';
+
+export default {
+  components: {
+    diffDiscussions,
+    diffLineNoteForm,
+  },
+  props: {
+    line: {
+      type: Object,
+      required: true,
+    },
+    diffFile: {
+      type: Object,
+      required: true,
+    },
+    diffLines: {
+      type: Array,
+      required: true,
+    },
+    lineIndex: {
+      type: Number,
+      required: true,
+    },
+  },
+  computed: {
+    ...mapState({
+      diffLineCommentForms: state => state.diffs.diffLineCommentForms,
+    }),
+    ...mapGetters(['discussionsByLineCode']),
+    leftLineCode() {
+      return this.line.left.lineCode;
+    },
+    rightLineCode() {
+      return this.line.right.lineCode;
+    },
+    hasDiscussion() {
+      const discussions = this.discussionsByLineCode;
+
+      return discussions[this.leftLineCode] || discussions[this.rightLineCode];
+    },
+    hasExpandedDiscussionOnLeft() {
+      const discussions = this.discussionsByLineCode[this.leftLineCode];
+
+      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+    },
+    hasExpandedDiscussionOnRight() {
+      const discussions = this.discussionsByLineCode[this.rightLineCode];
+
+      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+    },
+    hasAnyExpandedDiscussion() {
+      return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
+    },
+    shouldRenderDiscussionsRow() {
+      const hasDiscussion = this.hasDiscussion && this.hasAnyExpandedDiscussion;
+      const hasCommentFormOnLeft = this.diffLineCommentForms[this.leftLineCode];
+      const hasCommentFormOnRight = this.diffLineCommentForms[this.rightLineCode];
+
+      return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight;
+    },
+    shouldRenderDiscussionsOnLeft() {
+      return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
+    },
+    shouldRenderDiscussionsOnRight() {
+      return (
+        this.discussionsByLineCode[this.rightLineCode] &&
+        this.hasExpandedDiscussionOnRight &&
+        this.line.right.type
+      );
+    },
+    className() {
+      return this.hasDiscussion ? '' : 'js-temp-notes-holder';
+    },
+  },
+};
+</script>
+
+<template>
+  <tr
+    v-if="shouldRenderDiscussionsRow"
+    :class="className"
+    class="notes_holder"
+  >
+    <td class="notes_line old"></td>
+    <td class="notes_content parallel old">
+      <div
+        v-if="shouldRenderDiscussionsOnLeft"
+        class="content"
+      >
+        <diff-discussions
+          :discussions="discussionsByLineCode[leftLineCode]"
+        />
+      </div>
+      <diff-line-note-form
+        v-if="diffLineCommentForms[leftLineCode] &&
+        diffLineCommentForms[leftLineCode]"
+        :diff-file="diffFile"
+        :diff-lines="diffLines"
+        :line="line.left"
+        :note-target-line="diffLines[lineIndex].left"
+        position="left"
+      />
+    </td>
+    <td class="notes_line new"></td>
+    <td class="notes_content parallel new">
+      <div
+        v-if="shouldRenderDiscussionsOnRight"
+        class="content"
+      >
+        <diff-discussions
+          :discussions="discussionsByLineCode[rightLineCode]"
+        />
+      </div>
+      <diff-line-note-form
+        v-if="diffLineCommentForms[rightLineCode] &&
+        diffLineCommentForms[rightLineCode] && line.right.type"
+        :diff-file="diffFile"
+        :diff-lines="diffLines"
+        :line="line.right"
+        :note-target-line="diffLines[lineIndex].right"
+        position="right"
+      />
+    </td>
+  </tr>
+</template>

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

 <script>
 import diffContentMixin from '../mixins/diff_content';
-import {
-  EMPTY_CELL_TYPE,
-  MATCH_LINE_TYPE,
-  CONTEXT_LINE_TYPE,
-  OLD_NO_NEW_LINE_TYPE,
-  NEW_NO_NEW_LINE_TYPE,
-  LINE_HOVER_CLASS_NAME,
-  LINE_UNFOLD_CLASS_NAME,
-  LINE_POSITION_RIGHT,
-} from '../constants';
+import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
+import { EMPTY_CELL_TYPE } from '../constants';
 
 export default {
+  components: {
+    parallelDiffCommentRow,
+  },
   mixins: [diffContentMixin],
   computed: {
     parallelDiffLines() {
@@ -26,77 +21,6 @@ export default {
       });
     },
   },
-  methods: {
-    hasDiscussion(line) {
-      const discussions = this.discussionsByLineCode;
-      const hasDiscussion = discussions[line.left.lineCode] || discussions[line.right.lineCode];
-
-      return hasDiscussion;
-    },
-    getClassName(line, position) {
-      const { type, lineCode } = line[position];
-      const isMatchLine = type === MATCH_LINE_TYPE;
-      const isContextLine = !isMatchLine && type !== EMPTY_CELL_TYPE && type !== CONTEXT_LINE_TYPE;
-      const isMetaLine = type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE;
-      const isSameLine = this.hoveredLineCode && this.hoveredLineCode === lineCode;
-      const isSameSection = position === this.hoveredSection;
-
-      return {
-        [type]: type,
-        [LINE_UNFOLD_CLASS_NAME]: isMatchLine,
-        [LINE_HOVER_CLASS_NAME]:
-          this.isLoggedIn && isContextLine && isSameLine && isSameSection && !isMetaLine,
-      };
-    },
-    handleMouse(e, line, isHover) {
-      if (isHover) {
-        const cell = e.target.closest('td');
-
-        if (this.$refs.leftLines.indexOf(cell) > -1) {
-          this.hoveredLineCode = line.left.lineCode;
-          this.hoveredSection = 'left';
-        } else if (this.$refs.rightLines.indexOf(cell) > -1) {
-          this.hoveredLineCode = line.right.lineCode;
-          this.hoveredSection = 'right';
-        }
-      } else {
-        this.hoveredLineCode = null;
-        this.hoveredSection = null;
-      }
-    },
-    shouldRenderDiscussionsRow(line) {
-      const hasDiscussion = this.hasDiscussion(line) && this.hasAnyExpandedDiscussion(line);
-      const hasCommentFormOnLeft = this.diffLineCommentForms[line.left.lineCode];
-      const hasCommentFormOnRight = this.diffLineCommentForms[line.right.lineCode];
-
-      return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight;
-    },
-    shouldRenderDiscussions(line, position) {
-      const { lineCode } = line[position];
-      let render = this.discussionsByLineCode[lineCode] && this.isDiscussionExpanded(lineCode);
-
-      // Avoid rendering context line discussions on the right side in parallel view
-      if (position === LINE_POSITION_RIGHT) {
-        render = render && line.right.type;
-      }
-
-      return render;
-    },
-    hasAnyExpandedDiscussion(line) {
-      const isLeftExpanded = this.isDiscussionExpanded(line.left.lineCode);
-      const isRightExpanded = this.isDiscussionExpanded(line.right.lineCode);
-
-      return isLeftExpanded || isRightExpanded;
-    },
-    getLineCode(line, side) {
-      const { lineCode } = side;
-      if (lineCode) {
-        return lineCode;
-      }
-
-      return `${this.fileHash}_${line.left.oldLine}_${line.right.newLine}`;
-    },
-  },
 };
 </script>
 
@@ -104,119 +28,26 @@ export default {
   <div
     :class="userColorScheme"
     :data-commit-id="commitId"
-    class="code diff-wrap-lines js-syntax-highlight text-file">
+    class="code diff-wrap-lines js-syntax-highlight text-file"
+  >
     <table>
       <tbody>
         <template
           v-for="(line, index) in parallelDiffLines"
         >
-          <tr
-            :key="index"
-            :class="getRowClass(line)"
-            class="line_holder parallel"
-            @mouseover="handleMouse($event, line, true)"
-            @mouseout="handleMouse($event, line, false)"
-          >
-            <td
-              ref="leftLines"
-              :class="getClassName(line, 'left')"
-              class="diff-line-num old_line"
-            >
-              <diff-line-gutter-content
-                :file-hash="fileHash"
-                :line-type="line.left.type"
-                :line-code="line.left.lineCode"
-                :line-number="line.left.oldLine"
-                :meta-data="line.left.metaData"
-                :show-comment-button="true"
-                :context-lines-path="diffFile.contextLinesPath"
-                :is-bottom="index + 1 === diffLinesLength"
-                line-position="left"
-                @showCommentForm="handleShowCommentForm"
-              />
-            </td>
-            <td
-              ref="leftLines"
-              :class="getClassName(line, 'left')"
-              :id="getLineCode(line, line.left)"
-              class="line_content parallel left-side"
-              v-html="line.left.richText"
-            >
-            </td>
-            <td
-              ref="rightLines"
-              :class="getClassName(line, 'right')"
-              class="diff-line-num new_line"
-            >
-              <diff-line-gutter-content
-                :file-hash="fileHash"
-                :line-type="line.right.type"
-                :line-code="line.right.lineCode"
-                :line-number="line.right.newLine"
-                :meta-data="line.right.metaData"
-                :show-comment-button="true"
-                :context-lines-path="diffFile.contextLinesPath"
+          <diff-table-row
+            :diff-file="diffFile"
+            :line="line"
             :is-bottom="index + 1 === diffLinesLength"
-                line-position="right"
-                @showCommentForm="handleShowCommentForm"
+            :key="index"
           />
-            </td>
-            <td
-              ref="rightLines"
-              :class="getClassName(line, 'right')"
-              :id="getLineCode(line, line.right)"
-              class="line_content parallel right-side"
-              v-html="line.right.richText"
-            >
-            </td>
-          </tr>
-          <tr
-            v-if="shouldRenderDiscussionsRow(line)"
+          <parallel-diff-comment-row
             :key="line.left.lineCode || line.right.lineCode"
-            :class="hasDiscussion(line) ? '' : 'js-temp-notes-holder'"
-            class="notes_holder"
-          >
-            <td class="notes_line old"></td>
-            <td class="notes_content parallel old">
-              <div
-                v-if="shouldRenderDiscussions(line, 'left')"
-                class="content"
-              >
-                <diff-discussions
-                  :discussions="discussionsByLineCode[line.left.lineCode]"
-                />
-              </div>
-              <diff-line-note-form
-                v-if="diffLineCommentForms[line.left.lineCode] &&
-                diffLineCommentForms[line.left.lineCode]"
-                :diff-file="diffFile"
-                :diff-lines="diffLines"
-                :line="line.left"
-                :note-target-line="diffLines[index].left"
-                position="left"
-              />
-            </td>
-            <td class="notes_line new"></td>
-            <td class="notes_content parallel new">
-              <div
-                v-if="shouldRenderDiscussions(line, 'right')"
-                class="content"
-              >
-                <diff-discussions
-                  :discussions="discussionsByLineCode[line.right.lineCode]"
-                />
-              </div>
-              <diff-line-note-form
-                v-if="diffLineCommentForms[line.right.lineCode] &&
-                diffLineCommentForms[line.right.lineCode] && line.right.type"
+            :line="line"
             :diff-file="diffFile"
-                :diff-lines="diffLines"
-                :line="line.right"
-                :note-target-line="diffLines[index].right"
-                position="right"
+            :diff-lines="parallelDiffLines"
+            :line-index="index"
           />
-            </td>
-          </tr>
         </template>
       </tbody>
     </table>

app/assets/javascripts/diffs/constants.js

@@ -14,6 +14,8 @@ export const TEXT_DIFF_POSITION_TYPE = 'text';
 
 export const LINE_POSITION_LEFT = 'left';
 export const LINE_POSITION_RIGHT = 'right';
+export const LINE_SIDE_LEFT = 'left-side';
+export const LINE_SIDE_RIGHT = 'right-side';
 
 export const DIFF_VIEW_COOKIE_NAME = 'diff_view';
 export const LINE_HOVER_CLASS_NAME = 'is-over';

app/assets/javascripts/diffs/mixins/diff_content.js

-import { mapState, mapGetters, mapActions } from 'vuex';
+import { mapGetters } from 'vuex';
 import diffDiscussions from '../components/diff_discussions.vue';
 import diffLineGutterContent from '../components/diff_line_gutter_content.vue';
 import diffLineNoteForm from '../components/diff_line_note_form.vue';
+import diffTableRow from '../components/diff_table_row.vue';
 import { trimFirstCharOfLineContent } from '../store/utils';
-import { CONTEXT_LINE_TYPE, CONTEXT_LINE_CLASS_NAME } from '../constants';
 
 export default {
   props: {
@@ -16,22 +16,14 @@ export default {
       required: true,
     },
   },
-  data() {
-    return {
-      hoveredLineCode: null,
-      hoveredSection: null,
-    };
-  },
   components: {
     diffDiscussions,
+    diffTableRow,
     diffLineNoteForm,
     diffLineGutterContent,
   },
   computed: {
-    ...mapState({
-      diffLineCommentForms: state => state.diffs.diffLineCommentForms,
-    }),
-    ...mapGetters(['discussionsByLineCode', 'isLoggedIn', 'commit']),
+    ...mapGetters(['commit']),
     commitId() {
       return this.commit && this.commit.id;
     },
@@ -41,15 +33,15 @@ export default {
     normalizedDiffLines() {
       return this.diffLines.map(line => {
         if (line.richText) {
-          return this.trimFirstChar(line);
+          return trimFirstCharOfLineContent(line);
         }
 
         if (line.left) {
-          Object.assign(line, { left: this.trimFirstChar(line.left) });
+          Object.assign(line, { left: trimFirstCharOfLineContent(line.left) });
         }
 
         if (line.right) {
-          Object.assign(line, { right: this.trimFirstChar(line.right) });
+          Object.assign(line, { right: trimFirstCharOfLineContent(line.right) });
         }
 
         return line;
@@ -62,28 +54,4 @@ export default {
       return this.diffFile.fileHash;
     },
   },
-  methods: {
-    ...mapActions(['showCommentForm', 'cancelCommentForm']),
-    getRowClass(line) {
-      const isContextLine = line.left
-        ? line.left.type === CONTEXT_LINE_TYPE
-        : line.type === CONTEXT_LINE_TYPE;
-
-      return {
-        [line.type]: line.type,
-        [CONTEXT_LINE_CLASS_NAME]: isContextLine,
-      };
-    },
-    trimFirstChar(line) {
-      return trimFirstCharOfLineContent(line);
-    },
-    handleShowCommentForm(params) {
-      this.showCommentForm({ lineCode: params.lineCode });
-    },
-    isDiscussionExpanded(lineCode) {
-      const discussions = this.discussionsByLineCode[lineCode];
-
-      return discussions ? discussions.every(discussion => discussion.expanded) : false;
-    },
-  },
 };

app/assets/stylesheets/highlight/white_base.scss

@@ -102,7 +102,9 @@ pre.code,
 // Diff line
 .line_holder {
 
-  &.match .line_content {
+  &.match .line_content,
+  .new-nonewline.line_content,
+  .old-nonewline.line_content {
     @include matchLine;
   }
 

spec/javascripts/diffs/components/diff_line_gutter_content_spec.js

@@ -2,12 +2,6 @@ import Vue from 'vue';
 import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue';
 import store from '~/mr_notes/stores';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
-import {
-  MATCH_LINE_TYPE,
-  CONTEXT_LINE_TYPE,
-  OLD_NO_NEW_LINE_TYPE,
-  NEW_NO_NEW_LINE_TYPE,
-} from '~/diffs/constants';
 import discussionsMockData from '../mock_data/diff_discussions';
 import diffFileMockData from '../mock_data/diff_file';
 
@@ -31,45 +25,6 @@ describe('DiffLineGutterContent', () => {
   };
 
   describe('computed', () => {
-    describe('isMatchLine', () => {
-      it('should return true for match line type', () => {
-        const component = createComponent({ lineType: MATCH_LINE_TYPE });
-        expect(component.isMatchLine).toEqual(true);
-      });
-
-      it('should return false for non-match line type', () => {
-        const component = createComponent({ lineType: CONTEXT_LINE_TYPE });
-        expect(component.isMatchLine).toEqual(false);
-      });
-    });
-
-    describe('isContextLine', () => {
-      it('should return true for context line type', () => {
-        const component = createComponent({ lineType: CONTEXT_LINE_TYPE });
-        expect(component.isContextLine).toEqual(true);
-      });
-
-      it('should return false for non-context line type', () => {
-        const component = createComponent({ lineType: MATCH_LINE_TYPE });
-        expect(component.isContextLine).toEqual(false);
-      });
-    });
-
-    describe('isMetaLine', () => {
-      it('should return true for meta line type', () => {
-        const component = createComponent({ lineType: NEW_NO_NEW_LINE_TYPE });
-        expect(component.isMetaLine).toEqual(true);
-
-        const component2 = createComponent({ lineType: OLD_NO_NEW_LINE_TYPE });
-        expect(component2.isMetaLine).toEqual(true);
-      });
-
-      it('should return false for non-meta line type', () => {
-        const component = createComponent({ lineType: MATCH_LINE_TYPE });
-        expect(component.isMetaLine).toEqual(false);
-      });
-    });
-
     describe('lineHref', () => {
       it('should prepend # to lineCode', () => {
         const lineCode = 'LC_42';
@@ -109,7 +64,7 @@ describe('DiffLineGutterContent', () => {
   describe('template', () => {
     it('should render three dots for context lines', () => {
       const component = createComponent({
-        lineType: MATCH_LINE_TYPE,
+        isMatchLine: true,
       });
 
       expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true);

spec/javascripts/diffs/components/inline_diff_view_spec.js

 import Vue from 'vue';
 import InlineDiffView from '~/diffs/components/inline_diff_view.vue';
 import store from '~/mr_notes/stores';
-import * as constants from '~/diffs/constants';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import diffFileMockData from '../mock_data/diff_file';
 import discussionsMockData from '../mock_data/diff_discussions';
@@ -14,58 +13,13 @@ describe('InlineDiffView', () => {
   beforeEach(() => {
     const diffFile = getDiffFileMock();
 
+    store.dispatch('setInlineDiffViewType');
     component = createComponentWithStore(Vue.extend(InlineDiffView), store, {
       diffFile,
       diffLines: diffFile.highlightedDiffLines,
     }).$mount();
   });
 
-  describe('methods', () => {
-    describe('handleMouse', () => {
-      it('should set hoveredLineCode', () => {
-        expect(component.hoveredLineCode).toEqual(null);
-
-        component.handleMouse('lineCode1', true);
-        expect(component.hoveredLineCode).toEqual('lineCode1');
-
-        component.handleMouse('lineCode1', false);
-        expect(component.hoveredLineCode).toEqual(null);
-      });
-    });
-
-    describe('getLineClass', () => {
-      it('should return line class object', () => {
-        const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants;
-        const { MATCH_LINE_TYPE, NEW_LINE_TYPE } = constants;
-
-        expect(component.getLineClass(component.diffLines[0])).toEqual({
-          [NEW_LINE_TYPE]: NEW_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: false,
-          [LINE_HOVER_CLASS_NAME]: false,
-        });
-
-        component.handleMouse(component.diffLines[0].lineCode, true);
-        Object.defineProperty(component, 'isLoggedIn', {
-          get() {
-            return true;
-          },
-        });
-
-        expect(component.getLineClass(component.diffLines[0])).toEqual({
-          [NEW_LINE_TYPE]: NEW_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: false,
-          [LINE_HOVER_CLASS_NAME]: true,
-        });
-
-        expect(component.getLineClass(component.diffLines[5])).toEqual({
-          [MATCH_LINE_TYPE]: MATCH_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: true,
-          [LINE_HOVER_CLASS_NAME]: false,
-        });
-      });
-    });
-  });
-
   describe('template', () => {
     it('should have rendered diff lines', () => {
       const el = component.$el;
@@ -89,23 +43,5 @@ describe('InlineDiffView', () => {
         done();
       });
     });
-
-    it('should render new discussion forms', done => {
-      const el = component.$el;
-      const lines = getDiffFileMock().highlightedDiffLines;
-
-      component.handleShowCommentForm({ lineCode: lines[0].lineCode });
-      component.handleShowCommentForm({ lineCode: lines[1].lineCode });
-
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2);
-        expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true);
-        expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true);
-
-        store.state.diffs.diffLineCommentForms = {};
-
-        done();
-      });
-    });
   });
 });

spec/javascripts/diffs/components/parallel_diff_view_spec.js

@@ -4,12 +4,10 @@ import store from '~/mr_notes/stores';
 import * as constants from '~/diffs/constants';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import diffFileMockData from '../mock_data/diff_file';
-import discussionsMockData from '../mock_data/diff_discussions';
 
 describe('ParallelDiffView', () => {
   let component;
   const getDiffFileMock = () => Object.assign({}, diffFileMockData);
-  const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
 
   beforeEach(() => {
     const diffFile = getDiffFileMock();
@@ -28,197 +26,4 @@ describe('ParallelDiffView', () => {
       });
     });
   });
-
-  describe('methods', () => {
-    describe('hasDiscussion', () => {
-      it('it should return true if there is a discussion either for left or right section', () => {
-        Object.defineProperty(component, 'discussionsByLineCode', {
-          get() {
-            return { line_42: true };
-          },
-        });
-
-        expect(component.hasDiscussion({ left: {}, right: {} })).toEqual(undefined);
-        expect(component.hasDiscussion({ left: { lineCode: 'line_42' }, right: {} })).toEqual(true);
-        expect(component.hasDiscussion({ left: {}, right: { lineCode: 'line_42' } })).toEqual(true);
-      });
-    });
-
-    describe('getClassName', () => {
-      it('should return line class object', () => {
-        const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants;
-        const { MATCH_LINE_TYPE, NEW_LINE_TYPE, LINE_POSITION_RIGHT } = constants;
-
-        expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({
-          [NEW_LINE_TYPE]: NEW_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: false,
-          [LINE_HOVER_CLASS_NAME]: false,
-        });
-
-        const eventMock = {
-          target: component.$refs.rightLines[1],
-        };
-
-        component.handleMouse(eventMock, component.diffLines[1], true);
-        Object.defineProperty(component, 'isLoggedIn', {
-          get() {
-            return true;
-          },
-        });
-
-        expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({
-          [NEW_LINE_TYPE]: NEW_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: false,
-          [LINE_HOVER_CLASS_NAME]: true,
-        });
-
-        expect(component.getClassName(component.diffLines[5], LINE_POSITION_RIGHT)).toEqual({
-          [MATCH_LINE_TYPE]: MATCH_LINE_TYPE,
-          [LINE_UNFOLD_CLASS_NAME]: true,
-          [LINE_HOVER_CLASS_NAME]: false,
-        });
-      });
-    });
-
-    describe('handleMouse', () => {
-      it('should set hovered line code and line section to null when isHover is false', () => {
-        const rightLineEventMock = { target: component.$refs.rightLines[1] };
-        expect(component.hoveredLineCode).toEqual(null);
-        expect(component.hoveredSection).toEqual(null);
-
-        component.handleMouse(rightLineEventMock, null, false);
-        expect(component.hoveredLineCode).toEqual(null);
-        expect(component.hoveredSection).toEqual(null);
-      });
-
-      it('should set hovered line code and line section for right section', () => {
-        const rightLineEventMock = { target: component.$refs.rightLines[1] };
-        component.handleMouse(rightLineEventMock, component.diffLines[1], true);
-        expect(component.hoveredLineCode).toEqual(component.diffLines[1].right.lineCode);
-        expect(component.hoveredSection).toEqual(constants.LINE_POSITION_RIGHT);
-      });
-
-      it('should set hovered line code and line section for left section', () => {
-        const leftLineEventMock = { target: component.$refs.leftLines[2] };
-        component.handleMouse(leftLineEventMock, component.diffLines[2], true);
-        expect(component.hoveredLineCode).toEqual(component.diffLines[2].left.lineCode);
-        expect(component.hoveredSection).toEqual(constants.LINE_POSITION_LEFT);
-      });
-    });
-
-    describe('shouldRenderDiscussions', () => {
-      it('should return true if there is a discussion on left side and it is expanded', () => {
-        const line = { left: { lineCode: 'lineCode1' } };
-        spyOn(component, 'isDiscussionExpanded').and.returnValue(true);
-        Object.defineProperty(component, 'discussionsByLineCode', {
-          get() {
-            return {
-              [line.left.lineCode]: true,
-            };
-          },
-        });
-
-        expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual(true);
-        expect(component.isDiscussionExpanded).toHaveBeenCalledWith(line.left.lineCode);
-      });
-
-      it('should return false if there is a discussion on left side but it is collapsed', () => {
-        const line = { left: { lineCode: 'lineCode1' } };
-        spyOn(component, 'isDiscussionExpanded').and.returnValue(false);
-        Object.defineProperty(component, 'discussionsByLineCode', {
-          get() {
-            return {
-              [line.left.lineCode]: true,
-            };
-          },
-        });
-
-        expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual(
-          false,
-        );
-      });
-
-      it('should return false for discussions on the right side if there is no line type', () => {
-        const CUSTOM_RIGHT_LINE_TYPE = 'CUSTOM_RIGHT_LINE_TYPE';
-        const line = { right: { lineCode: 'lineCode1', type: CUSTOM_RIGHT_LINE_TYPE } };
-        spyOn(component, 'isDiscussionExpanded').and.returnValue(true);
-        Object.defineProperty(component, 'discussionsByLineCode', {
-          get() {
-            return {
-              [line.right.lineCode]: true,
-            };
-          },
-        });
-
-        expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_RIGHT)).toEqual(
-          CUSTOM_RIGHT_LINE_TYPE,
-        );
-      });
-    });
-
-    describe('hasAnyExpandedDiscussion', () => {
-      const LINE_CODE_LEFT = 'LINE_CODE_LEFT';
-      const LINE_CODE_RIGHT = 'LINE_CODE_RIGHT';
-
-      it('should return true if there is a discussion either on the left or the right side', () => {
-        const mockLineOne = {
-          right: { lineCode: LINE_CODE_RIGHT },
-          left: {},
-        };
-        const mockLineTwo = {
-          left: { lineCode: LINE_CODE_LEFT },
-          right: {},
-        };
-
-        spyOn(component, 'isDiscussionExpanded').and.callFake(lc => lc === LINE_CODE_RIGHT);
-        expect(component.hasAnyExpandedDiscussion(mockLineOne)).toEqual(true);
-        expect(component.hasAnyExpandedDiscussion(mockLineTwo)).toEqual(false);
-      });
-    });
-  });
-
-  describe('template', () => {
-    it('should have rendered diff lines', () => {
-      const el = component.$el;
-
-      expect(el.querySelectorAll('tr.line_holder.parallel').length).toEqual(6);
-      expect(el.querySelectorAll('td.empty-cell').length).toEqual(4);
-      expect(el.querySelectorAll('td.line_content.parallel.right-side').length).toEqual(6);
-      expect(el.querySelectorAll('td.line_content.parallel.left-side').length).toEqual(6);
-      expect(el.querySelectorAll('td.match').length).toEqual(4);
-      expect(el.textContent.indexOf('Bad dates') > -1).toEqual(true);
-    });
-
-    it('should render discussions', done => {
-      const el = component.$el;
-      component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
-
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
-        expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5);
-        expect(el.innerText.indexOf('comment 5') > -1).toEqual(true);
-        component.$store.dispatch('setInitialNotes', []);
-
-        done();
-      });
-    });
-
-    it('should render new discussion forms', done => {
-      const el = component.$el;
-      const lines = getDiffFileMock().parallelDiffLines;
-
-      component.handleShowCommentForm({ lineCode: lines[0].lineCode });
-      component.handleShowCommentForm({ lineCode: lines[1].lineCode });
-
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2);
-        expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true);
-        expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true);
-
-        store.state.diffs.diffLineCommentForms = {};
-
-        done();
-      });
-    });
-  });
 });