Merge branch '48789-remove-event-listeners-scroll' into 'master'

Resolve "Improve performance of MR Changes tab: reduce event listeners on scroll event"

Closes #48639 and #48789

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

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

@@ -41,11 +41,6 @@ export default {
       required: true,
     },
   },
-  data() {
-    return {
-      activeFile: '',
-    };
-  },
   computed: {
     ...mapState({
       isLoading: state => state.diffs.isLoading,
@@ -126,14 +121,6 @@ export default {
         eventHub.$emit('fetchNotesData');
       }
     },
-    setActive(filePath) {
-      this.activeFile = filePath;
-    },
-    unsetActive(filePath) {
-      if (this.activeFile === filePath) {
-        this.activeFile = '';
-      }
-    },
     adjustView() {
       if (this.shouldShow && this.isParallelView) {
         window.mrTabs.expandViewContainer();
@@ -195,7 +182,6 @@ export default {
 
       <changed-files
         :diff-files="diffFiles"
-        :active-file="activeFile"
       />
 
       <div
@@ -207,8 +193,6 @@ export default {
           :key="file.newPath"
           :file="file"
           :current-user="currentUser"
-          @setActive="setActive(file.filePath)"
-          @unsetActive="unsetActive(file.filePath)"
         />
       </div>
       <no-changes v-else />

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

@@ -16,13 +16,6 @@ export default {
     ClipboardButton,
   },
   mixins: [changedFilesMixin],
-  props: {
-    activeFile: {
-      type: String,
-      required: false,
-      default: '',
-    },
-  },
   data() {
     return {
       isStuck: false,
@@ -70,7 +63,7 @@ export default {
     pluralize,
     handleScroll() {
       if (!this.updating) {
-        requestAnimationFrame(this.updateIsStuck);
+        this.$nextTick(this.updateIsStuck);
         this.updating = true;
       }
     },
@@ -148,25 +141,8 @@ export default {
           />
 
           <span
-            v-show="activeFile"
-            class="prepend-left-5"
-          >
-            <strong class="prepend-right-5">
-              {{ truncatedDiffPath(activeFile) }}
-            </strong>
-            <clipboard-button
-              :text="activeFile"
-              :title="s__('Copy file name to clipboard')"
-              tooltip-placement="bottom"
-              tooltip-container="body"
-              class="btn btn-default btn-transparent btn-clipboard"
-            />
-          </span>
-
-          <span
-            v-show="!isStuck"
-            id="diff-stats"
-            class="diff-stats-additions-deletions-expanded"
+            class="js-diff-stats-additions-deletions-expanded
+            diff-stats-additions-deletions-expanded"
           >
             with
             <strong class="cgreen">
@@ -177,6 +153,17 @@ export default {
               {{ pluralize(`${sumRemovedLines} deletion`, sumRemovedLines) }}
             </strong>
           </span>
+          <div
+            class="js-diff-stats-additions-deletions-collapsed
+            diff-stats-additions-deletions-collapsed float-right d-sm-none"
+          >
+            <strong class="cgreen">
+              +{{ sumAddedLines }}
+            </strong>
+            <strong class="cred">
+              -{{ sumRemovedLines }}
+            </strong>
+          </div>
         </div>
       </div>
     </div>

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

@@ -40,7 +40,7 @@ export default {
         {{ n__('%d changed file', '%d changed files', diffFiles.length) }}
       </span>
       <icon
-        :size="8"
+        class="caret-icon"
         name="chevron-down"
       />
     </button>

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

@@ -25,7 +25,6 @@ export default {
   },
   data() {
     return {
-      isActive: false,
       isLoadingCollapsedDiff: false,
       forkMessageVisible: false,
     };
@@ -48,12 +47,6 @@ export default {
       return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
     },
   },
-  mounted() {
-    document.addEventListener('scroll', this.handleScroll);
-  },
-  beforeDestroy() {
-    document.removeEventListener('scroll', this.handleScroll);
-  },
   methods: {
     ...mapActions('diffs', ['loadCollapsedDiff']),
     handleToggle() {
@@ -65,36 +58,6 @@ export default {
         this.file.collapsed = !this.file.collapsed;
       }
     },
-    handleScroll() {
-      if (!this.updating) {
-        requestAnimationFrame(this.scrollUpdate.bind(this));
-        this.updating = true;
-      }
-    },
-    scrollUpdate() {
-      const header = document.querySelector('.js-diff-files-changed');
-      if (!header) {
-        this.updating = false;
-        return;
-      }
-
-      const { top, bottom } = this.$el.getBoundingClientRect();
-      const { top: topOfFixedHeader, bottom: bottomOfFixedHeader } = header.getBoundingClientRect();
-
-      const headerOverlapsContent = top < topOfFixedHeader && bottom > bottomOfFixedHeader;
-      const fullyAboveHeader = bottom < bottomOfFixedHeader;
-      const fullyBelowHeader = top > topOfFixedHeader;
-
-      if (headerOverlapsContent && !this.isActive) {
-        this.$emit('setActive');
-        this.isActive = true;
-      } else if (this.isActive && (fullyAboveHeader || fullyBelowHeader)) {
-        this.$emit('unsetActive');
-        this.isActive = false;
-      }
-
-      this.updating = false;
-    },
     handleLoadCollapsedDiff() {
       this.isLoadingCollapsedDiff = true;
 

app/assets/stylesheets/pages/diff.scss

@@ -518,6 +518,12 @@
     outline: none;
     color: $gl-link-hover-color;
   }
+
+  .caret-icon {
+    position: relative;
+    top: 2px;
+    left: -1px;
+  }
 }
 
 // Mobile

changelogs/unreleased/48789-remove-event-listeners-scroll.yml

+---
+title: Improves performance on Merge Request diff tab by removing the scroll event
+  listeners being added to every file
+merge_request:
+author:
+type: performance

locale/gitlab.pot

@@ -1666,9 +1666,6 @@ msgstr ""
 msgid "Copy commit SHA to clipboard"
 msgstr ""
 
-msgid "Copy file name to clipboard"
-msgstr ""
-
 msgid "Copy file path to clipboard"
 msgstr ""