From b0752217a425dfb1f6836a09783e3cd43ebe190c Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 13:09:07 -0600
Subject: [PATCH 01/13] Add `manuallyCollapsed` flag to Diff Files

Of course, this means that there's a bunch of logic
to coordinate the new flag with the original -
`automaticallyCollapsed` - flag.

In addition: the UI is updated to use the new flag
whenever it's been set, but to continue to fall back
to the old flag. This is primarily facilitated by a new
helper utility for diff files to do the resolution in a
single centralized place.

Of note: the diff_file is updated to depend on the
global store as the authoritative source of determining
whether the file is collapsed or not. The previous
version kept a local `isCollapsed` in sync with the
global store, which introduced a huge amount of
interwoven complexity. The update ingests the
resolved collapsed state from the store when
various observed properties update instead of
inverting that flow.

This is a full refactor of file collapsing.
Because the previous version was a single flag, then
modified with a user preference (file-by-file), there
were some issues including a THIRD possible way
that collapsing works. This update slightly simplifies
how collapsing works by depending more heavily
on the two flags stored in state and only modifying
them with the user preference when necessary.
---
 .../javascripts/diffs/components/app.vue      | 11 +--
 .../diffs/components/compare_versions.vue     |  2 +-
 .../diffs/components/diff_file.vue            | 95 ++++++++++++-------
 .../diffs/components/diff_file_header.vue     |  6 +-
 app/assets/javascripts/diffs/constants.js     |  4 +
 app/assets/javascripts/diffs/diff_file.js     | 24 ++++-
 app/assets/javascripts/diffs/store/actions.js | 18 +++-
 app/assets/javascripts/diffs/store/getters.js | 28 +++++-
 .../javascripts/diffs/store/mutations.js      | 34 +++++--
 .../notes/components/diff_with_note.vue       |  6 +-
 10 files changed, 167 insertions(+), 61 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 085f951147f8ca..7895407cef0541 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -124,7 +124,6 @@ export default {
     return {
       treeWidth,
       diffFilesLength: 0,
-      collapsedWarningDismissed: false,
     };
   },
   computed: {
@@ -206,11 +205,7 @@ export default {
         visible = this.$options.alerts.ALERT_OVERFLOW_HIDDEN;
       } else if (this.isDiffHead && this.hasConflicts) {
         visible = this.$options.alerts.ALERT_MERGE_CONFLICT;
-      } else if (
-        this.hasCollapsedFile &&
-        !this.collapsedWarningDismissed &&
-        !this.viewDiffsFileByFile
-      ) {
+      } else if (this.hasCollapsedFile.automatic && !this.viewDiffsFileByFile) {
         visible = this.$options.alerts.ALERT_COLLAPSED_FILES;
       }
 
@@ -429,9 +424,6 @@ export default {
         this.toggleShowTreeList(false);
       }
     },
