Merge branch...

Merge branch '232820-display-a-visible-warning-when-at-least-one-file-is-collapsed-in-a-mr-diff-2' into 'master'

Add a warning when some files are collapsed

See merge request gitlab-org/gitlab!40752

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

@@ -18,6 +18,7 @@ import TreeList from './tree_list.vue';
 
 import HiddenFilesWarning from './hidden_files_warning.vue';
 import MergeConflictWarning from './merge_conflict_warning.vue';
+import CollapsedFilesWarning from './collapsed_files_warning.vue';
 
 import {
   TREE_LIST_WIDTH_STORAGE_KEY,
@@ -27,6 +28,9 @@ import {
   TREE_HIDE_STATS_WIDTH,
   MR_TREE_SHOW_KEY,
   CENTERED_LIMITED_CONTAINER_CLASSES,
+  ALERT_OVERFLOW_HIDDEN,
+  ALERT_MERGE_CONFLICT,
+  ALERT_COLLAPSED_FILES,
 } from '../constants';
 
 export default {
@@ -37,6 +41,7 @@ export default {
     NoChanges,
     HiddenFilesWarning,
     MergeConflictWarning,
+    CollapsedFilesWarning,
     CommitWidget,
     TreeList,
     GlLoadingIcon,
@@ -45,6 +50,11 @@ export default {
     GlSprintf,
   },
   mixins: [glFeatureFlagsMixin()],
+  alerts: {
+    ALERT_OVERFLOW_HIDDEN,
+    ALERT_MERGE_CONFLICT,
+    ALERT_COLLAPSED_FILES,
+  },
   props: {
     endpoint: {
       type: String,
@@ -114,6 +124,7 @@ export default {
     return {
       treeWidth,
       diffFilesLength: 0,
+      collapsedWarningDismissed: false,
     };
   },
   computed: {
@@ -142,7 +153,7 @@ export default {
       'canMerge',
       'hasConflicts',
     ]),
-    ...mapGetters('diffs', ['isParallelView', 'currentDiffIndex']),
+    ...mapGetters('diffs', ['hasCollapsedFile', 'isParallelView', 'currentDiffIndex']),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
     diffs() {
       if (!this.viewDiffsFileByFile) {
@@ -188,6 +199,23 @@ export default {
 
       return currentFileNumber < diffFiles.length ? currentFileNumber + 1 : null;
     },
+    visibleWarning() {
+      let visible = false;
+
+      if (this.renderOverflowWarning) {
+        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
+      ) {
+        visible = this.$options.alerts.ALERT_COLLAPSED_FILES;
+      }
+
+      return visible;
+    },
   },
   watch: {
     commit(newCommit, oldCommit) {
@@ -401,6 +429,9 @@ export default {
         this.toggleShowTreeList(false);
       }
     },
+    dismissCollapsedWarning() {
+      this.collapsedWarningDismissed = true;
+    },
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,
@@ -418,18 +449,23 @@ export default {
       />
 
       <hidden-files-warning
-        v-if="renderOverflowWarning"
+        v-if="visibleWarning == $options.alerts.ALERT_OVERFLOW_HIDDEN"
         :visible="numVisibleFiles"
         :total="numTotalFiles"
         :plain-diff-path="plainDiffPath"
         :email-patch-path="emailPatchPath"
       />
       <merge-conflict-warning
-        v-if="isDiffHead && hasConflicts"
+        v-if="visibleWarning == $options.alerts.ALERT_MERGE_CONFLICT"
         :limited="isLimitedContainer"
         :resolution-path="conflictResolutionPath"
         :mergeable="canMerge"
       />
+      <collapsed-files-warning
+        v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
+        :limited="isLimitedContainer"
+        @dismiss="dismissCollapsedWarning"
+      />
 
       <div
         :data-can-create-note="getNoteableData.current_user.can_create_note"

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

+<script>
+import { mapActions } from 'vuex';
+
+import { GlAlert, GlButton } from '@gitlab/ui';
+
+import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants';
+
+export default {
+  components: {
+    GlAlert,
+    GlButton,
+  },
+  props: {
+    limited: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    dismissed: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  data() {
+    return {
+      isDismissed: this.dismissed,
+    };
+  },
+  computed: {
+    containerClasses() {
+      return {
+        [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited,
+      };
+    },
+  },
+
+  methods: {
+    ...mapActions('diffs', ['expandAllFiles']),
+    dismiss() {
+      this.isDismissed = true;
+      this.$emit('dismiss');
+    },
+    expand() {
+      this.expandAllFiles();
+      this.dismiss();
+    },
+  },
+};
+</script>
+
+<template>
+  <div v-if="!isDismissed" data-testid="root" :class="containerClasses">
+    <gl-alert
+      :dismissible="true"
+      :title="__('Some changes are not shown')"
+      variant="warning"
+      class="gl-mb-5"
+      @dismiss="dismiss"
+    >
+      <p class="gl-mb-0">
+        {{ __('For a faster browsing experience, some files are collapsed by default.') }}
+      </p>
+      <template #actions>
+        <gl-button category="secondary" variant="warning" class="gl-alert-action" @click="expand">
+          {{ __('Expand all files') }}
+        </gl-button>
+      </template>
+    </gl-alert>
+  </div>
+</template>

app/assets/javascripts/diffs/constants.js

@@ -68,6 +68,11 @@ export const DIFFS_PER_PAGE = 20;
 export const DIFF_COMPARE_BASE_VERSION_INDEX = -1;
 export const DIFF_COMPARE_HEAD_VERSION_INDEX = -2;
 
+// Diff View Alerts
+export const ALERT_OVERFLOW_HIDDEN = 'overflow';
+export const ALERT_MERGE_CONFLICT = 'merge-conflict';
+export const ALERT_COLLAPSED_FILES = 'collapsed';
+
 // State machine states
 export const STATE_IDLING = 'idle';
 export const STATE_LOADING = 'loading';

changelogs/unreleased/feature-collapsed-files-warning.yml

+---
+title: Add a warning when any diff files are collapsed
+merge_request: 40752
+author:
+type: added

locale/gitlab.pot

@@ -10374,6 +10374,9 @@ msgstr ""
 msgid "Expand all"
 msgstr ""
 
+msgid "Expand all files"
+msgstr ""
+
 msgid "Expand approvers"
 msgstr ""
 
@@ -11216,6 +11219,9 @@ msgstr ""
 msgid "Footer message"
 msgstr ""
 
+msgid "For a faster browsing experience, some files are collapsed by default."
+msgstr ""
+
 msgid "For help setting up the Service Desk for your instance, please contact an administrator."
 msgstr ""
 
@@ -23562,6 +23568,9 @@ msgstr ""
 msgid "Solution"
 msgstr ""
 
+msgid "Some changes are not shown"
+msgstr ""
+
 msgid "Some child epics may be hidden due to applied filters"
 msgstr ""
 

spec/frontend/diffs/components/app_spec.js

@@ -9,6 +9,7 @@ import NoChanges from '~/diffs/components/no_changes.vue';
 import DiffFile from '~/diffs/components/diff_file.vue';
 import CompareVersions from '~/diffs/components/compare_versions.vue';
 import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue';
+import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
 import CommitWidget from '~/diffs/components/commit_widget.vue';
 import TreeList from '~/diffs/components/tree_list.vue';
 import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants';
@@ -22,6 +23,10 @@ const TEST_ENDPOINT = `${TEST_HOST}/diff/endpoint`;
 const COMMIT_URL = '[BASE URL]/OLD';
 const UPDATED_COMMIT_URL = '[BASE URL]/NEW';
 
+function getCollapsedFilesWarning(wrapper) {
+  return wrapper.find(CollapsedFilesWarning);
+}
+
 describe('diffs/components/app', () => {
   const oldMrTabs = window.mrTabs;
   let store;
@@ -668,24 +673,51 @@ describe('diffs/components/app', () => {
       );
     });
 
-    it('should render hidden files warning if render overflow warning is present', () => {
-      createComponent({}, ({ state }) => {
-        state.diffs.renderOverflowWarning = true;
-        state.diffs.realSize = '5';
-        state.diffs.plainDiffPath = 'plain diff path';
-        state.diffs.emailPatchPath = 'email patch path';
-        state.diffs.size = 1;
+    describe('warnings', () => {
+      describe('hidden files', () => {
+        it('should render hidden files warning if render overflow warning is present', () => {
+          createComponent({}, ({ state }) => {
+            state.diffs.renderOverflowWarning = true;
+            state.diffs.realSize = '5';
+            state.diffs.plainDiffPath = 'plain diff path';
+            state.diffs.emailPatchPath = 'email patch path';
+            state.diffs.size = 1;
+          });
+
+          expect(wrapper.find(HiddenFilesWarning).exists()).toBe(true);
+          expect(wrapper.find(HiddenFilesWarning).props()).toEqual(
+            expect.objectContaining({
+              total: '5',
+              plainDiffPath: 'plain diff path',
+              emailPatchPath: 'email patch path',
+              visible: 1,
+            }),
+          );
+        });
       });
 
-      expect(wrapper.find(HiddenFilesWarning).exists()).toBe(true);
-      expect(wrapper.find(HiddenFilesWarning).props()).toEqual(
-        expect.objectContaining({
-          total: '5',
-          plainDiffPath: 'plain diff path',
-          emailPatchPath: 'email patch path',
-          visible: 1,
-        }),
-      );
+      describe('collapsed files', () => {
+        it('should render the collapsed files warning if there are any collapsed files', () => {
+          createComponent({}, ({ state }) => {
+            state.diffs.diffFiles = [{ viewer: { collapsed: true } }];
+          });
+
+          expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
+        });
+
+        it('should not render the collapsed files warning if the user has dismissed the alert already', async () => {
+          createComponent({}, ({ state }) => {
+            state.diffs.diffFiles = [{ viewer: { collapsed: true } }];
+          });
+
+          expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
+
+          wrapper.vm.collapsedWarningDismissed = true;
+          await wrapper.vm.$nextTick();
+
+          expect(getCollapsedFilesWarning(wrapper).exists()).toBe(false);
+        });
+      });
     });
 
     it('should display commit widget if store has a commit', () => {

spec/frontend/diffs/components/collapsed_files_warning_spec.js

+import Vuex from 'vuex';
+import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
+import createStore from '~/diffs/store/modules';
+import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
+import { CENTERED_LIMITED_CONTAINER_CLASSES } from '~/diffs/constants';
+
+const propsData = {
+  limited: true,
+  mergeable: true,
+  resolutionPath: 'a-path',
+};
+const limitedClasses = CENTERED_LIMITED_CONTAINER_CLASSES.split(' ');
+
+describe('CollapsedFilesWarning', () => {
+  const localVue = createLocalVue();
+  let store;
+  let wrapper;
+
+  localVue.use(Vuex);
+
+  const getAlertActionButton = () =>
+    wrapper.find(CollapsedFilesWarning).find('button.gl-alert-action:first-child');
+  const getAlertCloseButton = () => wrapper.find(CollapsedFilesWarning).find('button');
+
+  const createComponent = (props = {}, { full } = { full: false }) => {
+    const mounter = full ? mount : shallowMount;
+    store = new Vuex.Store({
+      modules: {
+        diffs: createStore(),
+      },
+    });
+
+    wrapper = mounter(CollapsedFilesWarning, {
+      propsData: { ...propsData, ...props },
+      localVue,
+      store,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it.each`
+    limited  | containerClasses
+    ${true}  | ${limitedClasses}
+    ${false} | ${[]}
+  `(
+    'has the correct container classes when limited is $limited',
+    ({ limited, containerClasses }) => {
+      createComponent({ limited });
+
+      expect(wrapper.classes()).toEqual(containerClasses);
+    },
+  );
+
+  it.each`
+    present  | dismissed
+    ${false} | ${true}
+    ${true}  | ${false}
+  `('toggles the alert when dismissed is $dismissed', ({ present, dismissed }) => {
+    createComponent({ dismissed });
+
+    expect(wrapper.find('[data-testid="root"]').exists()).toBe(present);
+  });
+
+  it('dismisses the component when the alert "x" is clicked', async () => {
+    createComponent({}, { full: true });
+
+    expect(wrapper.find('[data-testid="root"]').exists()).toBe(true);
+
+    getAlertCloseButton().element.click();
+
+    await wrapper.vm.$nextTick();
+
+    expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
+  });
+
+  it('triggers the expandAllFiles action when the alert action button is clicked', () => {
+    createComponent({}, { full: true });
+
+    jest.spyOn(wrapper.vm.$store, 'dispatch').mockReturnValue(undefined);
+
+    getAlertActionButton().vm.$emit('click');
+
+    expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/expandAllFiles', undefined);
+  });
+});