Fixes the diffs tree list showing incorrectly

This fixes the merge request tree list showing incorrectly
when the metadata comes back before any diffs get returned.

This also changes the logic to hide the tree list
when the window width is small.

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

 <script>
 import { mapState, mapGetters, mapActions } from 'vuex';
 import { GlLoadingIcon, GlPagination, GlSprintf } from '@gitlab/ui';
+import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import Mousetrap from 'mousetrap';
 import { __ } from '~/locale';
 import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils';
@@ -316,7 +317,7 @@ export default {
       'setHighlightedRow',
       'cacheTreeListWidth',
       'scrollToFile',
-      'toggleShowTreeList',
+      'setShowTreeList',
       'navigateToDiffFileIndex',
     ]),
     navigateToDiffFileNumber(number) {
@@ -343,7 +344,7 @@ export default {
       this.fetchDiffFilesMeta()
         .then(({ real_size }) => {
           this.diffFilesLength = parseInt(real_size, 10);
-          if (toggleTree) this.hideTreeListIfJustOneFile();
+          if (toggleTree) this.setTreeDisplay();
 
           this.startDiffRendering();
         })
@@ -353,6 +354,7 @@ export default {
 
       this.fetchDiffFilesBatch()
         .then(() => {
+          if (toggleTree) this.setTreeDisplay();
           // Guarantee the discussions are assigned after the batch finishes.
           // Just watching the length of the discussions or the diff files
           // isn't enough, because with split diff loading, neither will
@@ -422,12 +424,17 @@ export default {
         this.scrollToFile(this.diffFiles[targetIndex].file_path);
       }
     },
-    hideTreeListIfJustOneFile() {
+    setTreeDisplay() {
       const storedTreeShow = localStorage.getItem(MR_TREE_SHOW_KEY);
+      let showTreeList = true;
 
-      if ((storedTreeShow === null && this.diffFiles.length <= 1) || storedTreeShow === 'false') {
-        this.toggleShowTreeList(false);
+      if (storedTreeShow !== null) {
+        showTreeList = Boolean(storedTreeShow);
+      } else if (!bp.isDesktop() || (!this.isBatchLoading && this.diffFiles.length <= 1)) {
+        showTreeList = false;
       }
+
+      return this.setShowTreeList({ showTreeList, saving: false });
     },
   },
   minTreeWidth: MIN_TREE_WIDTH,

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

@@ -65,11 +65,7 @@ export default {
     polyfillSticky(this.$el);
   },
   methods: {
-    ...mapActions('diffs', [
-      'setInlineDiffViewType',
-      'setParallelDiffViewType',
-      'toggleShowTreeList',
-    ]),
+    ...mapActions('diffs', ['setInlineDiffViewType', 'setParallelDiffViewType', 'setShowTreeList']),
     expandAllFiles() {
       eventHub.$emit(EVT_EXPAND_ALL_FILES);
     },
@@ -92,7 +88,7 @@ export default {
         class="gl-mr-3 js-toggle-tree-list"
         :title="toggleFileBrowserTitle"
         :selected="showTreeList"
-        @click="toggleShowTreeList"
+        @click="setShowTreeList({ showTreeList: !showTreeList })"
       />
       <gl-sprintf
         v-if="showDropdowns"

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

@@ -447,11 +447,11 @@ export const scrollToFile = ({ state, commit }, path) => {
   commit(types.VIEW_DIFF_FILE, fileHash);
 };
 
-export const toggleShowTreeList = ({ commit, state }, saving = true) => {
-  commit(types.TOGGLE_SHOW_TREE_LIST);
+export const setShowTreeList = ({ commit }, { showTreeList, saving = true }) => {
+  commit(types.SET_SHOW_TREE_LIST, showTreeList);
 
   if (saving) {
-    localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList);
+    localStorage.setItem(MR_TREE_SHOW_KEY, showTreeList);
   }
 };
 

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

@@ -17,7 +17,7 @@ export const RENDER_FILE = 'RENDER_FILE';
 export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE';
 export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE';
 export const TOGGLE_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN';
-export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST';
+export const SET_SHOW_TREE_LIST = 'SET_SHOW_TREE_LIST';
 export const VIEW_DIFF_FILE = 'VIEW_DIFF_FILE';
 
 export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM';

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

@@ -247,8 +247,8 @@ export default {
   [types.TOGGLE_FOLDER_OPEN](state, path) {
     state.treeEntries[path].opened = !state.treeEntries[path].opened;
   },
-  [types.TOGGLE_SHOW_TREE_LIST](state) {
-    state.showTreeList = !state.showTreeList;
+  [types.SET_SHOW_TREE_LIST](state, showTreeList) {
+    state.showTreeList = showTreeList;
   },
   [types.VIEW_DIFF_FILE](state, fileId) {
     state.currentDiffFileId = fileId;

spec/frontend/diffs/components/app_spec.js

@@ -638,47 +638,47 @@ describe('diffs/components/app', () => {
     });
   });
 
-  describe('hideTreeListIfJustOneFile', () => {
-    let toggleShowTreeList;
+  describe('setTreeDisplay', () => {
+    let setShowTreeList;
 
     beforeEach(() => {
-      toggleShowTreeList = jest.fn();
+      setShowTreeList = jest.fn();
     });
 
     afterEach(() => {
       localStorage.removeItem('mr_tree_show');
     });
 
-    it('calls toggleShowTreeList when only 1 file', () => {
+    it('calls setShowTreeList when only 1 file', () => {
       createComponent({}, ({ state }) => {
         state.diffs.diffFiles.push({ sha: '123' });
       });
 
       wrapper.setMethods({
-        toggleShowTreeList,
+        setShowTreeList,
       });
 
-      wrapper.vm.hideTreeListIfJustOneFile();
+      wrapper.vm.setTreeDisplay();
 
-      expect(toggleShowTreeList).toHaveBeenCalledWith(false);
+      expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: false, saving: false });
     });
 
-    it('does not call toggleShowTreeList when more than 1 file', () => {
+    it('calls setShowTreeList with true when more than 1 file is in diffs array', () => {
       createComponent({}, ({ state }) => {
         state.diffs.diffFiles.push({ sha: '123' });
         state.diffs.diffFiles.push({ sha: '124' });
       });
 
       wrapper.setMethods({
-        toggleShowTreeList,
+        setShowTreeList,
       });
 
-      wrapper.vm.hideTreeListIfJustOneFile();
+      wrapper.vm.setTreeDisplay();
 
-      expect(toggleShowTreeList).not.toHaveBeenCalled();
+      expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: true, saving: false });
     });
 
-    it('does not call toggleShowTreeList when localStorage is set', () => {
+    it('calls setShowTreeList with localstorage value', () => {
       localStorage.setItem('mr_tree_show', 'true');
 
       createComponent({}, ({ state }) => {
@@ -686,12 +686,12 @@ describe('diffs/components/app', () => {
       });
 
       wrapper.setMethods({
-        toggleShowTreeList,
+        setShowTreeList,
       });
 
-      wrapper.vm.hideTreeListIfJustOneFile();
+      wrapper.vm.setTreeDisplay();
 
-      expect(toggleShowTreeList).not.toHaveBeenCalled();
+      expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: true, saving: false });
     });
   });
 

spec/frontend/diffs/store/actions_spec.js

@@ -32,7 +32,7 @@ import {
   setHighlightedRow,
   toggleTreeOpen,
   scrollToFile,
-  toggleShowTreeList,
+  setShowTreeList,
   renderFileForDiscussionId,
   setRenderTreeList,
   setShowWhitespace,
@@ -901,15 +901,22 @@ describe('DiffsStoreActions', () => {
     });
   });
 
-  describe('toggleShowTreeList', () => {
+  describe('setShowTreeList', () => {
     it('commits toggle', done => {
-      testAction(toggleShowTreeList, null, {}, [{ type: types.TOGGLE_SHOW_TREE_LIST }], [], done);
+      testAction(
+        setShowTreeList,
+        { showTreeList: true },
+        {},
+        [{ type: types.SET_SHOW_TREE_LIST, payload: true }],
+        [],
+        done,
+      );
     });
 
     it('updates localStorage', () => {
       jest.spyOn(localStorage, 'setItem').mockImplementation(() => {});
 
-      toggleShowTreeList({ commit() {}, state: { showTreeList: true } });
+      setShowTreeList({ commit() {} }, { showTreeList: true });
 
       expect(localStorage.setItem).toHaveBeenCalledWith('mr_tree_show', true);
     });
@@ -917,7 +924,7 @@ describe('DiffsStoreActions', () => {
     it('does not update localStorage', () => {
       jest.spyOn(localStorage, 'setItem').mockImplementation(() => {});
 
-      toggleShowTreeList({ commit() {}, state: { showTreeList: true } }, false);
+      setShowTreeList({ commit() {} }, { showTreeList: true, saving: false });
 
       expect(localStorage.setItem).not.toHaveBeenCalled();
     });

spec/frontend/diffs/store/mutations_spec.js

@@ -621,15 +621,11 @@ describe('DiffsStoreMutations', () => {
     });
   });
 
-  describe('TOGGLE_SHOW_TREE_LIST', () => {
-    it('toggles showTreeList', () => {
+  describe('SET_SHOW_TREE_LIST', () => {
+    it('sets showTreeList', () => {
       const state = createState();
 
-      mutations[types.TOGGLE_SHOW_TREE_LIST](state);
-
-      expect(state.showTreeList).toBe(false, 'Failed to toggle showTreeList to false');
-
-      mutations[types.TOGGLE_SHOW_TREE_LIST](state);
+      mutations[types.SET_SHOW_TREE_LIST](state, true);
 
       expect(state.showTreeList).toBe(true, 'Failed to toggle showTreeList to true');
     });