Merge branch '58035-expand-mr-diff' into 'master'

Resolve "Incrementally expand merge request diffs in both direction"

Closes #58035

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

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

+<script>
+import createFlash from '~/flash';
+import { s__ } from '~/locale';
+import { mapState, mapActions } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
+import { UNFOLD_COUNT } from '../constants';
+import * as utils from '../store/utils';
+import tooltip from '../../vue_shared/directives/tooltip';
+
+const EXPAND_ALL = 0;
+const EXPAND_UP = 1;
+const EXPAND_DOWN = 2;
+
+export default {
+  directives: {
+    tooltip,
+  },
+  components: {
+    Icon,
+  },
+  props: {
+    fileHash: {
+      type: String,
+      required: true,
+    },
+    contextLinesPath: {
+      type: String,
+      required: true,
+    },
+    line: {
+      type: Object,
+      required: true,
+    },
+    isTop: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isBottom: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    colspan: {
+      type: Number,
+      required: false,
+      default: 3,
+    },
+  },
+  computed: {
+    ...mapState({
+      diffViewType: state => state.diffs.diffViewType,
+      diffFiles: state => state.diffs.diffFiles,
+    }),
+    canExpandUp() {
+      return !this.isBottom;
+    },
+    canExpandDown() {
+      return this.isBottom || !this.isTop;
+    },
+  },
+  created() {
+    this.EXPAND_DOWN = EXPAND_DOWN;
+    this.EXPAND_UP = EXPAND_UP;
+  },
+  methods: {
+    ...mapActions('diffs', ['loadMoreLines']),
+    getPrevLineNumber(oldLineNumber, newLineNumber) {
+      const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash);
+      const indexForInline = utils.findIndexInInlineLines(diffFile.highlighted_diff_lines, {
+        oldLineNumber,
+        newLineNumber,
+      });
+      const prevLine = diffFile.highlighted_diff_lines[indexForInline - 2];
+      return (prevLine && prevLine.new_line) || 0;
+    },
+    callLoadMoreLines(
+      endpoint,
+      params,
+      lineNumbers,
+      fileHash,
+      isExpandDown = false,
+      nextLineNumbers = {},
+    ) {
+      this.loadMoreLines({ endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers })
+        .then(() => {
+          this.isRequesting = false;
+        })
+        .catch(() => {
+          createFlash(s__('Diffs|Something went wrong while fetching diff lines.'));
+          this.isRequesting = false;
+        });
+    },
+    handleExpandLines(type = EXPAND_ALL) {
+      if (this.isRequesting) {
+        return;
+      }
+
+      this.isRequesting = true;
+      const endpoint = this.contextLinesPath;
+      const { fileHash } = this;
+      const view = this.diffViewType;
+      const oldLineNumber = this.line.meta_data.old_pos || 0;
+      const newLineNumber = this.line.meta_data.new_pos || 0;
+      const offset = newLineNumber - oldLineNumber;
+
+      const expandOptions = { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset };
+
+      if (type === EXPAND_UP) {
+        this.handleExpandUpLines(expandOptions);
+      } else if (type === EXPAND_DOWN) {
+        this.handleExpandDownLines(expandOptions);
+      } else {
+        this.handleExpandAllLines(expandOptions);
+      }
+    },
+    handleExpandUpLines(expandOptions = EXPAND_ALL) {
+      const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions;
+
+      const bottom = this.isBottom;
+      const lineNumber = newLineNumber - 1;
+      const to = lineNumber;
+      let since = lineNumber - UNFOLD_COUNT;
+      let unfold = true;
+
+      const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
+      if (since <= prevLineNumber + 1) {
+        since = prevLineNumber + 1;
+        unfold = false;
+      }
+
+      const params = { since, to, bottom, offset, unfold, view };
+      const lineNumbers = { oldLineNumber, newLineNumber };
+      this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash);
+    },
+    handleExpandDownLines(expandOptions) {
+      const {
+        endpoint,
+        fileHash,
+        view,
+        oldLineNumber: metaOldPos,
+        newLineNumber: metaNewPos,
+        offset,
+      } = expandOptions;
+
+      const bottom = true;
+      const nextLineNumbers = {
+        old_line: metaOldPos,
+        new_line: metaNewPos,
+      };
+
+      let unfold = true;
+      let isExpandDown = false;
+      let oldLineNumber = metaOldPos;
+      let newLineNumber = metaNewPos;
+      let lineNumber = metaNewPos + 1;
+      let since = lineNumber;
+      let to = lineNumber + UNFOLD_COUNT;
+
+      if (!this.isBottom) {
+        const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
+
+        isExpandDown = true;
+        oldLineNumber = prevLineNumber - offset;
+        newLineNumber = prevLineNumber;
+        lineNumber = prevLineNumber + 1;
+        since = lineNumber;
+        to = lineNumber + UNFOLD_COUNT;
+
+        if (to >= metaNewPos) {
+          to = metaNewPos - 1;
+          unfold = false;
+        }
+      }
+
+      const params = { since, to, bottom, offset, unfold, view };
+      const lineNumbers = { oldLineNumber, newLineNumber };
+      this.callLoadMoreLines(
+        endpoint,
+        params,
+        lineNumbers,
+        fileHash,
+        isExpandDown,
+        nextLineNumbers,
+      );
+    },
+    handleExpandAllLines(expandOptions) {
+      const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions;
+      const bottom = this.isBottom;
+      const unfold = false;
+      let since;
+      let to;
+
+      if (this.isTop) {
+        since = 1;
+        to = newLineNumber - 1;
+      } else if (bottom) {
+        since = newLineNumber + 1;
+        to = -1;
+      } else {
+        const prevLineNumber = this.getPrevLineNumber(oldLineNumber, newLineNumber);
+        since = prevLineNumber + 1;
+        to = newLineNumber - 1;
+      }
+
+      const params = { since, to, bottom, offset, unfold, view };
+      const lineNumbers = { oldLineNumber, newLineNumber };
+      this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash);
+    },
+  },
+};
+</script>
+
+<template>
+  <td :colspan="colspan">
+    <div class="content">
+      <a
+        v-if="canExpandUp"
+        v-tooltip
+        class="cursor-pointer js-unfold unfold-icon"
+        data-placement="top"
+        data-container="body"
+        :title="__('Expand up')"
+        @click="handleExpandLines(EXPAND_UP)"
+      >
+        <!-- TODO: remove style & replace with correct icon, waiting for MR https://gitlab.com/gitlab-org/gitlab-design/issues/499 -->
+        <icon :size="12" name="expand-left" aria-hidden="true" style="transform: rotate(270deg);" />
+      </a>
+      <a class="mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()">
+        <span>{{ s__('Diffs|Show all lines') }}</span>
+      </a>
+      <a
+        v-if="canExpandDown"
+        v-tooltip
+        class="cursor-pointer js-unfold-down has-tooltip unfold-icon"
+        data-placement="top"
+        data-container="body"
+        :title="__('Expand down')"
+        @click="handleExpandLines(EXPAND_DOWN)"
+      >
+        <!-- TODO: remove style & replace with correct icon, waiting for MR https://gitlab.com/gitlab-org/gitlab-design/issues/499 -->
+        <icon :size="12" name="expand-left" aria-hidden="true" style="transform: rotate(90deg);" />
+      </a>
+    </div>
+  </td>
+</template>

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

 <script>