-    dismissCollapsedWarning() {
-      this.collapsedWarningDismissed = true;
-    },
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,
@@ -464,7 +456,6 @@ export default {
       <collapsed-files-warning
         v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
         :limited="isLimitedContainer"
-        @dismiss="dismissCollapsedWarning"
       />
 
       <div
diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue
index b1ebd8e6ebc602..124a15a498a634 100644
--- a/app/assets/javascripts/diffs/components/compare_versions.vue
+++ b/app/assets/javascripts/diffs/components/compare_versions.vue
@@ -129,7 +129,7 @@ export default {
           {{ __('Show latest version') }}
         </gl-button>
         <gl-button
-          v-show="hasCollapsedFile"
+          v-show="hasCollapsedFile.any"
           variant="default"
           class="gl-mr-3"
           @click="expandAllFiles"
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 529723a349d6df..4a1860d6ad82e9 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -10,6 +10,8 @@ import eventHub from '../../notes/event_hub';
 import DiffFileHeader from './diff_file_header.vue';
 import DiffContent from './diff_content.vue';
 import { diffViewerErrors } from '~/ide/constants';
+import { collapsedType, isCollapsed } from '../diff_file';
+import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '../constants';
 
 export default {
   components: {
@@ -44,7 +46,7 @@ export default {
     return {
       isLoadingCollapsedDiff: false,
       forkMessageVisible: false,
-      isCollapsed: this.file.viewer.automaticallyCollapsed || false,
+      isCollapsed: false,
     };
   },
   computed: {
@@ -71,7 +73,7 @@ export default {
       return this.file.viewer.error === diffViewerErrors.too_large;
     },
     errorMessage() {
-      return this.file.viewer.error_message;
+      return !this.manualCollapse ? this.file.viewer.error_message : '';
     },
     forkMessage() {
       return sprintf(
@@ -85,57 +87,78 @@ export default {
         false,
       );
     },
+    automaticCollapse() {
+      return collapsedType(this.file) === DIFF_FILE_AUTOMATIC_COLLAPSE;
+    },
+    manualCollapse() {
+      return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE;
+    },
+    showBody() {
+      return !this.isCollapsed || this.automaticCollapse;
+    },
+    showWarning() {
+      return this.isCollapsed && (this.automaticCollapse && !this.viewDiffsFileByFile);
+    },
+    showContent() {
+      return !this.isCollapsed && !this.isFileTooLarge;
+    },
   },
   watch: {
-    isCollapsed: function fileCollapsedWatch(newVal, oldVal) {
-      if (!newVal && oldVal && !this.hasDiff) {
-        this.handleLoadCollapsedDiff();
-      }
+    'file.file_hash': {
+      handler: function hashChangeWatch(newHash, oldHash) {
+        this.isCollapsed = isCollapsed(this.file);
 
-      this.setFileCollapsed({ filePath: this.file.file_path, collapsed: newVal });
+        if (newHash && oldHash && !this.hasDiff) {
+          this.requestDiffLoad();
+        }
+      },
+      immediate: true,
     },
-    'file.file_hash': {
-      handler: function watchFileHash() {
-        if (this.viewDiffsFileByFile && this.file.viewer.automaticallyCollapsed) {
-          this.isCollapsed = false;
-          this.handleLoadCollapsedDiff();
-        } else {
-          this.isCollapsed = this.file.viewer.automaticallyCollapsed || false;
+    'file.viewer.automaticallyCollapsed': {
+      handler: function autoChangeWatch(automaticValue) {
+        if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) {
+          this.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
         }
       },
       immediate: true,
     },
-    'file.viewer.automaticallyCollapsed': function setIsCollapsed(newVal) {
-      if (!this.viewDiffsFileByFile) {
-        this.isCollapsed = newVal;
-      }
+    'file.viewer.manuallyCollapsed': {
+      handler: function manualChangeWatch(manualValue) {
+        if (manualValue !== null) {
+          this.isCollapsed = manualValue;
+        }
+      },
+      immediate: true,
     },
   },
   created() {
-    eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.handleLoadCollapsedDiff);
+    eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiffLoad);
   },
   methods: {
     ...mapActions('diffs', [
       'loadCollapsedDiff',
       'assignDiscussionsToDiff',
       'setRenderIt',
-      'setFileCollapsed',
+      'setFileCollapsedByUser',
     ]),
     handleToggle() {
-      if (!this.hasDiff) {
-        this.handleLoadCollapsedDiff();
-      } else {
-        this.isCollapsed = !this.isCollapsed;
-        this.setRenderIt(this.file);
+      const currentCollapsedFlag = this.isCollapsed;
+
+      this.setFileCollapsedByUser({
+        filePath: this.file.file_path,
+        collapsed: !currentCollapsedFlag,
+      });
+
+      if (!this.hasDiff && currentCollapsedFlag) {
+        this.requestDiffLoad();
       }
     },
-    handleLoadCollapsedDiff() {
+    requestDiffLoad() {
       this.isLoadingCollapsedDiff = true;
 
       this.loadCollapsedDiff(this.file)
         .then(() => {
           this.isLoadingCollapsedDiff = false;
-          this.isCollapsed = false;
           this.setRenderIt(this.file);
         })
         .then(() => {
@@ -167,6 +190,7 @@ export default {
     :class="{
       'is-active': currentDiffFileId === file.file_hash,
       'comments-disabled': Boolean(file.brokenSymlink),
+      'has-body': showBody,
     }"
     :data-path="file.new_path"
     class="diff-file file-holder"
@@ -200,22 +224,23 @@ export default {
     </div>
     <gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" />
     <template v-else>
-      <div :id="`diff-content-${file.file_hash}`">
-        <div v-if="errorMessage" class="diff-viewer">
+      <div :id="`diff-content-${file.file_hash}`" data-testid="content-area">
+        <gl-loading-icon
+          v-if="showLoadingIcon"
+          class="diff-content loading gl-my-0 gl-pt-3"
+          data-testid="loader-icon"
+        />
+        <div v-else-if="errorMessage" class="diff-viewer">
           <div v-safe-html="errorMessage" class="nothing-here-block"></div>
         </div>
         <template v-else>
-          <div v-show="isCollapsed" class="nothing-here-block diff-collapsed">
+          <div v-show="showWarning" class="nothing-here-block diff-collapsed">
             {{ __('This diff is collapsed.') }}
             <a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">{{
               __('Click to expand it.')
             }}</a>
           </div>
-          <diff-content
-            v-show="!isCollapsed && !isFileTooLarge"
-            :diff-file="file"
-            :help-page-path="helpPagePath"
-          />
+          <diff-content v-show="showContent" :diff-file="file" :help-page-path="helpPagePath" />
         </template>
       </div>
     </template>
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue
index b08b9df13a43d8..ea7112689c1440 100644
--- a/app/assets/javascripts/diffs/components/diff_file_header.vue
+++ b/app/assets/javascripts/diffs/components/diff_file_header.vue
@@ -18,6 +18,7 @@ import { __, s__, sprintf } from '~/locale';
 import { diffViewerModes } from '~/ide/constants';
 import DiffStats from './diff_stats.vue';
 import { scrollToElement } from '~/lib/utils/common_utils';
+import { isCollapsed } from '../diff_file';
 import { DIFF_FILE_HEADER } from '../i18n';
 
 export default {
@@ -125,6 +126,9 @@ export default {
     isUsingLfs() {
       return this.diffFile.stored_externally && this.diffFile.external_storage === 'lfs';
     },
+    isCollapsed() {
+      return isCollapsed(this.diffFile, { fileByFile: this.viewDiffsFileByFile });
+    },
     collapseIcon() {
       return this.expanded ? 'chevron-down' : 'chevron-right';
     },
@@ -334,7 +338,7 @@ export default {
             </gl-dropdown-item>
           </template>
 
-          <template v-if="!diffFile.viewer.automaticallyCollapsed">
+          <template v-if="!isCollapsed">
             <gl-dropdown-divider
               v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)"
             />
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index dc97d9993da21b..0735470bfa2e27 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -73,6 +73,10 @@ export const ALERT_OVERFLOW_HIDDEN = 'overflow';
 export const ALERT_MERGE_CONFLICT = 'merge-conflict';
 export const ALERT_COLLAPSED_FILES = 'collapsed';
 
+// Diff File collapse types
+export const DIFF_FILE_AUTOMATIC_COLLAPSE = 'automatic';
+export const DIFF_FILE_MANUAL_COLLAPSE = 'manual';
+
 // State machine states
 export const STATE_IDLING = 'idle';
 export const STATE_LOADING = 'loading';
diff --git a/app/assets/javascripts/diffs/diff_file.js b/app/assets/javascripts/diffs/diff_file.js
index 933197a2c7f9f9..a14a30b41a9cbe 100644
--- a/app/assets/javascripts/diffs/diff_file.js
+++ b/app/assets/javascripts/diffs/diff_file.js
@@ -1,4 +1,9 @@
-import { DIFF_FILE_SYMLINK_MODE, DIFF_FILE_DELETED_MODE } from './constants';
+import {
+  DIFF_FILE_SYMLINK_MODE,
+  DIFF_FILE_DELETED_MODE,
+  DIFF_FILE_MANUAL_COLLAPSE,
+  DIFF_FILE_AUTOMATIC_COLLAPSE,
+} from './constants';
 
 function fileSymlinkInformation(file, fileList) {
   const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash);
@@ -23,6 +28,7 @@ function collapsed(file) {
 
   return {
     automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false,
+    manuallyCollapsed: null,
   };
 }
 
@@ -37,3 +43,19 @@ export function prepareRawDiffFile({ file, allFiles }) {
 
   return file;
 }
+
+export function collapsedType(file) {
+  const isManual = typeof file.viewer?.manuallyCollapsed === 'boolean';
+
+  return isManual ? DIFF_FILE_MANUAL_COLLAPSE : DIFF_FILE_AUTOMATIC_COLLAPSE;
+}
+
+export function isCollapsed(file) {
+  const type = collapsedType(file);
+  const collapsedStates = {
+    [DIFF_FILE_AUTOMATIC_COLLAPSE]: file.viewer?.automaticallyCollapsed || false,
+    [DIFF_FILE_MANUAL_COLLAPSE]: file.viewer?.manuallyCollapsed,
+  };
+
+  return collapsedStates[type];
+}
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 966b706fc31cc5..322051cff05cbf 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -40,8 +40,11 @@ import {
   DIFF_WHITESPACE_COOKIE_NAME,
   SHOW_WHITESPACE,
   NO_SHOW_WHITESPACE,
+  DIFF_FILE_MANUAL_COLLAPSE,
+  DIFF_FILE_AUTOMATIC_COLLAPSE,
 } from '../constants';
 import { diffViewerModes } from '~/ide/constants';
+import { isCollapsed } from '../diff_file';
 
 export const setBaseConfig = ({ commit }, options) => {
   const {
@@ -239,6 +242,13 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
       if (file.viewer.automaticallyCollapsed) {
         eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`);
         scrollToElement(document.getElementById(file.file_hash));
+      } else if (file.viewer.manuallyCollapsed) {
+        commit(types.SET_FILE_COLLAPSED, {
+          filePath: file.file_path,
+          collapsed: false,
+          trigger: DIFF_FILE_AUTOMATIC_COLLAPSE,
+        });
+        eventHub.$emit('scrollToDiscussion');
       } else {
         eventHub.$emit('scrollToDiscussion');
       }
@@ -252,8 +262,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
       const nextFile = state.diffFiles.find(
         file =>
           !file.renderIt &&
-          (file.viewer &&
-            (!file.viewer.automaticallyCollapsed || file.viewer.name !== diffViewerModes.text)),
+          (file.viewer && (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text)),
       );
 
       if (nextFile) {
@@ -641,8 +650,9 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d
     });
 }
 
-export const setFileCollapsed = ({ commit }, { filePath, collapsed }) =>
-  commit(types.SET_FILE_COLLAPSED, { filePath, collapsed });
+export const setFileCollapsedByUser = ({ commit }, { filePath, collapsed }) => {
+  commit(types.SET_FILE_COLLAPSED, { filePath, collapsed, trigger: DIFF_FILE_MANUAL_COLLAPSE });
+};
 
 export const setSuggestPopoverDismissed = ({ commit, state }) =>
   axios
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 91425c7825bb89..38ae3ad503e757 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -8,8 +8,32 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW
 
 export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE;
 
-export const hasCollapsedFile = state =>
-  state.diffFiles.some(file => file.viewer && file.viewer.automaticallyCollapsed);
+export const hasCollapsedFile = state => {
+  const automatic = state.diffFiles.some(file => file.viewer?.automaticallyCollapsed);
+  const manual = state.diffFiles.some(file => file.viewer?.manuallyCollapsed);
+
+  return {
+    any: automatic || manual,
+    automatic,
+    manual,
+  };
+};
+export const collapsedFilesCount = state => {
+  const counts = {
+    automatic: 0,
+    manual: 0,
+  };
+
+  state.diffFiles.forEach(file => {
+    if (file.viewer?.manuallyCollapsed) {
+      counts.manual += 1;
+    } else if (file.viewer?.automaticallyCollapsed) {
+      counts.automatic += 1;
+    }
+  });
+
+  return counts;
+};
 
 export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
 
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 13ecf6a997d884..ca4d7b0461be63 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,6 +1,10 @@
 import Vue from 'vue';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { INLINE_DIFF_VIEW_TYPE } from '../constants';
+import {
+  DIFF_FILE_MANUAL_COLLAPSE,
+  DIFF_FILE_AUTOMATIC_COLLAPSE,
+  INLINE_DIFF_VIEW_TYPE,
+} from '../constants';
 import {
   findDiffFile,
   addLineReferences,
@@ -16,6 +20,12 @@ function updateDiffFilesInState(state, files) {
   return Object.assign(state, { diffFiles: files });
 }
 
+function renderFile(file) {
+  return Object.assign(file, {
+    renderIt: true,
+  });
+}
+
 export default {
   [types.SET_BASE_CONFIG](state, options) {
     const {
@@ -81,9 +91,7 @@ export default {
   },
 
   [types.RENDER_FILE](state, file) {
-    Object.assign(file, {
-      renderIt: true,
-    });
+    renderFile(file);
   },
 
   [types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) {
@@ -173,6 +181,7 @@ export default {
       Object.assign(file, {
         viewer: Object.assign(file.viewer, {
           automaticallyCollapsed: false,
+          manuallyCollapsed: false,
         }),
       });
     });
@@ -351,11 +360,24 @@ export default {
     file.isShowingFullFile = true;
     file.isLoadingFullFile = false;
   },
-  [types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) {
+  [types.SET_FILE_COLLAPSED](
+    state,
+    { filePath, collapsed, trigger = DIFF_FILE_AUTOMATIC_COLLAPSE },
+  ) {
     const file = state.diffFiles.find(f => f.file_path === filePath);
 
     if (file && file.viewer) {
-      file.viewer.automaticallyCollapsed = collapsed;
+      if (trigger === DIFF_FILE_MANUAL_COLLAPSE) {
+        file.viewer.automaticallyCollapsed = false;
+        file.viewer.manuallyCollapsed = collapsed;
+      } else if (trigger === DIFF_FILE_AUTOMATIC_COLLAPSE) {
+        file.viewer.automaticallyCollapsed = collapsed;
+        file.viewer.manuallyCollapsed = null;
+      }
+    }
+
+    if (file && !collapsed) {
+      renderFile(file);
     }
   },
   [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue
index a4271852563423..91cf682943e29f 100644
--- a/app/assets/javascripts/notes/components/diff_with_note.vue
+++ b/app/assets/javascripts/notes/components/diff_with_note.vue
@@ -7,6 +7,7 @@ import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
 import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
 import { getDiffMode } from '~/diffs/store/utils';
 import { diffViewerModes } from '~/ide/constants';
+import { isCollapsed } from '../../diffs/diff_file';
 
 const FIRST_CHAR_REGEX = /^(\+|-| )/;
 
@@ -46,6 +47,9 @@ export default {
         this.discussion.truncated_diff_lines && this.discussion.truncated_diff_lines.length !== 0
       );
     },
+    isCollapsed() {
+      return isCollapsed(this.discussion.diff_file);
+    },
   },
   mounted() {
     if (this.isTextFile && !this.hasTruncatedDiffLines) {
@@ -76,7 +80,7 @@ export default {
       :discussion-path="discussion.discussion_path"
       :diff-file="discussion.diff_file"
       :can-current-user-fork="false"
-      :expanded="!discussion.diff_file.viewer.automaticallyCollapsed"
+      :expanded="!isCollapsed"
     />
     <div v-if="isTextFile" class="diff-content">
       <table class="code js-syntax-highlight" :class="$options.userColorSchemeClass">
-- 
GitLab


From 7c975f4c2e0ade75ad6921cddd542eeeb0aec53e Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Tue, 20 Oct 2020 16:52:57 -0600
Subject: [PATCH 02/13] Fix styles for diff files when either open or closed

---
 .../diffs/components/diff_file.vue            |  5 +--
 app/assets/stylesheets/framework/diffs.scss   | 40 ++++++++++++++++---
 .../page_bundles/merge_requests.scss          | 12 +++++-
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 4a1860d6ad82e9..3628e808908d68 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -193,7 +193,7 @@ export default {
       'has-body': showBody,
     }"
     :data-path="file.new_path"
-    class="diff-file file-holder"
+    class="diff-file file-holder gl-border-none"
   >
     <diff-file-header
       :can-current-user-fork="canCurrentUserFork"
@@ -202,7 +202,7 @@ export default {
       :expanded="!isCollapsed"
       :add-merge-request-buttons="true"
       :view-diffs-file-by-file="viewDiffsFileByFile"
-      class="js-file-title file-title"
+      class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100"
       @toggleFile="handleToggle"
       @showForkMessage="showForkMessage"
     />
@@ -222,7 +222,6 @@ export default {
         {{ __('Cancel') }}
       </button>
     </div>
-    <gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" />
     <template v-else>
       <div :id="`diff-content-${file.file_hash}`" data-testid="content-area">
         <gl-loading-icon
diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss
index c0a2350d080742..6c22202f67f59b 100644
--- a/app/assets/stylesheets/framework/diffs.scss
+++ b/app/assets/stylesheets/framework/diffs.scss
@@ -6,11 +6,37 @@
     border-top: 1px solid $border-color;
   }
 
+  &.has-body {
+    .file-title {
+      @include gl-rounded-bottom-left-none();
+      @include gl-rounded-bottom-right-none();
+
+      box-shadow: 0 -2px 0 0 var(--white);
+    }
+
+    .diff-content {
+      @include gl-rounded-bottom-left-base();
+      @include gl-rounded-bottom-right-base();
+    }
+
+    [id^='diff-content-'] {
+      @include gl-rounded-0();
+      @include gl-rounded-bottom-left-base();
+      @include gl-rounded-bottom-right-base();
+      @include gl-border-1();
+      @include gl-border-t-0();
+      @include gl-border-solid();
+      @include gl-border-gray-100();
+    }
+  }
+
+  table.code tr:last-of-type td:last-of-type {
+    @include gl-rounded-bottom-right-base();
+  }
+
   .file-title,
   .file-title-flex-parent {
-    border-top-left-radius: $border-radius-default;
-    border-top-right-radius: $border-radius-default;
-    box-shadow: 0 -2px 0 0 var(--white);
+    @include gl-rounded-base();
     cursor: pointer;
 
     .dropdown-menu {
@@ -113,7 +139,6 @@
   .diff-content {
     background: $white;
     color: $gl-text-color;
-    border-radius: 0 0 3px 3px;
 
     .unfold {
       cursor: pointer;
@@ -457,8 +482,11 @@ table.code {
     border-top: 0;
   }
 
-  tr:nth-last-of-type(2).line_expansion > td {
-    border-bottom: 0;
+  tr:nth-last-of-type(2).line_expansion,
+  tr:last-of-type.line_expansion {
+    > td {
+      border-bottom: 0;
+    }
   }
 
   tr.line_holder td {
diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss
index 5553dffac05631..be74503c21f203 100644
--- a/app/assets/stylesheets/page_bundles/merge_requests.scss
+++ b/app/assets/stylesheets/page_bundles/merge_requests.scss
@@ -11,9 +11,19 @@
 }
 
 .diff-tree-list {
+  // This 11px value should match the additional value found in
+  //      /assets/stylesheets/framework/diffs.scss
+  // for the $mr-file-header-top SCSS variable within the
+  //      .file-title,
+  //      .file-title-flex-parent {
+  // rule.
+  // If they don't match, the file tree and the diff files stick
+  // to the top at different heights, which is a bad-looking defect
+  $diff-file-header-top: 11px;
+  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + $diff-file-header-top;
+
   position: -webkit-sticky;
   position: sticky;
-  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 15px;
   top: $top-pos;
   max-height: calc(100vh - #{$top-pos});
   z-index: 202;
-- 
GitLab


From edc7d5ef3308a1c083b1f5e58e901152e17b7c44 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 21 Oct 2020 10:11:27 -0600
Subject: [PATCH 03/13] Update tests for new manual collapse flag

Other notable changes:

- Moves tests that were testing internals of
other components into the tests for those
components

- Abstracts tests for internals of other
components to more "simply" testing for
the wrapping component
---
 .../diffs/components/diff_file.vue            |  11 +-
 spec/frontend/diffs/components/app_spec.js    |  14 +-
 .../diffs/components/diff_file_header_spec.js |  25 +-
 .../diffs/components/diff_file_spec.js        | 435 ++++++++++--------
 spec/frontend/diffs/mock_data/diff_file.js    |   1 +
 .../diffs/mock_data/diff_file_unreadable.js   |   1 +
 spec/frontend/diffs/store/actions_spec.js     |  13 +-
 spec/frontend/diffs/store/getters_spec.js     |  42 +-
 8 files changed, 318 insertions(+), 224 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 3628e808908d68..5824871faf4826 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -235,9 +235,14 @@ export default {
         <template v-else>
           <div v-show="showWarning" class="nothing-here-block diff-collapsed">
             {{ __('This diff is collapsed.') }}
-            <a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">{{
-              __('Click to expand it.')
-            }}</a>
+            <a
+              class="click-to-expand"
+              data-testid="toggle-link"
+              href="#"
+              @click.prevent="handleToggle"
+            >
+              {{ __('Click to expand it.') }}
+            </a>
           </div>
           <diff-content v-show="showContent" :diff-file="file" :help-page-path="helpPagePath" />
         </template>
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 86560470ada6f0..225710eab63389 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/spec/frontend/diffs/components/app_spec.js
@@ -697,7 +697,7 @@ describe('diffs/components/app', () => {
       });
 
       describe('collapsed files', () => {
-        it('should render the collapsed files warning if there are any collapsed files', () => {
+        it('should render the collapsed files warning if there are any automatically collapsed files', () => {
           createComponent({}, ({ state }) => {
             state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
           });
@@ -705,16 +705,14 @@ describe('diffs/components/app', () => {
           expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
         });
 
-        it('should not render the collapsed files warning if the user has dismissed the alert already', async () => {
+        it('should not render the collapsed files warning if there are no automatically collapsed files', () => {
           createComponent({}, ({ state }) => {
-            state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
+            state.diffs.diffFiles = [
+              { viewer: { automaticallyCollapsed: false, manuallyCollapsed: true } },
+              { viewer: { automaticallyCollapsed: false, manuallyCollapsed: false } },
+            ];
           });
 
-          expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
-
-          wrapper.vm.collapsedWarningDismissed = true;
-          await wrapper.vm.$nextTick();
-
           expect(getCollapsedFilesWarning(wrapper).exists()).toBe(false);
         });
       });
diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js
index a04486fc5c73ab..4db46b1029370c 100644
--- a/spec/frontend/diffs/components/diff_file_header_spec.js
+++ b/spec/frontend/diffs/components/diff_file_header_spec.js
@@ -1,6 +1,9 @@
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import { cloneDeep } from 'lodash';
+
+import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
+
 import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import diffDiscussionsMockData from '../mock_data/diff_discussions';
@@ -136,9 +139,25 @@ describe('DiffFileHeader component', () => {
     });
   });
 
-  it('displays a copy to clipboard button', () => {
-    createComponent();
-    expect(wrapper.find(ClipboardButton).exists()).toBe(true);
+  describe('copy to clipboard', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('displays a copy to clipboard button', () => {
+      expect(wrapper.find(ClipboardButton).exists()).toBe(true);
+    });
+
+    it('triggers the copy to clipboard tracking event', () => {
+      const trackingSpy = mockTracking('_category_', wrapper.vm.$el, jest.spyOn);
+
+      triggerEvent('[data-testid="diff-file-copy-clipboard"]');
+
+      expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_copy_file_button', {
+        label: 'diff_copy_file_path_button',
+        property: 'diff_copy_file',
+      });
+    });
   });
 
   describe('for submodule', () => {
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index a6f0d2bf11d7c3..7bd9da2f673620 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -1,262 +1,317 @@
-import Vue from 'vue';
-import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
-import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
-import { createStore } from '~/mr_notes/stores';
-import DiffFileComponent from '~/diffs/components/diff_file.vue';
-import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
+import Vuex from 'vuex';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+
+import createDiffsStore from '~/diffs/store/modules';
 import diffFileMockDataReadable from '../mock_data/diff_file';
 import diffFileMockDataUnreadable from '../mock_data/diff_file_unreadable';
 
-describe('DiffFile', () => {
-  let vm;
-  let trackingSpy;
+import DiffFileComponent from '~/diffs/components/diff_file.vue';
+import DiffFileHeaderComponent from '~/diffs/components/diff_file_header.vue';
+import DiffContentComponent from '~/diffs/components/diff_content.vue';
 
-  beforeEach(() => {
-    vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
-      file: JSON.parse(JSON.stringify(diffFileMockDataReadable)),
+import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
+
+function changeViewer(store, index, { automaticallyCollapsed, manuallyCollapsed, name }) {
+  const file = store.state.diffs.diffFiles[index];
+  const newViewer = {
+    ...file.viewer,
+  };
+
+  if (automaticallyCollapsed !== undefined) {
+    newViewer.automaticallyCollapsed = automaticallyCollapsed;
+  }
+
+  if (manuallyCollapsed !== undefined) {
+    newViewer.manuallyCollapsed = manuallyCollapsed;
+  }
+
+  if (name !== undefined) {
+    newViewer.name = name;
+  }
+
+  Object.assign(file, {
+    viewer: newViewer,
+  });
+}
+
+function forceHasDiff({ store, index = 0, inlineLines, parallelLines, readableText }) {
+  const file = store.state.diffs.diffFiles[index];
+
+  Object.assign(file, {
+    highlighted_diff_lines: inlineLines,
+    parallel_diff_lines: parallelLines,
+    blob: {
+      ...file.blob,
+      readable_text: readableText,
+    },
+  });
+}
+
+function markFileToBeRendered(store, index = 0) {
+  const file = store.state.diffs.diffFiles[index];
+
+  Object.assign(file, {
+    renderIt: true,
+  });
+}
+
+function createComponent({ file }) {
+  const localVue = createLocalVue();
+
+  localVue.use(Vuex);
+
+  const store = new Vuex.Store({
+    modules: {
+      diffs: createDiffsStore(),
+    },
+  });
+
+  store.state.diffs.diffFiles = [file];
+
+  const wrapper = shallowMount(DiffFileComponent, {
+    store,
+    localVue,
+    propsData: {
+      file,
       canCurrentUserFork: false,
       viewDiffsFileByFile: false,
-    }).$mount();
-    trackingSpy = mockTracking('_category_', vm.$el, jest.spyOn);
+    },
   });
 
-  afterEach(() => {
-    vm.$destroy();
+  return {
+    localVue,
+    wrapper,
+    store,
+  };
+}
+
+const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent);
+const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]');
+const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]');
+const findToggleLinks = wrapper => wrapper.findAll('[data-testid="toggle-link"]');
+
+const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile');
+const isDisplayNone = element => element.style.display === 'none';
+const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable));
+const getUnreadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataUnreadable));
+
+const makeFileAutomaticallyCollapsed = (store, index = 0) =>
+  changeViewer(store, index, { automaticallyCollapsed: true, manuallyCollapsed: null });
+const makeFileOpenByDefault = (store, index = 0) =>
+  changeViewer(store, index, { automaticallyCollapsed: false, manuallyCollapsed: null });
+const makeFileManuallyCollapsed = (store, index = 0) =>
+  changeViewer(store, index, { automaticallyCollapsed: false, manuallyCollapsed: true });
+const changeViewerType = (store, newType, index = 0) =>
+  changeViewer(store, index, { name: diffViewerModes[newType] });
+
+describe('DiffFile', () => {
+  let wrapper;
+  let store;
+
+  beforeEach(() => {
+    ({ wrapper, store } = createComponent({ file: getReadableFile() }));
   });
 
-  const findDiffContent = () => vm.$el.querySelector('.diff-content');
-  const isVisible = el => el.style.display !== 'none';
+  afterEach(() => {
+    wrapper.destroy();
+  });
 
   describe('template', () => {
-    it('should render component with file header, file content components', done => {
-      const el = vm.$el;
-      const { file_hash, file_path } = vm.file;
+    it('should render component with file header, file content components', async () => {
+      const el = wrapper.vm.$el;
+      const { file_hash } = wrapper.vm.file;
 
       expect(el.id).toEqual(file_hash);
       expect(el.classList.contains('diff-file')).toEqual(true);
 
       expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0);
       expect(el.querySelector('.js-file-title')).toBeDefined();
-      expect(el.querySelector('[data-testid="diff-file-copy-clipboard"]')).toBeDefined();
-      expect(el.querySelector('.file-title-name').innerText.indexOf(file_path)).toBeGreaterThan(-1);
+      expect(wrapper.find(DiffFileHeaderComponent).exists()).toBe(true);
       expect(el.querySelector('.js-syntax-highlight')).toBeDefined();
 
-      vm.file.renderIt = true;
-
-      vm.$nextTick()
-        .then(() => {
-          expect(el.querySelectorAll('.line_content').length).toBe(8);
-          expect(el.querySelectorAll('.js-line-expansion-content').length).toBe(1);
-          triggerEvent('[data-testid="diff-file-copy-clipboard"]');
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should track a click event on copy to clip board button', done => {
-      const el = vm.$el;
+      markFileToBeRendered(store);
 
-      expect(el.querySelector('[data-testid="diff-file-copy-clipboard"]')).toBeDefined();
-      vm.file.renderIt = true;
-      vm.$nextTick()
-        .then(() => {
-          triggerEvent('[data-testid="diff-file-copy-clipboard"]');
+      await wrapper.vm.$nextTick();
 
-          expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_copy_file_button', {
-            label: 'diff_copy_file_path_button',
-            property: 'diff_copy_file',
-          });
-        })
-        .then(done)
-        .catch(done.fail);
+      expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
     });
+  });
 
-    describe('collapsed', () => {
-      it('should not have file content', done => {
-        expect(isVisible(findDiffContent())).toBe(true);
-        expect(vm.isCollapsed).toEqual(false);
-        vm.isCollapsed = true;
-        vm.file.renderIt = true;
+  describe('collapsing', () => {
+    describe('user collapsed', () => {
+      beforeEach(() => {
+        makeFileManuallyCollapsed(store);
+      });
 
-        vm.$nextTick(() => {
-          expect(isVisible(findDiffContent())).toBe(false);
+      it('should not have any content at all', async () => {
+        await wrapper.vm.$nextTick();
 
-          done();
+        Array.from(findDiffContentArea(wrapper).element.children).forEach(child => {
+          expect(isDisplayNone(child)).toBe(true);
         });
       });
 
-      it('should have collapsed text and link', done => {
-        vm.renderIt = true;
-        vm.isCollapsed = true;
+      it('should not have the class `has-body` to present the header differently', () => {
+        expect(wrapper.classes('has-body')).toBe(false);
+      });
+    });
 
-        vm.$nextTick(() => {
-          expect(vm.$el.innerText).toContain('This diff is collapsed');
-          expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+    describe('automatically collapsed', () => {
+      beforeEach(() => {
+        makeFileAutomaticallyCollapsed(store);
+      });
 
-          done();
-        });
+      it('should show the collapsed file warning with expansion link', () => {
+        expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed');
+        expect(findToggleLinks(wrapper).length).toEqual(1);
       });
 
-      it('should have collapsed text and link even before rendered', done => {
-        vm.renderIt = false;
-        vm.isCollapsed = true;
+      it('should style the component so that it `.has-body` for layout purposes', () => {
+        expect(wrapper.classes('has-body')).toBe(true);
+      });
+    });
 
-        vm.$nextTick(() => {
-          expect(vm.$el.innerText).toContain('This diff is collapsed');
-          expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+    describe('not collapsed', () => {
+      beforeEach(() => {
+        makeFileOpenByDefault(store);
+        markFileToBeRendered(store);
+      });
 
-          done();
-        });
+      it('should have the file content', async () => {
+        expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
       });
 
-      it('should be collapsable for unreadable files', done => {
-        vm.$destroy();
-        vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
-          file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
-          canCurrentUserFork: false,
-          viewDiffsFileByFile: false,
-        }).$mount();
+      it('should style the component so that it `.has-body` for layout purposes', () => {
+        expect(wrapper.classes('has-body')).toBe(true);
+      });
+    });
 
-        vm.renderIt = false;
-        vm.isCollapsed = true;
+    describe('toggle', () => {
+      it('should update store state', async () => {
+        jest.spyOn(wrapper.vm.$store, 'dispatch').mockImplementation(() => {});
 
-        vm.$nextTick(() => {
-          expect(vm.$el.innerText).toContain('This diff is collapsed');
-          expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+        toggleFile(wrapper);
 
-          done();
+        expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/setFileCollapsedByUser', {
+          filePath: wrapper.vm.file.file_path,
+          collapsed: true,
         });
       });
 
-      it('should be collapsed for renamed files', done => {
-        vm.renderIt = true;
-        vm.isCollapsed = false;
-        vm.file.highlighted_diff_lines = null;
-        vm.file.viewer.name = diffViewerModes.renamed;
-
-        vm.$nextTick(() => {
-          expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+      describe('fetch collapsed diff', () => {
+        const prepFile = async (inlineLines, parallelLines, readableText) => {
+          forceHasDiff({
+            store,
+            inlineLines,
+            parallelLines,
+            readableText,
+          });
 
-          done();
-        });
-      });
+          await wrapper.vm.$nextTick();
 
-      it('should be collapsed for mode changed files', done => {
-        vm.renderIt = true;
-        vm.isCollapsed = false;
-        vm.file.highlighted_diff_lines = null;
-        vm.file.viewer.name = diffViewerModes.mode_changed;
+          toggleFile(wrapper);
+        };
 
-        vm.$nextTick(() => {
-          expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+        beforeEach(() => {
+          jest.spyOn(wrapper.vm, 'requestDiffLoad').mockImplementation(() => {});
 
-          done();
+          makeFileAutomaticallyCollapsed(store);
         });
-      });
 
-      it('should have loading icon while loading a collapsed diffs', done => {
-        vm.isCollapsed = true;
-        vm.isLoadingCollapsedDiff = true;
+        it.each`
+          inlineLines | parallelLines | readableText
+          ${[1]}      | ${[1]}        | ${true}
+          ${[]}       | ${[1]}        | ${true}
+          ${[1]}      | ${[]}         | ${true}
+          ${[1]}      | ${[1]}        | ${false}
+          ${[]}       | ${[]}         | ${false}
+        `(
+          'does not make a request to fetch the diff for a diff file like { inline: $inlineLines, parallel: $parallelLines, readableText: $readableText }',
+          async ({ inlineLines, parallelLines, readableText }) => {
+            await prepFile(inlineLines, parallelLines, readableText);
+
+            expect(wrapper.vm.requestDiffLoad).not.toHaveBeenCalled();
+          },
+        );
 
-        vm.$nextTick(() => {
-          expect(vm.$el.querySelectorAll('.diff-content.loading').length).toEqual(1);
+        it.each`
+          inlineLines | parallelLines | readableText
+          ${[]}       | ${[]}         | ${true}
+        `(
+          'makes a request to fetch the diff for a diff file like { inline: $inlineLines, parallel: $parallelLines, readableText: $readableText }',
+          async ({ inlineLines, parallelLines, readableText }) => {
+            await prepFile(inlineLines, parallelLines, readableText);
 
-          done();
-        });
+            expect(wrapper.vm.requestDiffLoad).toHaveBeenCalled();
+          },
+        );
       });
+    });
 
-      it('should update store state', done => {
-        jest.spyOn(vm.$store, 'dispatch').mockImplementation(() => {});
-
-        vm.isCollapsed = true;
+    describe('loading', () => {
+      it('should have loading icon while loading a collapsed diffs', async () => {
+        makeFileAutomaticallyCollapsed(store);
+        wrapper.vm.isLoadingCollapsedDiff = true;
 
-        vm.$nextTick(() => {
-          expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/setFileCollapsed', {
-            filePath: vm.file.file_path,
-            collapsed: true,
-          });
+        await wrapper.vm.$nextTick();
 
-          done();
-        });
+        expect(findLoader(wrapper).exists()).toBe(true);
       });
+    });
 
-      it('updates local state when changing file state', done => {
-        vm.file.viewer.automaticallyCollapsed = true;
+    describe('general (other) collapsed', () => {
+      it('should be expandable for unreadable files', async () => {
+        ({ wrapper, store } = createComponent({ file: getUnreadableFile() }));
+        makeFileAutomaticallyCollapsed(store);
 
-        vm.$nextTick(() => {
-          expect(vm.isCollapsed).toBe(true);
+        await wrapper.vm.$nextTick();
 
-          done();
-        });
+        expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed');
+        expect(findToggleLinks(wrapper).length).toEqual(1);
       });
+
+      it.each`
+        mode
+        ${'renamed'}
+        ${'mode_changed'}
+      `(
+        'should render the DiffContent component for files whose mode is $mode',
+        async ({ mode }) => {
+          makeFileOpenByDefault(store);
+          markFileToBeRendered(store);
+          changeViewerType(store, mode);
+
+          await wrapper.vm.$nextTick();
+
+          expect(wrapper.classes('has-body')).toBe(true);
+          expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
+          expect(wrapper.find(DiffContentComponent).isVisible()).toBe(true);
+        },
+      );
     });
   });
 
   describe('too large diff', () => {
-    it('should have too large warning and blob link', done => {
+    it('should have too large warning and blob link', async () => {
+      const file = store.state.diffs.diffFiles[0];
       const BLOB_LINK = '/file/view/path';
-      vm.file.viewer.error = diffViewerErrors.too_large;
-      vm.file.viewer.error_message =
-        'This source diff could not be displayed because it is too large';
-      vm.file.view_path = BLOB_LINK;
-      vm.file.renderIt = true;
-
-      vm.$nextTick(() => {
-        expect(vm.$el.innerText).toContain(
-          'This source diff could not be displayed because it is too large',
-        );
 
-        done();
+      Object.assign(store.state.diffs.diffFiles[0], {
+        ...file,
+        view_path: BLOB_LINK,
+        renderIt: true,
+        viewer: {
+          ...file.viewer,
+          error: diffViewerErrors.too_large,
+          error_message: 'This source diff could not be displayed because it is too large',
+        },
       });
-    });
-  });
 
-  describe('watch collapsed', () => {
-    it('calls handleLoadCollapsedDiff if collapsed changed & file has no lines', done => {
-      jest.spyOn(vm, 'handleLoadCollapsedDiff').mockImplementation(() => {});
-
-      vm.file.highlighted_diff_lines = [];
-      vm.file.parallel_diff_lines = [];
-      vm.isCollapsed = true;
-
-      vm.$nextTick()
-        .then(() => {
-          vm.isCollapsed = false;
-
-          return vm.$nextTick();
-        })
-        .then(() => {
-          expect(vm.handleLoadCollapsedDiff).toHaveBeenCalled();
-        })
-        .then(done)
-        .catch(done.fail);
-    });
+      await wrapper.vm.$nextTick();
 
-    it('does not call handleLoadCollapsedDiff if collapsed changed & file is unreadable', done => {
-      vm.$destroy();
-      vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
-        file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
-        canCurrentUserFork: false,
-        viewDiffsFileByFile: false,
-      }).$mount();
-
-      jest.spyOn(vm, 'handleLoadCollapsedDiff').mockImplementation(() => {});
-
-      vm.file.highlighted_diff_lines = [];
-      vm.file.parallel_diff_lines = undefined;
-      vm.isCollapsed = true;
-
-      vm.$nextTick()
-        .then(() => {
-          vm.isCollapsed = false;
-
-          return vm.$nextTick();
-        })
-        .then(() => {
-          expect(vm.handleLoadCollapsedDiff).not.toHaveBeenCalled();
-        })
-        .then(done)
-        .catch(done.fail);
+      expect(wrapper.vm.$el.innerText).toContain(
+        'This source diff could not be displayed because it is too large',
+      );
     });
   });
 });
diff --git a/spec/frontend/diffs/mock_data/diff_file.js b/spec/frontend/diffs/mock_data/diff_file.js
index d3886819a911d8..cef776c885a11e 100644
--- a/spec/frontend/diffs/mock_data/diff_file.js
+++ b/spec/frontend/diffs/mock_data/diff_file.js
@@ -27,6 +27,7 @@ export default {
     name: 'text',
     error: null,
     automaticallyCollapsed: false,
+    manuallyCollapsed: null,
   },
   added_lines: 2,
   removed_lines: 0,
diff --git a/spec/frontend/diffs/mock_data/diff_file_unreadable.js b/spec/frontend/diffs/mock_data/diff_file_unreadable.js
index f6cdca9950a5bd..2a5d694e3b8102 100644
--- a/spec/frontend/diffs/mock_data/diff_file_unreadable.js
+++ b/spec/frontend/diffs/mock_data/diff_file_unreadable.js
@@ -26,6 +26,7 @@ export default {
     name: 'text',
     error: null,
     automaticallyCollapsed: false,
+    manuallyCollapsed: null,
   },
   added_lines: 0,
   removed_lines: 0,
diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index c3e4ee9c531c23..d41c4ae44ebddd 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/spec/frontend/diffs/store/actions_spec.js
@@ -42,7 +42,7 @@ import {
   fetchFullDiff,
   toggleFullDiff,
   switchToFullDiffFromRenamedFile,
-  setFileCollapsed,
+  setFileCollapsedByUser,
   setExpandedDiffLines,
   setSuggestPopoverDismissed,
   changeCurrentCommit,
@@ -1216,13 +1216,18 @@ describe('DiffsStoreActions', () => {
     });
   });
 
-  describe('setFileCollapsed', () => {
+  describe('setFileUserCollapsed', () => {
     it('commits SET_FILE_COLLAPSED', done => {
       testAction(
-        setFileCollapsed,
+        setFileCollapsedByUser,
         { filePath: 'test', collapsed: true },
         null,
-        [{ type: types.SET_FILE_COLLAPSED, payload: { filePath: 'test', collapsed: true } }],
+        [
+          {
+            type: types.SET_FILE_COLLAPSED,
+            payload: { filePath: 'test', collapsed: true, trigger: 'manual' },
+          },
+        ],
         [],
         done,
       );
diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js
index 0083f1d8b4466c..effe96ad3e6e04 100644
--- a/spec/frontend/diffs/store/getters_spec.js
+++ b/spec/frontend/diffs/store/getters_spec.js
@@ -50,22 +50,32 @@ describe('Diffs Module Getters', () => {
   });
 
   describe('hasCollapsedFile', () => {
-    it('returns true when all files are collapsed', () => {
-      localState.diffFiles = [
-        { viewer: { automaticallyCollapsed: true } },
-        { viewer: { automaticallyCollapsed: true } },
-      ];
-
-      expect(getters.hasCollapsedFile(localState)).toEqual(true);
-    });
-
-    it('returns true when at least one file is collapsed', () => {
-      localState.diffFiles = [
-        { viewer: { automaticallyCollapsed: false } },
-        { viewer: { automaticallyCollapsed: true } },
-      ];
-
-      expect(getters.hasCollapsedFile(localState)).toEqual(true);
+    const autoCollapsedFile = { viewer: { automaticallyCollapsed: true, manuallyCollapsed: null } };
+    const manuallyCollapsedFile = {
+      viewer: { automaticallyCollapsed: false, manuallyCollapsed: true },
+    };
+    const openFile = { viewer: { automaticallyCollapsed: false, manuallyCollapsed: false } };
+
+    it.each`
+      description                                 | key            | value    | files
+      ${'all files are automatically collapsed'}  | ${'any'}       | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are automatically collapsed'}  | ${'automatic'} | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are automatically collapsed'}  | ${'manual'}    | ${false} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${'any'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${'automatic'} | ${false} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${'manual'}    | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'no files are collapsed in any way'}      | ${'any'}       | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'no files are collapsed in any way'}      | ${'automatic'} | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'no files are collapsed in any way'}      | ${'manual'}    | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${'any'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${'automatic'} | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${'manual'}    | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+    `('$key is $value when $description', ({ key, value, files }) => {
+      localState.diffFiles = files;
+
+      const getterResult = getters.hasCollapsedFile(localState);
+
+      expect(getterResult[key]).toEqual(value);
     });
   });
 
-- 
GitLab


From 9664e0d52743f77d1cdb71f7a92e70377a2f26a4 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 14 Oct 2020 01:14:54 -0600
Subject: [PATCH 04/13] Add changelog describing the basic feature change

---
 .../feature-collapsed-diff-files-user-collapsed-flag.yml    | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml

diff --git a/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml b/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml
new file mode 100644
index 00000000000000..5c38a1eb4696f2
--- /dev/null
+++ b/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml
@@ -0,0 +1,6 @@
+---
+title: Manually collapsed diff files are now significantly shorter and less visually
+  intrusive
+merge_request: 43911
+author:
+type: changed
-- 
GitLab


From 580633e7e11b7a9f667d429e90bc0a5035ba5389 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 13:14:53 -0600
Subject: [PATCH 05/13] Break up getters tests into three separate tables

---
 spec/frontend/diffs/store/getters_spec.js | 50 ++++++++++++++++-------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js
index effe96ad3e6e04..e490abcda23aed 100644
--- a/spec/frontend/diffs/store/getters_spec.js
+++ b/spec/frontend/diffs/store/getters_spec.js
@@ -57,25 +57,45 @@ describe('Diffs Module Getters', () => {
     const openFile = { viewer: { automaticallyCollapsed: false, manuallyCollapsed: false } };
 
     it.each`
-      description                                 | key            | value    | files
-      ${'all files are automatically collapsed'}  | ${'any'}       | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
-      ${'all files are automatically collapsed'}  | ${'automatic'} | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
-      ${'all files are automatically collapsed'}  | ${'manual'}    | ${false} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
-      ${'all files are manually collapsed'}       | ${'any'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
-      ${'all files are manually collapsed'}       | ${'automatic'} | ${false} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
-      ${'all files are manually collapsed'}       | ${'manual'}    | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
-      ${'no files are collapsed in any way'}      | ${'any'}       | ${false} | ${[{ ...openFile }, { ...openFile }]}
-      ${'no files are collapsed in any way'}      | ${'automatic'} | ${false} | ${[{ ...openFile }, { ...openFile }]}
-      ${'no files are collapsed in any way'}      | ${'manual'}    | ${false} | ${[{ ...openFile }, { ...openFile }]}
-      ${'some files are collapsed in either way'} | ${'any'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
-      ${'some files are collapsed in either way'} | ${'automatic'} | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
-      ${'some files are collapsed in either way'} | ${'manual'}    | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
-    `('$key is $value when $description', ({ key, value, files }) => {
+      description                                 | value    | files
+      ${'all files are automatically collapsed'}  | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'no files are collapsed in any way'}      | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+    `('`any` is $value when $description', ({ value, files }) => {
       localState.diffFiles = files;
 
       const getterResult = getters.hasCollapsedFile(localState);
 
-      expect(getterResult[key]).toEqual(value);
+      expect(getterResult.any).toEqual(value);
+    });
+
+    it.each`
+      description                                 | value    | files
+      ${'all files are automatically collapsed'}  | ${true}  | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${false} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'no files are collapsed in any way'}      | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+    `('`automatic` is $value when $description', ({ value, files }) => {
+      localState.diffFiles = files;
+
+      const getterResult = getters.hasCollapsedFile(localState);
+
+      expect(getterResult.automatic).toEqual(value);
+    });
+
+    it.each`
+      description                                 | value    | files
+      ${'all files are automatically collapsed'}  | ${false} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+      ${'all files are manually collapsed'}       | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+      ${'no files are collapsed in any way'}      | ${false} | ${[{ ...openFile }, { ...openFile }]}
+      ${'some files are collapsed in either way'} | ${true}  | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+    `('`manual` is $value when $description', ({ value, files }) => {
+      localState.diffFiles = files;
+
+      const getterResult = getters.hasCollapsedFile(localState);
+
+      expect(getterResult.manual).toEqual(value);
     });
   });
 
-- 
GitLab


From 348d4f31f758922ebaf407fe459a34d1fc3375f2 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 14:32:06 -0600
Subject: [PATCH 06/13] Switch getter to be named `whichCollapsedTypes`

Which more accurately represents its return: an object of
all of which types of collapsed files are present in the state.
---
 app/assets/javascripts/diffs/components/app.vue           | 4 ++--
 .../javascripts/diffs/components/compare_versions.vue     | 4 ++--
 app/assets/javascripts/diffs/store/getters.js             | 2 +-
 spec/frontend/diffs/store/getters_spec.js                 | 8 ++++----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 7895407cef0541..a800cc8edc8131 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -152,7 +152,7 @@ export default {
       'canMerge',
       'hasConflicts',
     ]),
-    ...mapGetters('diffs', ['hasCollapsedFile', 'isParallelView', 'currentDiffIndex']),
+    ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
     diffs() {
       if (!this.viewDiffsFileByFile) {
@@ -205,7 +205,7 @@ export default {
         visible = this.$options.alerts.ALERT_OVERFLOW_HIDDEN;
       } else if (this.isDiffHead && this.hasConflicts) {
         visible = this.$options.alerts.ALERT_MERGE_CONFLICT;
-      } else if (this.hasCollapsedFile.automatic && !this.viewDiffsFileByFile) {
+      } else if (this.whichCollapsedTypes.automatic && !this.viewDiffsFileByFile) {
         visible = this.$options.alerts.ALERT_COLLAPSED_FILES;
       }
 
diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue
index 124a15a498a634..59c64303c606e7 100644
--- a/app/assets/javascripts/diffs/components/compare_versions.vue
+++ b/app/assets/javascripts/diffs/components/compare_versions.vue
@@ -38,7 +38,7 @@ export default {
   },
   computed: {
     ...mapGetters('diffs', [
-      'hasCollapsedFile',
+      'whichCollapsedTypes',
       'diffCompareDropdownTargetVersions',
       'diffCompareDropdownSourceVersions',
     ]),
@@ -129,7 +129,7 @@ export default {
           {{ __('Show latest version') }}
         </gl-button>
         <gl-button
-          v-show="hasCollapsedFile.any"
+          v-show="whichCollapsedTypes.any"
           variant="default"
           class="gl-mr-3"
           @click="expandAllFiles"
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 38ae3ad503e757..a965002fbb45a4 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -8,7 +8,7 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW
 
 export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE;
 
-export const hasCollapsedFile = state => {
+export const whichCollapsedTypes = state => {
   const automatic = state.diffFiles.some(file => file.viewer?.automaticallyCollapsed);
   const manual = state.diffFiles.some(file => file.viewer?.manuallyCollapsed);
 
diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js
index e490abcda23aed..7e936c561fc097 100644
--- a/spec/frontend/diffs/store/getters_spec.js
+++ b/spec/frontend/diffs/store/getters_spec.js
@@ -49,7 +49,7 @@ describe('Diffs Module Getters', () => {
     });
   });
 
-  describe('hasCollapsedFile', () => {
+  describe('whichCollapsedTypes', () => {
     const autoCollapsedFile = { viewer: { automaticallyCollapsed: true, manuallyCollapsed: null } };
     const manuallyCollapsedFile = {
       viewer: { automaticallyCollapsed: false, manuallyCollapsed: true },
@@ -65,7 +65,7 @@ describe('Diffs Module Getters', () => {
     `('`any` is $value when $description', ({ value, files }) => {
       localState.diffFiles = files;
 
-      const getterResult = getters.hasCollapsedFile(localState);
+      const getterResult = getters.whichCollapsedTypes(localState);
 
       expect(getterResult.any).toEqual(value);
     });
@@ -79,7 +79,7 @@ describe('Diffs Module Getters', () => {
     `('`automatic` is $value when $description', ({ value, files }) => {
       localState.diffFiles = files;
 
-      const getterResult = getters.hasCollapsedFile(localState);
+      const getterResult = getters.whichCollapsedTypes(localState);
 
       expect(getterResult.automatic).toEqual(value);
     });
@@ -93,7 +93,7 @@ describe('Diffs Module Getters', () => {
     `('`manual` is $value when $description', ({ value, files }) => {
       localState.diffFiles = files;
 
-      const getterResult = getters.hasCollapsedFile(localState);
+      const getterResult = getters.whichCollapsedTypes(localState);
 
       expect(getterResult.manual).toEqual(value);
     });
-- 
GitLab


From e3c7dbec5a5d720f9bf383ef1707e330e5a55af6 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 14:32:53 -0600
Subject: [PATCH 07/13] Remove unused getter for counts of collapsed files

---
 app/assets/javascripts/diffs/store/getters.js | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index a965002fbb45a4..4900dcd473d76b 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -18,22 +18,6 @@ export const whichCollapsedTypes = state => {
     manual,
   };
 };
-export const collapsedFilesCount = state => {
-  const counts = {
-    automatic: 0,
-    manual: 0,
-  };
-
-  state.diffFiles.forEach(file => {
-    if (file.viewer?.manuallyCollapsed) {
-      counts.manual += 1;
-    } else if (file.viewer?.automaticallyCollapsed) {
-      counts.automatic += 1;
-    }
-  });
-
-  return counts;
-};
 
 export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
 
-- 
GitLab


From 66f4837a90bdd3f53173a7431eeb26747554e7bc Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 17:49:20 -0600
Subject: [PATCH 08/13] Start files with a smarter collapsed default

---
 app/assets/javascripts/diffs/components/diff_file.vue | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 5824871faf4826..bd7eb47ebb1849 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -46,7 +46,7 @@ export default {
     return {
       isLoadingCollapsedDiff: false,
       forkMessageVisible: false,
-      isCollapsed: false,
+      isCollapsed: isCollapsed(this.file),
     };
   },
   computed: {
-- 
GitLab


From 9601ad5d683aec578e6ddb7fbc8f97f5a3d4f0ac Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 19:07:35 -0600
Subject: [PATCH 09/13] Update to a slightly different computed name scheme

---
 app/assets/javascripts/diffs/components/diff_file.vue | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index bd7eb47ebb1849..1dd2d3e743054c 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -73,7 +73,7 @@ export default {
       return this.file.viewer.error === diffViewerErrors.too_large;
     },
     errorMessage() {
-      return !this.manualCollapse ? this.file.viewer.error_message : '';
+      return !this.manuallyCollapsed ? this.file.viewer.error_message : '';
     },
     forkMessage() {
       return sprintf(
@@ -87,17 +87,17 @@ export default {
         false,
       );
     },
-    automaticCollapse() {
+    automaticallyCollapsed() {
       return collapsedType(this.file) === DIFF_FILE_AUTOMATIC_COLLAPSE;
     },
-    manualCollapse() {
+    manuallyCollapsed() {
       return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE;
     },
     showBody() {
-      return !this.isCollapsed || this.automaticCollapse;
+      return !this.isCollapsed || this.automaticallyCollapsed;
     },
     showWarning() {
-      return this.isCollapsed && (this.automaticCollapse && !this.viewDiffsFileByFile);
+      return this.isCollapsed && (this.automaticallyCollapsed && !this.viewDiffsFileByFile);
     },
     showContent() {
       return !this.isCollapsed && !this.isFileTooLarge;
-- 
GitLab


From 4552fda6a14b6a61ab8e79b1578548ca0f2b8908 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 21:50:07 -0600
Subject: [PATCH 10/13] Update to simpler method name for getting a diff

---
 app/assets/javascripts/diffs/components/diff_file.vue | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 1dd2d3e743054c..733ad033fcdf19 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -109,7 +109,7 @@ export default {
         this.isCollapsed = isCollapsed(this.file);
 
         if (newHash && oldHash && !this.hasDiff) {
-          this.requestDiffLoad();
+          this.requestDiff();
         }
       },
       immediate: true,
@@ -132,7 +132,7 @@ export default {
     },
   },
   created() {
-    eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiffLoad);
+    eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff);
   },
   methods: {
     ...mapActions('diffs', [
@@ -150,10 +150,10 @@ export default {
       });
 
       if (!this.hasDiff && currentCollapsedFlag) {
-        this.requestDiffLoad();
+        this.requestDiff();
       }
     },
-    requestDiffLoad() {
+    requestDiff() {
       this.isLoadingCollapsedDiff = true;
 
       this.loadCollapsedDiff(this.file)
-- 
GitLab


From 438c1fd1ff75e2e03396b5b834073698e79bc2b2 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 21:57:31 -0600
Subject: [PATCH 11/13] Remove unused `return`

---
 app/assets/javascripts/diffs/store/mutations.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index ca4d7b0461be63..5328845e4d9457 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -21,7 +21,7 @@ function updateDiffFilesInState(state, files) {
 }
 
 function renderFile(file) {
-  return Object.assign(file, {
+  Object.assign(file, {
     renderIt: true,
   });
 }
-- 
GitLab


From 4ddadccc159057e9591fdfce58bb8a385e71d2b1 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Fri, 23 Oct 2020 22:38:53 -0600
Subject: [PATCH 12/13] Switch to running JS listeners instead of CSS

---
 .../diffs/components/diff_file.vue            | 30 +++++++++++++++++--
 app/assets/stylesheets/framework/diffs.scss   | 19 ------------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 733ad033fcdf19..0d5181d5720ee4 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -87,6 +87,22 @@ export default {
         false,
       );
     },
+    hasBodyClasses() {
+      const domParts = {
+        header: 'gl-rounded-base!',
+        contentByHash: '',
+        content: '',
+      };
+
+      if (this.showBody) {
+        domParts.header = 'gl-rounded-bottom-left-none gl-rounded-bottom-right-none';
+        domParts.contentByHash =
+          'gl-rounded-none gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-border-1 gl-border-t-0! gl-border-solid gl-border-gray-100';
+        domParts.content = 'gl-rounded-bottom-left-base gl-rounded-bottom-right-base';
+      }
+
+      return domParts;
+    },
     automaticallyCollapsed() {
       return collapsedType(this.file) === DIFF_FILE_AUTOMATIC_COLLAPSE;
     },
@@ -203,6 +219,7 @@ export default {
       :add-merge-request-buttons="true"
       :view-diffs-file-by-file="viewDiffsFileByFile"
       class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100"
+      :class="hasBodyClasses.header"
       @toggleFile="handleToggle"
       @showForkMessage="showForkMessage"
     />
@@ -223,7 +240,11 @@ export default {
       </button>
     </div>
     <template v-else>
-      <div :id="`diff-content-${file.file_hash}`" data-testid="content-area">
+      <div
+        :id="`diff-content-${file.file_hash}`"
+        :class="hasBodyClasses.contentByHash"
+        data-testid="content-area"
+      >
         <gl-loading-icon
           v-if="showLoadingIcon"
           class="diff-content loading gl-my-0 gl-pt-3"
@@ -244,7 +265,12 @@ export default {
               {{ __('Click to expand it.') }}
             </a>
           </div>
-          <diff-content v-show="showContent" :diff-file="file" :help-page-path="helpPagePath" />
+          <diff-content
+            v-show="showContent"
+            :class="hasBodyClasses.content"
+            :diff-file="file"
+            :help-page-path="helpPagePath"
+          />
         </template>
       </div>
     </template>
diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss
index 6c22202f67f59b..62c2e15fcc8e13 100644
--- a/app/assets/stylesheets/framework/diffs.scss
+++ b/app/assets/stylesheets/framework/diffs.scss
@@ -8,26 +8,8 @@
 
   &.has-body {
     .file-title {
-      @include gl-rounded-bottom-left-none();
-      @include gl-rounded-bottom-right-none();
-
       box-shadow: 0 -2px 0 0 var(--white);
     }
-
-    .diff-content {
-      @include gl-rounded-bottom-left-base();
-      @include gl-rounded-bottom-right-base();
-    }
-
-    [id^='diff-content-'] {
-      @include gl-rounded-0();
-      @include gl-rounded-bottom-left-base();
-      @include gl-rounded-bottom-right-base();
-      @include gl-border-1();
-      @include gl-border-t-0();
-      @include gl-border-solid();
-      @include gl-border-gray-100();
-    }
   }
 
   table.code tr:last-of-type td:last-of-type {
@@ -36,7 +18,6 @@
 
   .file-title,
   .file-title-flex-parent {
-    @include gl-rounded-base();
     cursor: pointer;
 
     .dropdown-menu {
-- 
GitLab


From c032609b6feba931fd94c51cf68f4389a27b8019 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Sat, 24 Oct 2020 10:41:53 -0600
Subject: [PATCH 13/13] Fix test for renamed method

---
 spec/frontend/diffs/components/diff_file_spec.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index 7bd9da2f673620..59aa6d811eee4d 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -213,7 +213,7 @@ describe('DiffFile', () => {
         };
 
         beforeEach(() => {
-          jest.spyOn(wrapper.vm, 'requestDiffLoad').mockImplementation(() => {});
+          jest.spyOn(wrapper.vm, 'requestDiff').mockImplementation(() => {});
 
           makeFileAutomaticallyCollapsed(store);
         });
@@ -230,7 +230,7 @@ describe('DiffFile', () => {
           async ({ inlineLines, parallelLines, readableText }) => {
             await prepFile(inlineLines, parallelLines, readableText);
 
-            expect(wrapper.vm.requestDiffLoad).not.toHaveBeenCalled();
+            expect(wrapper.vm.requestDiff).not.toHaveBeenCalled();
           },
         );
 
@@ -242,7 +242,7 @@ describe('DiffFile', () => {
           async ({ inlineLines, parallelLines, readableText }) => {
             await prepFile(inlineLines, parallelLines, readableText);
 
-            expect(wrapper.vm.requestDiffLoad).toHaveBeenCalled();
+            expect(wrapper.vm.requestDiff).toHaveBeenCalled();
           },
         );
       });
-- 
GitLab