Merge branch '53139-hide-tree-single-file' into 'master'

Collapse diff tree is only one file is present

Closes #53139

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

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

@@ -19,6 +19,7 @@ import {
   MIN_TREE_WIDTH,
   MAX_TREE_WIDTH,
   TREE_HIDE_STATS_WIDTH,
+  MR_TREE_SHOW_KEY,
 } from '../constants';
 
 export default {
@@ -162,10 +163,13 @@ export default {
       'setHighlightedRow',
       'cacheTreeListWidth',
       'scrollToFile',
+      'toggleShowTreeList',
     ]),
     fetchData() {
       this.fetchDiffFiles()
         .then(() => {
+          this.hideTreeListIfJustOneFile();
+
           requestIdleCallback(
             () => {
               this.setDiscussions();
@@ -231,6 +235,13 @@ export default {
         this.scrollToFile(this.diffFiles[targetIndex].file_path);
       }
     },
+    hideTreeListIfJustOneFile() {
+      const storedTreeShow = localStorage.getItem(MR_TREE_SHOW_KEY);
+
+      if ((storedTreeShow === null && this.diffFiles.length <= 1) || storedTreeShow === 'false') {
+        this.toggleShowTreeList(false);
+      }
+    },
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,

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

@@ -266,9 +266,12 @@ export const scrollToFile = ({ state, commit }, path) => {
   commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
 };
 
-export const toggleShowTreeList = ({ commit, state }) => {
+export const toggleShowTreeList = ({ commit, state }, saving = true) => {
   commit(types.TOGGLE_SHOW_TREE_LIST);
-  localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList);
+
+  if (saving) {
+    localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList);
+  }
 };
 
 export const openDiffFileCommentForm = ({ commit, getters }, formData) => {

app/assets/javascripts/diffs/store/modules/diff_state.js

@@ -1,13 +1,10 @@
 import Cookies from 'js-cookie';
 import { getParameterValues } from '~/lib/utils/url_utility';
-import bp from '~/breakpoints';
+import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../../constants';
-import { parseBoolean } from '~/lib/utils/common_utils';
-import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME, MR_TREE_SHOW_KEY } from '../../constants';
 
 const viewTypeFromQueryString = getParameterValues('view')[0];
 const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME);
 const defaultViewType = INLINE_DIFF_VIEW_TYPE;
-const storedTreeShow = localStorage.getItem(MR_TREE_SHOW_KEY);
 
 export default () => ({
   isLoading: true,
@@ -23,8 +20,7 @@ export default () => ({
   diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType,
   tree: [],
   treeEntries: {},
-  showTreeList:
+  showTreeList: true,
-    storedTreeShow === null ? bp.getBreakpointSize() !== 'xs' : parseBoolean(storedTreeShow),
   currentDiffFileId: '',
   projectPath: '',
   commentForms: [],

changelogs/unreleased/53139-hide-tree-single-file.yml

@@ -0,0 +1,5 @@
+---
+title: collapse file tree by default if the merge request changes only one file
+merge_request:
+author: Riccardo Padovani <riccardo@rpadovani.com>
+type: changed

spec/javascripts/diffs/components/app_spec.js

@@ -397,4 +397,61 @@ describe('diffs/components/app', () => {
       expect(wrapper.find(TreeList).exists()).toBe(true);
     });
   });
+
+  describe('hideTreeListIfJustOneFile', () => {
+    let toggleShowTreeList;
+
+    beforeEach(() => {
+      toggleShowTreeList = jasmine.createSpy('toggleShowTreeList');
+    });
+
+    afterEach(() => {
+      localStorage.removeItem('mr_tree_show');
+    });
+
+    it('calls toggleShowTreeList when only 1 file', () => {
+      createComponent({}, ({ state }) => {
+        state.diffs.diffFiles.push({ sha: '123' });
+      });
+
+      wrapper.setMethods({
+        toggleShowTreeList,
+      });
+
+      wrapper.vm.hideTreeListIfJustOneFile();
+
+      expect(toggleShowTreeList).toHaveBeenCalledWith(false);
+    });
+
+    it('does not call toggleShowTreeList when more than 1 file', () => {
+      createComponent({}, ({ state }) => {
+        state.diffs.diffFiles.push({ sha: '123' });
+        state.diffs.diffFiles.push({ sha: '124' });
+      });
+
+      wrapper.setMethods({
+        toggleShowTreeList,
+      });
+
+      wrapper.vm.hideTreeListIfJustOneFile();
+
+      expect(toggleShowTreeList).not.toHaveBeenCalled();
+    });
+
+    it('does not call toggleShowTreeList when localStorage is set', () => {
+      localStorage.setItem('mr_tree_show', 'true');
+
+      createComponent({}, ({ state }) => {
+        state.diffs.diffFiles.push({ sha: '123' });
+      });
+
+      wrapper.setMethods({
+        toggleShowTreeList,
+      });
+
+      wrapper.vm.hideTreeListIfJustOneFile();
+
+      expect(toggleShowTreeList).not.toHaveBeenCalled();
+    });
+  });
 });

spec/javascripts/diffs/store/actions_spec.js

@@ -734,6 +734,14 @@ describe('DiffsStoreActions', () => {
 
       expect(localStorage.setItem).toHaveBeenCalledWith('mr_tree_show', true);
     });
+
+    it('does not update localStorage', () => {
+      spyOn(localStorage, 'setItem');
+
+      toggleShowTreeList({ commit() {}, state: { showTreeList: true } }, false);
+
+      expect(localStorage.setItem).not.toHaveBeenCalled();
+    });
   });
 
   describe('renderFileForDiscussionId', () => {