-import createFlash from '~/flash';
-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 { LINE_POSITION_RIGHT, UNFOLD_COUNT } from '../constants';
-import * as utils from '../store/utils';
+import { LINE_POSITION_RIGHT } from '../constants';
 
 export default {
   components: {
@@ -115,64 +112,12 @@ export default {
     handleCommentButton() {
       this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash });
     },
-    handleLoadMoreLines() {
-      if (this.isRequesting) {
-        return;
-      }
-
-      this.isRequesting = true;
-      const endpoint = this.contextLinesPath;
-      const oldLineNumber = this.line.meta_data.old_pos || 0;
-      const newLineNumber = this.line.meta_data.new_pos || 0;
-      const offset = newLineNumber - oldLineNumber;
-      const bottom = this.isBottom;
-      const { fileHash } = this;
-      const view = this.diffViewType;
-      let unfold = true;
-      let lineNumber = newLineNumber - 1;
-      let since = lineNumber - UNFOLD_COUNT;
-      let to = lineNumber;
-
-      if (bottom) {
-        lineNumber = newLineNumber + 1;
-        since = lineNumber;
-        to = lineNumber + UNFOLD_COUNT;
-      } else {
-        const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash);
-        const indexForInline = utils.findIndexInInlineLines(diffFile.highlighted_diff_lines, {
-          oldLineNumber,
-          newLineNumber,
-        });
-        const prevLine = diffFile.highlighted_diff_lines[indexForInline - 2];
-        const prevLineNumber = (prevLine && prevLine.new_line) || 0;
-
-        if (since <= prevLineNumber + 1) {
-          since = prevLineNumber + 1;
-          unfold = false;
-        }
-      }
-
-      const params = { since, to, bottom, offset, unfold, view };
-      const lineNumbers = { oldLineNumber, newLineNumber };
-      this.loadMoreLines({ endpoint, params, lineNumbers, fileHash })
-        .then(() => {
-          this.isRequesting = false;
-        })
-        .catch(() => {
-          createFlash(s__('Diffs|Something went wrong while fetching diff lines.'));
-          this.isRequesting = false;
-        });
-    },
   },
 };
 </script>
 
 <template>
   <div>
-    <span v-if="isMatchLine" class="context-cell" role="button" @click="handleLoadMoreLines"
-      >...</span
-    >
-    <template v-else>
     <button
       v-if="shouldRenderCommentButton"
       v-show="shouldShowCommentButton"
