Merge branch 'ph/fixDiffStateWithVirtualScroller' into 'master'

Fixed diff file states getting incorrectly set

See merge request gitlab-org/gitlab!65069

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

@@ -2,6 +2,7 @@
 import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml, GlSprintf } from '@gitlab/ui';
 import { escape } from 'lodash';
 import { mapActions, mapGetters, mapState } from 'vuex';
+import { IdState } from 'vendor/vue-virtual-scroller';
 import createFlash from '~/flash';
 import { hasDiff } from '~/helpers/diffs_helper';
 import { diffViewerErrors } from '~/ide/constants';
@@ -34,7 +35,7 @@ export default {
   directives: {
     SafeHtml,
   },
-  mixins: [glFeatureFlagsMixin()],
+  mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.file.file_hash })],
   props: {
     file: {
       type: Object,
@@ -79,7 +80,7 @@ export default {
       default: false,
     },
   },
-  data() {
+  idState() {
     return {
       isLoadingCollapsedDiff: false,
       forkMessageVisible: false,
@@ -101,7 +102,9 @@ export default {
       return getShortShaFromFile(this.file);
     },
     showLoadingIcon() {
-      return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
+      return (
+        this.idState.isLoadingCollapsedDiff || (!this.file.renderIt && !this.idState.isCollapsed)
+      );
     },
     hasDiff() {
       return hasDiff(this.file);
@@ -145,13 +148,13 @@ export default {
       return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE;
     },
     showBody() {
-      return !this.isCollapsed || this.automaticallyCollapsed;
+      return !this.idState.isCollapsed || this.automaticallyCollapsed;
     },
     showWarning() {
-      return this.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile;
+      return this.idState.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile;
     },
     showContent() {
-      return !this.isCollapsed && !this.isFileTooLarge;
+      return !this.idState.isCollapsed && !this.isFileTooLarge;
     },
     showLocalFileReviews() {
       const loggedIn = Boolean(gon.current_user_id);
@@ -179,23 +182,16 @@ export default {
           this.requestDiff();
         }
       },
-      immediate: true,
     },
     'file.viewer.automaticallyCollapsed': {
       handler: function autoChangeWatch(automaticValue) {
-        if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) {
-          this.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
-        }
+        this.handleAutomaticallyCollapsed(automaticValue);
       },
-      immediate: true,
     },
     'file.viewer.manuallyCollapsed': {
       handler: function manualChangeWatch(manualValue) {
-        if (manualValue !== null) {
-          this.isCollapsed = manualValue;
-        }
+        this.handleManualChangeWatch(manualValue);
       },
-      immediate: true,
     },
   },
   created() {
@@ -212,6 +208,8 @@ export default {
     }
 
     this.manageViewedEffects();
+    this.handleAutomaticallyCollapsed(this.file.viewer.automaticallyCollapsed);
+    this.handleManualChangeWatch(this.file.viewer.manuallyCollapsed);
   },
   beforeDestroy() {
     if (this.preRender) return;
@@ -226,12 +224,12 @@ export default {
       'setFileCollapsedByUser',
     ]),
     manageViewedEffects() {
-      if (this.reviewed && !this.isCollapsed && this.showLocalFileReviews) {
+      if (this.reviewed && !this.idState.isCollapsed && this.showLocalFileReviews) {
         this.handleToggle();
       }
     },
     expandAllListener() {
-      if (this.isCollapsed) {
+      if (this.idState.isCollapsed) {
         this.handleToggle();
       }
     },
@@ -253,7 +251,7 @@ export default {
       });
     },
     handleToggle({ viaUserInteraction = false } = {}) {
-      const collapsingNow = !this.isCollapsed;
+      const collapsingNow = !this.idState.isCollapsed;
       const contentElement = this.$el.querySelector(`#diff-content-${this.file.file_hash}`);
 
       this.setFileCollapsedByUser({
@@ -270,11 +268,11 @@ export default {
       }
     },
     requestDiff() {
-      this.isLoadingCollapsedDiff = true;
+      this.idState.isLoadingCollapsedDiff = true;
 
       this.loadCollapsedDiff(this.file)
         .then(() => {
-          this.isLoadingCollapsedDiff = false;
+          this.idState.isLoadingCollapsedDiff = false;
           this.setRenderIt(this.file);
         })
         .then(() => {
@@ -287,17 +285,27 @@ export default {
           );
         })
         .catch(() => {
-          this.isLoadingCollapsedDiff = false;
+          this.idState.isLoadingCollapsedDiff = false;
           createFlash({
             message: this.$options.i18n.genericError,
           });
         });
     },
     showForkMessage() {
-      this.forkMessageVisible = true;
+      this.idState.forkMessageVisible = true;
     },
     hideForkMessage() {
-      this.forkMessageVisible = false;
+      this.idState.forkMessageVisible = false;
+    },
+    handleAutomaticallyCollapsed(automaticValue) {
+      if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) {
+        this.idState.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
+      }
+    },
+    handleManualChangeWatch(manualValue) {
+      if (manualValue !== null) {
+        this.idState.isCollapsed = manualValue;
+      }
     },
   },
 };
@@ -320,7 +328,7 @@ export default {
       :diff-file="file"
       :collapsible="true"
       :reviewed="reviewed"
-      :expanded="!isCollapsed"
+      :expanded="!idState.isCollapsed"
       :add-merge-request-buttons="true"
       :view-diffs-file-by-file="viewDiffsFileByFile"
       :show-local-file-reviews="showLocalFileReviews"
@@ -331,7 +339,10 @@ export default {
       @showForkMessage="showForkMessage"
     />
 
-    <div v-if="forkMessageVisible" class="js-file-fork-suggestion-section file-fork-suggestion">
+    <div
+      v-if="idState.forkMessageVisible"
+      class="js-file-fork-suggestion-section file-fork-suggestion"
+    >
       <span v-safe-html="forkMessage" class="file-fork-suggestion-note"></span>
       <a
         :href="file.fork_path"

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

@@ -13,6 +13,7 @@ import {
 } from '@gitlab/ui';
 import { escape } from 'lodash';
 import { mapActions, mapGetters, mapState } from 'vuex';
+import { IdState } from 'vendor/vue-virtual-scroller';
 import { diffViewerModes } from '~/ide/constants';
 import { scrollToElement } from '~/lib/utils/common_utils';
 import { truncateSha } from '~/lib/utils/text_utility';
@@ -47,7 +48,7 @@ export default {
     GlTooltip: GlTooltipDirective,
     SafeHtml: GlSafeHtmlDirective,
   },
-  mixins: [glFeatureFlagsMixin()],
+  mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.diffFile.file_hash })],
   i18n: {
     ...DIFF_FILE_HEADER,
     compareButtonLabel: s__('Compare submodule commit revisions'),
@@ -102,7 +103,7 @@ export default {
       default: () => [],
     },
   },
