Merge branch 'ph/330125/virtualScrollToFile' into 'master'

Fixes links to diff files not working with virtual scroller

See merge request gitlab-org/gitlab!62672

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

@@ -42,6 +42,7 @@ import {
   TRACKING_MULTIPLE_FILES_MODE,
 } from '../constants';
 
+import diffsEventHub from '../event_hub';
 import { reviewStatuses } from '../utils/file_reviews';
 import { diffsApp } from '../utils/performance';
 import { fileByFile } from '../utils/preferences';
@@ -52,7 +53,9 @@ import DiffFile from './diff_file.vue';
 import HiddenFilesWarning from './hidden_files_warning.vue';
 import MergeConflictWarning from './merge_conflict_warning.vue';
 import NoChanges from './no_changes.vue';
+import PreRenderer from './pre_renderer.vue';
 import TreeList from './tree_list.vue';
+import VirtualScrollerScrollSync from './virtual_scroller_scroll_sync';
 
 export default {
   name: 'DiffsApp',
@@ -71,6 +74,8 @@ export default {
     GlSprintf,
     DynamicScroller,
     DynamicScrollerItem,
+    PreRenderer,
+    VirtualScrollerScrollSync,
   },
   alerts: {
     ALERT_OVERFLOW_HIDDEN,
@@ -166,6 +171,7 @@ export default {
     return {
       treeWidth,
       diffFilesLength: 0,
+      virtualScrollCurrentIndex: -1,
     };
   },
   computed: {
@@ -323,6 +329,11 @@ export default {
       this.setHighlightedRow(id.split('diff-content').pop().slice(1));
     }
 
+    if (window.gon?.features?.diffsVirtualScrolling) {
+      diffsEventHub.$on('scrollToFileHash', this.scrollVirtualScrollerToFileHash);
+      diffsEventHub.$on('scrollToIndex', this.scrollVirtualScrollerToIndex);
+    }
+
     if (window.gon?.features?.diffSettingsUsageData) {
       if (this.renderTreeList) {
         api.trackRedisHllUserEvent(TRACKING_FILE_BROWSER_TREE);
@@ -377,6 +388,11 @@ export default {
     diffsApp.deinstrument();
     this.unsubscribeFromEvents();
     this.removeEventListeners();
+
+    if (window.gon?.features?.diffsVirtualScrolling) {
+      diffsEventHub.$off('scrollToFileHash', this.scrollVirtualScrollerToFileHash);
+      diffsEventHub.$off('scrollToIndex', this.scrollVirtualScrollerToIndex);
+    }
   },
   methods: {
     ...mapActions(['startTaskList']),
@@ -508,6 +524,20 @@ export default {
 
       return this.setShowTreeList({ showTreeList, saving: false });
     },
+    async scrollVirtualScrollerToFileHash(hash) {
+      const index = this.diffFiles.findIndex((f) => f.file_hash === hash);
+
+      if (index !== -1) {
+        this.scrollVirtualScrollerToIndex(index);
+      }
+    },
+    async scrollVirtualScrollerToIndex(index) {
+      this.virtualScrollCurrentIndex = index;
+
+      await this.$nextTick();
+
+      this.virtualScrollCurrentIndex = -1;
+    },
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,
@@ -572,12 +602,29 @@ export default {
           <template v-else-if="renderDiffFiles">
             <dynamic-scroller
               v-if="isVirtualScrollingEnabled"
+              ref="virtualScroller"
               :items="diffs"
               :min-item-size="70"
               :buffer="1000"
               :use-transform="false"
               page-mode
             >
+              <template #default="{ item, index, active }">
+                <dynamic-scroller-item :item="item" :active="active" :class="{ active }">
+                  <diff-file
+                    :file="item"
+                    :reviewed="fileReviews[item.id]"
+                    :is-first-file="index === 0"
+                    :is-last-file="index === diffFilesLength - 1"
+                    :help-page-path="helpPagePath"
+                    :can-current-user-fork="canCurrentUserFork"
+                    :view-diffs-file-by-file="viewDiffsFileByFile"
+                    :active="active"
+                  />
+                </dynamic-scroller-item>
+              </template>
+              <template #before>
+                <pre-renderer :max-length="diffFilesLength">
                   <template #default="{ item, index, active }">
                     <dynamic-scroller-item :item="item" :active="active">
                       <diff-file
@@ -588,9 +635,13 @@ export default {
                         :help-page-path="helpPagePath"
                         :can-current-user-fork="canCurrentUserFork"
                         :view-diffs-file-by-file="viewDiffsFileByFile"
+                        pre-render
                       />
                     </dynamic-scroller-item>
                   </template>
+                </pre-renderer>
+                <virtual-scroller-scroll-sync :index="virtualScrollCurrentIndex" />
+              </template>
             </dynamic-scroller>
             <template v-else>
               <diff-file

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

@@ -68,6 +68,16 @@ export default {
       type: Boolean,
       required: true,
     },
+    active: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+    preRender: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
@@ -156,6 +166,8 @@ export default {
   watch: {
     'file.id': {
       handler: function fileIdHandler() {
+        if (this.preRender) return;
+
         this.manageViewedEffects();
       },
     },
@@ -163,7 +175,7 @@ export default {
       handler: function hashChangeWatch(newHash, oldHash) {
         this.isCollapsed = isCollapsed(this.file);
 
-        if (newHash && oldHash && !this.hasDiff) {
+        if (newHash && oldHash && !this.hasDiff && !this.preRender) {
           this.requestDiff();
         }
       },
@@ -187,10 +199,14 @@ export default {
     },
   },
   created() {
+    if (this.preRender) return;
+
     notesEventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff);
     eventHub.$on(EVT_EXPAND_ALL_FILES, this.expandAllListener);
   },
   mounted() {
+    if (this.preRender) return;
+
     if (this.hasDiff) {
       this.postRender();
     }
@@ -198,6 +214,8 @@ export default {
     this.manageViewedEffects();
   },
   beforeDestroy() {
+    if (this.preRender) return;
+
     eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener);
   },
   methods: {
@@ -287,7 +305,7 @@ export default {
 
 <template>
   <div
-    :id="file.file_hash"
+    :id="!preRender && active && file.file_hash"
     :class="{
       'is-active': currentDiffFileId === file.file_hash,
       'comments-disabled': Boolean(file.brokenSymlink),
@@ -330,7 +348,7 @@ export default {
     </div>
     <template v-else>
       <div
-        :id="`diff-content-${file.file_hash}`"
+        :id="!preRender && active && `diff-content-${file.file_hash}`"
         :class="hasBodyClasses.contentByHash"
         data-testid="content-area"
       >

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

+<script>
+export default {
+  inject: ['vscrollParent'],
+  props: {
+    maxLength: {
+      type: Number,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      nextIndex: -1,
+      nextItem: null,
+      startedRender: false,
+      width: 0,
+    };
+  },
+  mounted() {
+    this.width = this.$el.parentNode.offsetWidth;
+
+    this.$_itemsWithSizeWatcher = this.$watch('vscrollParent.itemsWithSize', async () => {
+      await this.$nextTick();
+
+      const nextItem = this.findNextToRender();
+
+      if (nextItem) {
+        this.startedRender = true;
+        requestIdleCallback(() => {
+          this.nextItem = nextItem;
+        });
+      } else if (this.startedRender) {
+        this.clearRendering();
+      }
+    });
+  },
+  beforeDestroy() {
+    this.$_itemsWithSizeWatcher();
+  },
+  methods: {
+    clearRendering() {
+      this.nextItem = null;
+
+      if (this.maxLength === this.vscrollParent.itemsWithSize.length) {
+        this.$_itemsWithSizeWatcher();
+      }
+    },
+    findNextToRender() {
+      return this.vscrollParent.itemsWithSize.find(({ size }, index) => {
+        const isNext = size === 0;
+
+        if (isNext) {
+          this.nextIndex = index;
+        }
+
+        return isNext;
+      });
+    },
+  },
+};
+</script>
+
+<template>
+  <div v-if="nextItem" :style="{ width: `${width}px` }" class="gl-absolute diff-file-offscreen">
+    <slot
+      v-bind="{ item: nextItem.item, index: nextIndex, active: true, itemWithSize: nextItem }"
+    ></slot>
+  </div>
+</template>
+
+<style scoped>
+.diff-file-offscreen {
+  top: -200%;
+  left: -200%;
+}
+</style>

app/assets/javascripts/diffs/components/virtual_scroller_scroll_sync.js

+import { handleLocationHash } from '~/lib/utils/common_utils';
+
+export default {
+  inject: ['vscrollParent'],
+  props: {
+    index: {
+      type: Number,
+      required: true,
+    },
+  },
+  watch: {
+    index: {
+      handler() {
+        const { index } = this;
+
+        if (index < 0) return;
+
+        if (this.vscrollParent.itemsWithSize[index].size) {
+          this.scrollToIndex(index);
+        } else {
+          this.$_itemsWithSizeWatcher = this.$watch('vscrollParent.itemsWithSize', async () => {
+            await this.$nextTick();
+
+            if (this.vscrollParent.itemsWithSize[index].size) {
+              this.$_itemsWithSizeWatcher();
+              this.scrollToIndex(index);
+
+              await this.$nextTick();
+            }
+          });
+        }
+      },
+      immediate: true,
+    },
+  },
+  beforeDestroy() {
+    if (this.$_itemsWithSizeWatcher) this.$_itemsWithSizeWatcher();
+  },
+  methods: {
+    scrollToIndex(index) {
+      this.vscrollParent.scrollToItem(index);
+
+      setTimeout(() => {
+        handleLocationHash();
+      });
+    },
+  },
+  render(h) {
+    return h(null);
+  },
+};

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

@@ -100,7 +100,9 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
     w: state.showWhitespace ? '0' : '1',
     view: 'inline',
   };
+  const hash = window.location.hash.replace('#', '').split('diff-content-').pop();
   let totalLoaded = 0;
+  let scrolledVirtualScroller = false;
 
   commit(types.SET_BATCH_LOADING, true);
   commit(types.SET_RETRIEVING_BATCHES, true);
@@ -115,6 +117,15 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
         commit(types.SET_DIFF_DATA_BATCH, { diff_files });
         commit(types.SET_BATCH_LOADING, false);
 
+        if (window.gon?.features?.diffsVirtualScrolling && !scrolledVirtualScroller) {
+          const index = state.diffFiles.findIndex((f) => f.file_hash === hash);
+
+          if (index >= 0) {
+            eventHub.$emit('scrollToIndex', index);
+            scrolledVirtualScroller = true;
+          }
+        }
+
         if (!isNoteLink && !state.currentDiffFileId) {
           commit(types.VIEW_DIFF_FILE, diff_files[0].file_hash);
         }
@@ -171,7 +182,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
       .catch(() => commit(types.SET_RETRIEVING_BATCHES, false));
 
   return getBatch()
-    .then(handleLocationHash)
+    .then(() => !window.gon?.features?.diffsVirtualScrolling && handleLocationHash())
     .catch(() => null);
 };
 
@@ -510,9 +521,18 @@ export const scrollToFile = ({ state, commit }, path) => {
   if (!state.treeEntries[path]) return;
 
   const { fileHash } = state.treeEntries[path];
-  document.location.hash = fileHash;
 
   commit(types.VIEW_DIFF_FILE, fileHash);
+
+  if (window.gon?.features?.diffsVirtualScrolling) {
+    eventHub.$emit('scrollToFileHash', fileHash);
+
+    setTimeout(() => {
+      window.history.replaceState(null, null, `#${fileHash}`);
+    });
+  } else {
+    document.location.hash = fileHash;
+  }
 };
 
 export const setShowTreeList = ({ commit }, { showTreeList, saving = true }) => {

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

@@ -381,9 +381,15 @@ function prepareDiffFileLines(file) {
 }
 
 function finalizeDiffFile(file, index) {
+  let renderIt = Boolean(window.gon?.features?.diffsVirtualScrolling);
+
+  if (!window.gon?.features?.diffsVirtualScrolling) {
+    renderIt =
+      index < 3 ? file[INLINE_DIFF_LINES_KEY].length < LINES_TO_BE_RENDERED_DIRECTLY : false;
+  }
+
   Object.assign(file, {
-    renderIt:
-      index < 3 ? file[INLINE_DIFF_LINES_KEY].length < LINES_TO_BE_RENDERED_DIRECTLY : false,
+    renderIt,
     isShowingFullFile: false,
     isLoadingFullFile: false,
     discussions: [],

app/assets/stylesheets/framework/diffs.scss

@@ -1188,3 +1188,9 @@ table.code {
     margin-top: 0;
   }
 }
+
+// Note: Prevents tall files from appearing above sticky tabs
+.diffs .vue-recycle-scroller__item-view > div:not(.active) {
+  position: absolute;
+  top: 100vh;
+}

vendor/assets/javascripts/vue-virtual-scroller/src/components/DynamicScrollerItem.vue

@@ -195,14 +195,15 @@ export default {
 
     observeSize () {
       if (!this.vscrollResizeObserver) return
-      this.vscrollResizeObserver.observe(this.$el.parentNode)
-      this.$el.parentNode.addEventListener('resize', this.onResize)
+      this.$_parentNode = this.$el.parentNode;
+      this.vscrollResizeObserver.observe(this.$_parentNode)
+      this.$_parentNode.addEventListener('resize', this.onResize)
     },
 
     unobserveSize () {
       if (!this.vscrollResizeObserver) return
-      this.vscrollResizeObserver.unobserve(this.$el.parentNode)
-      this.$el.parentNode.removeEventListener('resize', this.onResize)
+      this.vscrollResizeObserver.unobserve(this.$_parentNode)
+      this.$_parentNode.removeEventListener('resize', this.onResize)
     },
 
     onResize (event) {

vendor/assets/javascripts/vue-virtual-scroller/src/components/RecycleScroller.vue

@@ -572,20 +572,49 @@ export default {
     },
 
     scrollToItem (index) {
-      let scroll
+      this.$_scrollDirty = true
+      const { viewport, scrollDirection, scrollDistance } = this.scrollToPosition(index)
+      viewport[scrollDirection] = scrollDistance
+
+      setTimeout(() => {
+        this.$_scrollDirty = false
+        this.updateVisibleItems(false, true)
+      })
+    },
+
+    scrollToPosition (index) {
+      const getPositionOfItem = (index) => {
         if (this.itemSize === null) {
-        scroll = index > 0 ? this.sizes[index - 1].accumulator : 0
+          return index > 0 ? this.sizes[index - 1].accumulator : 0
         } else {
-        scroll = index * this.itemSize
+          return index * this.itemSize
         }
-      this.scrollToPosition(scroll)
-    },
+      }
+      const position = getPositionOfItem(index)
+      const direction = this.direction === 'vertical'
+        ? { scroll: 'scrollTop', start: 'top' }
+        : { scroll: 'scrollLeft', start: 'left' }
 
-    scrollToPosition (position) {
-      if (this.direction === 'vertical') {
-        this.$el.scrollTop = position
-      } else {
-        this.$el.scrollLeft = position
+      if (this.pageMode) {
+        const viewportEl = ScrollParent(this.$el)
+        // HTML doesn't overflow like other elements
+        const scrollTop = viewportEl.tagName === 'HTML' ? 0 : viewportEl[direction.scroll]
+        const viewport = viewportEl.getBoundingClientRect()
+
+        const scroller = this.$el.getBoundingClientRect()
+        const scrollerPosition = scroller[direction.start] - viewport[direction.start]
+
+        return {
+          viewport: viewportEl,
+          scrollDirection: direction.scroll,
+          scrollDistance: position + scrollTop + scrollerPosition,
+        }
+      }
+
+      return {
+        viewport: this.$el,
+        scrollDirection: direction.scroll,
+        scrollDistance: position,
       }
     },