@@ -198,6 +143,5 @@ export default {
         toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
       "
     />
-    </template>
   </div>
 </template>

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

+<script>
+import Icon from '~/vue_shared/components/icon.vue';
+import DiffExpansionCell from './diff_expansion_cell.vue';
+import { MATCH_LINE_TYPE } from '../constants';
+
+export default {
+  components: {
+    Icon,
+    DiffExpansionCell,
+  },
+  props: {
+    fileHash: {
+      type: String,
+      required: true,
+    },
+    contextLinesPath: {
+      type: String,
+      required: true,
+    },
+    line: {
+      type: Object,
+      required: true,
+    },
+    isTop: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isBottom: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    isMatchLine() {
+      return this.line.type === MATCH_LINE_TYPE;
+    },
+  },
+};
+</script>
+
+<template>
+  <tr v-if="isMatchLine" class="line_expansion match">
+    <diff-expansion-cell
+      :file-hash="fileHash"
+      :context-lines-path="contextLinesPath"
+      :line="line"
+      :is-top="isTop"
+      :is-bottom="isBottom"
+    />
+  </tr>
+</template>

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

@@ -2,6 +2,7 @@
 import { mapActions, mapState } from 'vuex';
 import DiffTableCell from './diff_table_cell.vue';
 import {
+  MATCH_LINE_TYPE,
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
   CONTEXT_LINE_TYPE,
@@ -58,6 +59,9 @@ export default {
     inlineRowId() {
       return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`;
     },
+    isMatchLine() {
+      return this.line.type === MATCH_LINE_TYPE;
+    },
   },
   created() {
     this.newLineType = NEW_LINE_TYPE;
@@ -81,6 +85,7 @@ export default {
 
 <template>
   <tr
+    v-if="!isMatchLine"
     :id="inlineRowId"
     :class="classNameMap"
     class="line_holder"

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

@@ -3,6 +3,7 @@ import { mapGetters } from 'vuex';
 import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
 import inlineDiffTableRow from './inline_diff_table_row.vue';
 import inlineDiffCommentRow from './inline_diff_comment_row.vue';
+import inlineDiffExpansionRow from './inline_diff_expansion_row.vue';
 
 export default {
   components: {
@@ -10,6 +11,7 @@ export default {
     inlineDiffTableRow,
     InlineDraftCommentRow: () =>
       import('ee_component/batch_comments/components/inline_draft_comment_row.vue'),
+    inlineDiffExpansionRow,
   },
   mixins: [draftCommentsMixin],
   props: {
@@ -43,10 +45,24 @@ export default {
     :data-commit-id="commitId"
     class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"
   >
+    <!-- Need to insert an empty row to solve "table-layout:fixed" equal width when expansion row is the first line -->
+    <tr>
+      <td style="width: 50px;"></td>
+      <td style="width: 50px;"></td>
+      <td></td>
+    </tr>
     <tbody>
       <template v-for="(line, index) in diffLines">
+        <inline-diff-expansion-row
+          :key="`expand-${index}`"
+          :file-hash="diffFile.file_hash"
+          :context-lines-path="diffFile.context_lines_path"
+          :line="line"
+          :is-top="index === 0"
+          :is-bottom="index + 1 === diffLinesLength"
+        />
         <inline-diff-table-row
-          :key="line.line_code"
+          :key="`${line.line_code || index}`"
           :file-hash="diffFile.file_hash"
           :context-lines-path="diffFile.context_lines_path"
           :line="line"

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

+<script>
+import { MATCH_LINE_TYPE } from '../constants';
+import DiffExpansionCell from './diff_expansion_cell.vue';
+
+export default {
+  components: {
+    DiffExpansionCell,
+  },
+  props: {
+    fileHash: {
+      type: String,
+      required: true,
+    },
+    contextLinesPath: {
+      type: String,
+      required: true,
+    },
+    line: {
+      type: Object,
+      required: true,
+    },
+    isTop: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    isBottom: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    isMatchLineLeft() {
+      return this.line.left && this.line.left.type === MATCH_LINE_TYPE;
+    },
+    isMatchLineRight() {
+      return this.line.right && this.line.right.type === MATCH_LINE_TYPE;
+    },
+  },
+};
+</script>
+<template>
+  <tr class="line_expansion match">
+    <template v-if="isMatchLineLeft || isMatchLineRight">
+      <diff-expansion-cell
+        :file-hash="fileHash"
+        :context-lines-path="contextLinesPath"
+        :line="line.left"
+        :is-top="isTop"
+        :is-bottom="isBottom"
+        :colspan="4"
+      />
+    </template>
+  </tr>
+</template>

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

@@ -3,6 +3,7 @@ import { mapActions, mapState } from 'vuex';
 import $ from 'jquery';
 import DiffTableCell from './diff_table_cell.vue';
 import {
+  MATCH_LINE_TYPE,
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
   CONTEXT_LINE_TYPE,
@@ -75,6 +76,12 @@ export default {
         },
       ];
     },
+    isMatchLineLeft() {
+      return this.line.left && this.line.left.type === MATCH_LINE_TYPE;
+    },
+    isMatchLineRight() {
+      return this.line.right && this.line.right.type === MATCH_LINE_TYPE;
+    },
   },
   created() {
     this.newLineType = NEW_LINE_TYPE;
@@ -122,7 +129,7 @@ export default {
     @mouseover="handleMouseMove"
     @mouseout="handleMouseMove"
   >
-    <template v-if="line.left">
+    <template v-if="line.left && !isMatchLineLeft">
       <diff-table-cell
         :file-hash="fileHash"
         :context-lines-path="contextLinesPath"
@@ -148,7 +155,7 @@ export default {
       <td class="diff-line-num old_line empty-cell"></td>
       <td class="line_content parallel left-side empty-cell"></td>
     </template>
-    <template v-if="line.right">
+    <template v-if="line.right && !isMatchLineRight">
       <diff-table-cell
         :file-hash="fileHash"
         :context-lines-path="contextLinesPath"

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

@@ -3,9 +3,11 @@ import { mapGetters } from 'vuex';
 import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
 import parallelDiffTableRow from './parallel_diff_table_row.vue';
 import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
+import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue';
 
 export default {
   components: {
+    parallelDiffExpansionRow,
     parallelDiffTableRow,
     parallelDiffCommentRow,
     ParallelDraftCommentRow: () =>
@@ -43,8 +45,23 @@ export default {
     :data-commit-id="commitId"
     class="code diff-wrap-lines js-syntax-highlight text-file"
   >
+    <!-- Need to insert an empty row to solve "table-layout:fixed" equal width when expansion row is the first line -->
+    <tr>
+      <td style="width: 50px;"></td>
+      <td></td>
+      <td style="width: 50px;"></td>
+      <td></td>
+    </tr>
     <tbody>
       <template v-for="(line, index) in diffLines">
+        <parallel-diff-expansion-row
+          :key="`expand-${index}`"
+          :file-hash="diffFile.file_hash"
+          :context-lines-path="diffFile.context_lines_path"
+          :line="line"
+          :is-top="index === 0"
+          :is-bottom="index + 1 === diffLinesLength"
+        />
         <parallel-diff-table-row
           :key="line.line_code"
           :file-hash="diffFile.file_hash"

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

@@ -183,7 +183,7 @@ export const cancelCommentForm = ({ commit }, { lineCode, fileHash }) => {
 };
 
 export const loadMoreLines = ({ commit }, options) => {
-  const { endpoint, params, lineNumbers, fileHash } = options;
+  const { endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers } = options;
 
   params.from_merge_request = true;
 
@@ -195,6 +195,8 @@ export const loadMoreLines = ({ commit }, options) => {
       contextLines,
       params,
       fileHash,
+      isExpandDown,
+      nextLineNumbers,
     });
   });
 };

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

@@ -71,18 +71,30 @@ export default {
   },
 
   [types.ADD_CONTEXT_LINES](state, options) {
-    const { lineNumbers, contextLines, fileHash } = options;
+    const { lineNumbers, contextLines, fileHash, isExpandDown, nextLineNumbers } = options;
     const { bottom } = options.params;
     const diffFile = findDiffFile(state.diffFiles, fileHash);
 
     removeMatchLine(diffFile, lineNumbers, bottom);
 
-    const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({
+    const lines = addLineReferences(
+      contextLines,
+      lineNumbers,
+      bottom,
+      isExpandDown,
+      nextLineNumbers,
+    ).map(line => {
+      const lineCode =
+        line.type === 'match'
+          ? `${fileHash}_${line.meta_data.old_pos}_${line.meta_data.new_pos}_match`
+          : line.line_code || `${fileHash}_${line.old_line}_${line.new_line}`;
+      return {
         ...line,
-      line_code: line.line_code || `${fileHash}_${line.old_line}_${line.new_line}`,
+        line_code: lineCode,
         discussions: line.discussions || [],
         hasForm: false,
-    }));
+      };
+    });
 
     addContextLines({
       inlineLines: diffFile.highlighted_diff_lines,
@@ -90,6 +102,7 @@ export default {
       contextLines: lines,
       bottom,
       lineNumbers,
+      isExpandDown,
     });
   },
 

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

@@ -121,7 +121,7 @@ export function removeMatchLine(diffFile, lineNumbers, bottom) {
   diffFile.parallel_diff_lines.splice(indexForParallel + factor, 1);
 }
 
-export function addLineReferences(lines, lineNumbers, bottom) {
+export function addLineReferences(lines, lineNumbers, bottom, isExpandDown, nextLineNumbers) {
   const { oldLineNumber, newLineNumber } = lineNumbers;
   const lineCount = lines.length;
   let matchLineIndex = -1;
@@ -135,15 +135,20 @@ export function addLineReferences(lines, lineNumbers, bottom) {
         new_line: bottom ? newLineNumber + index + 1 : newLineNumber + index - lineCount,
       });
     }
-
     return l;
   });
 
   if (matchLineIndex > -1) {
     const line = linesWithNumbers[matchLineIndex];
-    const targetLine = bottom
-      ? linesWithNumbers[matchLineIndex - 1]
-      : linesWithNumbers[matchLineIndex + 1];
+    let targetLine;
+
+    if (isExpandDown) {
+      targetLine = nextLineNumbers;
+    } else if (bottom) {
+      targetLine = linesWithNumbers[matchLineIndex - 1];
+    } else {
+      targetLine = linesWithNumbers[matchLineIndex + 1];
+    }
 
     Object.assign(line, {
       meta_data: {
@@ -152,26 +157,27 @@ export function addLineReferences(lines, lineNumbers, bottom) {
       },
     });
   }
-
   return linesWithNumbers;
 }
 
 export function addContextLines(options) {
-  const { inlineLines, parallelLines, contextLines, lineNumbers } = options;
+  const { inlineLines, parallelLines, contextLines, lineNumbers, isExpandDown } = options;
   const normalizedParallelLines = contextLines.map(line => ({
     left: line,
     right: line,
     line_code: line.line_code,
   }));
+  const factor = isExpandDown ? 1 : 0;
 
-  if (options.bottom) {
+  if (!isExpandDown && options.bottom) {
     inlineLines.push(...contextLines);
     parallelLines.push(...normalizedParallelLines);
   } else {
     const inlineIndex = findIndexInInlineLines(inlineLines, lineNumbers);
     const parallelIndex = findIndexInParallelLines(parallelLines, lineNumbers);
-    inlineLines.splice(inlineIndex, 0, ...contextLines);
-    parallelLines.splice(parallelIndex, 0, ...normalizedParallelLines);
+
+    inlineLines.splice(inlineIndex + factor, 0, ...contextLines);
+    parallelLines.splice(parallelIndex + factor, 0, ...normalizedParallelLines);
   }
 }
 

app/assets/stylesheets/highlight/white_base.scss

@@ -101,6 +101,26 @@ pre.code,
   color: $white-code-color;
 }
 
+// Expansion line
+.line_expansion {
+  background-color: $gray-light;
+
+  td {
+    border-top: 1px solid $border-color;
+    border-bottom: 1px solid $border-color;
+    text-align: center;
+  }
+
+  a {
+    color: $blue-600;
+  }
+
+  .unfold-icon {
+    display: inline-block;
+    padding: 8px 0;
+  }
+}
+
 // Diff line
 .line_holder {
   &.match .line_content,

app/assets/stylesheets/pages/diff.scss

@@ -408,6 +408,14 @@ table.code {
   table-layout: fixed;
   border-radius: 0 0 $border-radius-default $border-radius-default;
 
+  tr:first-of-type.line_expansion > td {
+    border-top: 0;
+  }
+
+  tr:nth-last-of-type(2).line_expansion > td {
+    border-bottom: 0;
+  }
+
   tr.line_holder td {
     line-height: $code-line-height;
     font-size: $code-font-size;

app/presenters/blobs/unfold_presenter.rb

@@ -9,7 +9,7 @@ module Blobs
 
     attribute :full, Boolean, default: false
     attribute :since, GtOneCoercion
-    attribute :to, GtOneCoercion
+    attribute :to, Integer
     attribute :bottom, Boolean
     attribute :unfold, Boolean, default: true
     attribute :offset, Integer
@@ -24,9 +24,7 @@ module Blobs
       @all_lines = blob.data.lines
       super(params)
 
-      if full?
-        self.attributes = { since: 1, to: @all_lines.size, bottom: false, unfold: false, offset: 0, indent: 0 }
-      end
+      self.attributes = prepare_attributes
     end
 
     # Returns an array of Gitlab::Diff::Line with match line added
@@ -43,7 +41,9 @@ module Blobs
     end
 
     def lines
-      @lines ||= limit(highlight.lines).map(&:html_safe)
+      strong_memoize(:lines) do
+        limit(highlight.lines).map(&:html_safe)
+      end
     end
 
     def match_line_text
@@ -56,10 +56,34 @@ module Blobs
 
     private
 
+    def prepare_attributes
+      return attributes unless full? || to == -1
+
+      full_opts = {
+        since: 1,
+        to: all_lines_size,
+        bottom: false,
+        unfold: false,
+        offset: 0,
+        indent: 0
+      }
+
+      return full_opts if full?
+
+      full_opts.merge(attributes.slice(:since))
+    end
+
+    def all_lines_size
+      strong_memoize(:all_lines_size) do
+        @all_lines.size
+      end
+    end
+
     def add_match_line(diff_lines)
       return unless unfold?
+      return if bottom? && to >= all_lines_size
 
-      if bottom? && to < @all_lines.size
+      if bottom? && to < all_lines_size
         old_pos = to - offset
         new_pos = to
       elsif since != 1
@@ -75,7 +99,9 @@ module Blobs
     end
 
     def limited_blob_lines
-      @limited_blob_lines ||= limit(@all_lines)
+      strong_memoize(:limited_blob_lines) do
+        limit(@all_lines)
+      end
     end
 
     def limit(lines)

changelogs/unreleased/58035-expand-mr-diff.yml

+---
+title: Add new expansion options for merge request diffs
+merge_request: 30927
+author:
+type: added

locale/gitlab.pot

@@ -3899,6 +3899,9 @@ msgstr ""
 msgid "Diffs|No file name available"
 msgstr ""
 
+msgid "Diffs|Show all lines"
+msgstr ""
+
 msgid "Diffs|Something went wrong while fetching diff lines."
 msgstr ""
 
@@ -4649,12 +4652,18 @@ msgstr ""
 msgid "Expand all"
 msgstr ""
 
+msgid "Expand down"
+msgstr ""
+
 msgid "Expand dropdown"
 msgstr ""
 
 msgid "Expand sidebar"
 msgstr ""
 
+msgid "Expand up"
+msgstr ""
+
 msgid "Expiration date"
 msgstr ""
 

spec/features/merge_request/user_posts_diff_notes_spec.rb

@@ -68,12 +68,12 @@ describe 'Merge request > User posts diff notes', :js do
     context 'with a match line' do
       it 'does not allow commenting on the left side' do
         line_holder = find('.match', match: :first).find(:xpath, '..')
-        should_not_allow_commenting(line_holder, 'left')
+        match_should_not_allow_commenting(line_holder)
       end
 
       it 'does not allow commenting on the right side' do
         line_holder = find('.match', match: :first).find(:xpath, '..')
-        should_not_allow_commenting(line_holder, 'right')
+        match_should_not_allow_commenting(line_holder)
       end
     end
 
@@ -136,7 +136,7 @@ describe 'Merge request > User posts diff notes', :js do
 
     context 'with a match line' do
       it 'does not allow commenting' do
-        should_not_allow_commenting(find('.match', match: :first))
+        match_should_not_allow_commenting(find('.match', match: :first))
       end
     end
 
@@ -222,7 +222,7 @@ describe 'Merge request > User posts diff notes', :js do
 
     context 'with a match line' do
       it 'does not allow commenting' do
-        should_not_allow_commenting(find('.match', match: :first))
+        match_should_not_allow_commenting(find('.match', match: :first))
       end
     end
   end
@@ -251,6 +251,10 @@ describe 'Merge request > User posts diff notes', :js do
     expect(line[:num]).not_to have_css comment_button_class
   end
 
+  def match_should_not_allow_commenting(line_holder)
+    expect(line_holder).not_to have_css comment_button_class
+  end
+
   def write_comment_on_line(line_holder, diff_side)
     click_diff_line(line_holder, diff_side)
 

spec/features/merge_request/user_views_diffs_spec.rb

@@ -17,11 +17,25 @@ describe 'User views diffs', :js do
   end
 
   shared_examples 'unfold diffs' do
-    it 'unfolds diffs' do
+    it 'unfolds diffs upwards' do
       first('.js-unfold').click
-
       expect(find('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"] .text-file')).to have_content('.bundle')
     end
+
+    it 'unfolds diffs to the start' do
+      first('.js-unfold-all').click
+      expect(find('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"] .text-file')).to have_content('.rbc')
+    end
+
+    it 'unfolds diffs downwards' do
+      first('.js-unfold-down').click
+      expect(find('.file-holder[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"] .text-file')).to have_content('.popen3')
+    end
+
+    it 'unfolds diffs to the end' do
+      page.all('.js-unfold-down').last
+      expect(find('.file-holder[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9"] .text-file')).to have_content('end')
+    end
   end
 
   it 'shows diffs' do

spec/javascripts/diffs/components/diff_expansion_cell_spec.js

+import Vue from 'vue';
+import store from '~/mr_notes/stores';
+import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue';
+import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import diffFileMockData from '../mock_data/diff_file';
+
+const EXPAND_UP_CLASS = '.js-unfold';
+const EXPAND_DOWN_CLASS = '.js-unfold-down';
+const EXPAND_ALL_CLASS = '.js-unfold-all';
+
+describe('DiffExpansionCell', () => {
+  const matchLine = diffFileMockData.highlighted_diff_lines[5];
+
+  const createComponent = (options = {}) => {
+    const cmp = Vue.extend(DiffExpansionCell);
+    const defaults = {
+      fileHash: diffFileMockData.file_hash,
+      contextLinesPath: 'contextLinesPath',
+      line: matchLine,
+      isTop: false,
+      isBottom: false,
+    };
+    const props = Object.assign({}, defaults, options);
+
+    return createComponentWithStore(cmp, store, props).$mount();
+  };
+
+  describe('top row', () => {
+    it('should have "expand up" and "show all" option', () => {
+      const vm = createComponent({
+        isTop: true,
+      });
+      const el = vm.$el;
+
+      expect(el.querySelector(EXPAND_UP_CLASS)).not.toBe(null);
+      expect(el.querySelector(EXPAND_DOWN_CLASS)).toBe(null);
+      expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
+    });
+  });
+
+  describe('middle row', () => {
+    it('should have "expand down", "show all", "expand up" option', () => {
+      const vm = createComponent();
+      const el = vm.$el;
+
+      expect(el.querySelector(EXPAND_UP_CLASS)).not.toBe(null);
+      expect(el.querySelector(EXPAND_DOWN_CLASS)).not.toBe(null);
+      expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
+    });
+  });
+
+  describe('bottom row', () => {
+    it('should have "expand down" and "show all" option', () => {
+      const vm = createComponent({
+        isBottom: true,
+      });
+      const el = vm.$el;
+
+      expect(el.querySelector(EXPAND_UP_CLASS)).toBe(null);
+      expect(el.querySelector(EXPAND_DOWN_CLASS)).not.toBe(null);
+      expect(el.querySelector(EXPAND_ALL_CLASS)).not.toBe(null);
+    });
+  });
+});

spec/javascripts/diffs/components/diff_line_gutter_content_spec.js

@@ -70,15 +70,6 @@ describe('DiffLineGutterContent', () => {
   });
 
   describe('template', () => {
-    it('should render three dots for context lines', () => {
-      const component = createComponent({
-        isMatchLine: true,
-      });
-
-      expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true);
-      expect(component.$el.innerText).toEqual('...');
-    });
-
     it('should render comment button', () => {
       const component = createComponent({
         showCommentButton: true,

spec/javascripts/diffs/components/inline_diff_expansion_row_spec.js

+import Vue from 'vue';
+import store from '~/mr_notes/stores';
+import InlineDiffExpansionRow from '~/diffs/components/inline_diff_expansion_row.vue';
+import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import diffFileMockData from '../mock_data/diff_file';
+
+describe('InlineDiffExpansionRow', () => {
+  const matchLine = diffFileMockData.highlighted_diff_lines[5];
+
+  const createComponent = (options = {}) => {
+    const cmp = Vue.extend(InlineDiffExpansionRow);
+    const defaults = {
+      fileHash: diffFileMockData.file_hash,
+      contextLinesPath: 'contextLinesPath',
+      line: matchLine,
+      isTop: false,
+      isBottom: false,
+    };
+    const props = Object.assign({}, defaults, options);
+
+    return createComponentWithStore(cmp, store, props).$mount();
+  };
+
+  describe('template', () => {
+    it('should render expansion row for match lines', () => {
+      const vm = createComponent();
+
+      expect(vm.$el.classList.contains('line_expansion')).toBe(true);
+    });
+  });
+});

spec/javascripts/diffs/components/inline_diff_view_spec.js

@@ -28,9 +28,9 @@ describe('InlineDiffView', () => {
     it('should have rendered diff lines', () => {
       const el = component.$el;
 
-      expect(el.querySelectorAll('tr.line_holder').length).toEqual(6);
+      expect(el.querySelectorAll('tr.line_holder').length).toEqual(5);
       expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(2);
-      expect(el.querySelectorAll('tr.line_holder.match').length).toEqual(1);
+      expect(el.querySelectorAll('tr.line_expansion.match').length).toEqual(1);
       expect(el.textContent.indexOf('Bad dates')).toBeGreaterThan(-1);
     });
 

spec/javascripts/diffs/components/parallel_diff_expansion_row_spec.js

+import Vue from 'vue';
+import store from '~/mr_notes/stores';
+import ParallelDiffExpansionRow from '~/diffs/components/parallel_diff_expansion_row.vue';
+import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import diffFileMockData from '../mock_data/diff_file';
+
+describe('ParallelDiffExpansionRow', () => {
+  const matchLine = diffFileMockData.highlighted_diff_lines[5];
+
+  const createComponent = (options = {}) => {
+    const cmp = Vue.extend(ParallelDiffExpansionRow);
+    const defaults = {
+      fileHash: diffFileMockData.file_hash,
+      contextLinesPath: 'contextLinesPath',
+      line: matchLine,
+      isTop: false,
+      isBottom: false,
+    };
+    const props = Object.assign({}, defaults, options);
+
+    return createComponentWithStore(cmp, store, props).$mount();
+  };
+
+  describe('template', () => {
+    it('should render expansion row for match lines', () => {
+      const vm = createComponent();
+
+      expect(vm.$el.classList.contains('line_expansion')).toBe(true);
+    });
+  });
+});

spec/javascripts/diffs/store/actions_spec.js

@@ -380,7 +380,9 @@ describe('DiffsStoreActions', () => {
       const params = { since: 6, to: 26 };
       const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 };
       const fileHash = 'ff9200';
-      const options = { endpoint, params, lineNumbers, fileHash };
+      const isExpandDown = false;
+      const nextLineNumbers = {};
+      const options = { endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers };
       const mock = new MockAdapter(axios);
       const contextLines = { contextLines: [{ lineCode: 6 }] };
       mock.onGet(endpoint).reply(200, contextLines);
@@ -392,7 +394,7 @@ describe('DiffsStoreActions', () => {
         [
           {
             type: types.ADD_CONTEXT_LINES,
-            payload: { lineNumbers, contextLines, params, fileHash },
+            payload: { lineNumbers, contextLines, params, fileHash, isExpandDown, nextLineNumbers },
           },
         ],
         [],

spec/javascripts/diffs/store/mutations_spec.js

@@ -81,6 +81,8 @@ describe('DiffsStoreMutations', () => {
         params: {
           bottom: true,
         },
+        isExpandDown: false,
+        nextLineNumbers: {},
       };
       const diffFile = {
         file_hash: options.fileHash,
@@ -108,6 +110,8 @@ describe('DiffsStoreMutations', () => {
         options.contextLines,
         options.lineNumbers,
         options.params.bottom,
+        options.isExpandDown,
+        options.nextLineNumbers,
       );
 
       expect(addContextLinesSpy).toHaveBeenCalledWith({
@@ -116,6 +120,7 @@ describe('DiffsStoreMutations', () => {
         contextLines: options.contextLines,
         bottom: options.params.bottom,
         lineNumbers: options.lineNumbers,
+        isExpandDown: false,
       });
     });
   });

spec/javascripts/diffs/store/utils_spec.js

@@ -260,6 +260,17 @@ describe('DiffsStoreUtils', () => {
       expect(linesWithReferences[1].meta_data.old_pos).toEqual(2);
       expect(linesWithReferences[1].meta_data.new_pos).toEqual(3);
     });
+
+    it('should add correct line references when isExpandDown is true', () => {
+      const lines = [{ type: null }, { type: MATCH_LINE_TYPE }];
+      const linesWithReferences = utils.addLineReferences(lines, lineNumbers, false, true, {
+        old_line: 10,
+        new_line: 11,
+      });
+
+      expect(linesWithReferences[1].meta_data.old_pos).toEqual(10);
+      expect(linesWithReferences[1].meta_data.new_pos).toEqual(11);
+    });
   });
 
   describe('trimFirstCharOfLineContent', () => {

spec/presenters/blobs/unfold_presenter_spec.rb

@@ -39,6 +39,21 @@ describe Blobs::UnfoldPresenter do
         expect(result.indent).to eq(0)
       end
     end
+
+    context 'when to is -1' do
+      let(:params) { { full: false, since: 2, to: -1, bottom: true, offset: 1, indent: 1 } }
+
+      it 'sets other attributes' do
+        result = subject
+
+        expect(result.full?).to eq(false)
+        expect(result.since).to eq(2)
+        expect(result.to).to eq(blob.lines.size)
+        expect(result.bottom).to eq(false)
+        expect(result.offset).to eq(0)
+        expect(result.indent).to eq(0)
+      end
+    end
   end
 
   describe '#diff_lines' do
@@ -83,8 +98,9 @@ describe Blobs::UnfoldPresenter do
       end
     end
 
-    context 'when since is greater than 1' do
-      let(:params) { { since: 5, to: 10, offset: 10 } }
+    context 'when "since" is greater than 1' do
+      let(:default_params) { { since: 5, to: 10, offset: 10 } }
+      let(:params) { default_params }
 
       it 'adds top match line' do
         line = subject.diff_lines.first
@@ -93,6 +109,38 @@ describe Blobs::UnfoldPresenter do
         expect(line.old_pos).to eq(5)
         expect(line.new_pos).to eq(5)
       end
+
+      context '"to" is higher than blob size' do
+        let(:params) { default_params.merge(to: total_lines + 10, bottom: true) }
+
+        it 'does not add bottom match line' do
+          line = subject.diff_lines.last
+
+          expect(line.type).to be_nil
+        end
+      end
+
+      context '"to" is equal to blob size' do
+        let(:params) { default_params.merge(to: total_lines, bottom: true) }
+
+        it 'does not add bottom match line' do
+          line = subject.diff_lines.last
+
+          expect(line.type).to be_nil
+        end
+      end
+
+      context '"to" is less than blob size' do
+        let(:params) { default_params.merge(to: total_lines - 3, bottom: true) }
+
+        it 'adds bottom match line' do
+          line = subject.diff_lines.last
+
+          expect(line.type).to eq('match')
+          expect(line.old_pos).to eq(total_lines - 3 - params[:offset])
+          expect(line.new_pos).to eq(total_lines - 3)
+        end
+      end
     end
 
     context 'when "to" is less than blob size' do
@@ -116,6 +164,22 @@ describe Blobs::UnfoldPresenter do
         expect(line.type).to be_nil
       end
     end
+
+    context 'when "to" is "-1"' do
+      let(:params) { { since: 10, to: -1, offset: 10, bottom: true } }
+
+      it 'does not add bottom match line' do
+        line = subject.diff_lines.last
+
+        expect(line.type).to be_nil
+      end
+
+      it 'last line is the latest blob line' do
+        line = subject.diff_lines.last
+
+        expect(line.text).to eq(total_lines.to_s)
+      end
+    end
   end
 
   describe '#lines' do