-  data() {
+  idState() {
     return {
       moreActionsShown: false,
     };
@@ -239,7 +240,7 @@ export default {
       }
     },
     setMoreActionsShown(val) {
-      this.moreActionsShown = val;
+      this.idState.moreActionsShown = val;
     },
     toggleReview(newReviewedStatus) {
       const autoCollapsed =
@@ -268,7 +269,7 @@ export default {
 <template>
   <div
     ref="header"
-    :class="{ 'gl-z-dropdown-menu!': moreActionsShown }"
+    :class="{ 'gl-z-dropdown-menu!': idState.moreActionsShown }"
     class="js-file-title file-title file-title-flex-parent"
     data-qa-selector="file_title_container"
     :data-qa-file-name="filePath"

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

 <script>
 import { mapGetters, mapState, mapActions } from 'vuex';
+import { IdState } from 'vendor/vue-virtual-scroller';
 import DraftNote from '~/batch_comments/components/draft_note.vue';
 import draftCommentsMixin from '~/diffs/mixins/draft_comments';
 import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
@@ -17,7 +18,11 @@ export default {
     DiffCommentCell,
     DraftNote,
   },
-  mixins: [draftCommentsMixin, glFeatureFlagsMixin()],
+  mixins: [
+    draftCommentsMixin,
+    glFeatureFlagsMixin(),
+    IdState({ idProp: (vm) => vm.diffFile.file_hash }),
+  ],
   props: {
     diffFile: {
       type: Object,
@@ -38,7 +43,7 @@ export default {
       default: false,
     },
   },
-  data() {
+  idState() {
     return {
       dragStart: null,
       updatedLineRange: null,
@@ -80,29 +85,29 @@ export default {
       if (event.target?.parentNode) {
         hide(event.target.parentNode);
       }
-      this.dragStart = line;
+      this.idState.dragStart = line;
     },
     onDragOver(line) {
-      if (line.chunk !== this.dragStart.chunk) return;
+      if (line.chunk !== this.idState.dragStart.chunk) return;
 
-      let start = this.dragStart;
+      let start = this.idState.dragStart;
       let end = line;
 
-      if (this.dragStart.index >= line.index) {
+      if (this.idState.dragStart.index >= line.index) {
         start = line;
-        end = this.dragStart;
+        end = this.idState.dragStart;
       }
 
-      this.updatedLineRange = { start, end };
+      this.idState.updatedLineRange = { start, end };
 
-      this.setSelectedCommentPosition(this.updatedLineRange);
+      this.setSelectedCommentPosition(this.idState.updatedLineRange);
     },
     onStopDragging() {
       this.showCommentForm({
-        lineCode: this.updatedLineRange?.end?.line_code,
+        lineCode: this.idState.updatedLineRange?.end?.line_code,
         fileHash: this.diffFile.file_hash,
       });
-      this.dragStart = null;
+      this.idState.dragStart = null;
     },
     isHighlighted(line) {
       return isHighlighted(

spec/frontend/diffs/components/diff_view_spec.js

@@ -41,7 +41,7 @@ describe('DiffView', () => {
     });
 
     const propsData = {
-      diffFile: {},
+      diffFile: { file_hash: '123' },
       diffLines: [],
       ...props,
     };
@@ -85,7 +85,7 @@ describe('DiffView', () => {
       const wrapper = createWrapper({ diffLines: [{}] });
 
       wrapper.findComponent(DiffRow).vm.$emit('startdragging', { line: { test: true } });
-      expect(wrapper.vm.dragStart).toEqual({ test: true });
+      expect(wrapper.vm.idState.dragStart).toEqual({ test: true });
     });
 
     it('does not call `setSelectedCommentPosition` on different chunks onDragOver', () => {
@@ -123,10 +123,10 @@ describe('DiffView', () => {
       const diffRow = getDiffRow(wrapper);
 
       diffRow.$emit('startdragging', { line: { test: true } });
-      expect(wrapper.vm.dragStart).toEqual({ test: true });
+      expect(wrapper.vm.idState.dragStart).toEqual({ test: true });
 
       diffRow.$emit('stopdragging');
-      expect(wrapper.vm.dragStart).toBeNull();
+      expect(wrapper.vm.idState.dragStart).toBeNull();
       expect(showCommentForm).toHaveBeenCalled();
     });
   });