Update scroll position to synchronize the Jump To Next button

When you scroll a discussion halfway up the page, the
Jump To Next Unresolved Discussion button will go to the
discussion after that one.

When you scroll the page such that a discussion scrolls down
through the middle of the page and below it, the Jump button
considers that discussion the "next" in line to be seen, and
will scroll to it.

Changelog: added

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

@@ -44,6 +44,7 @@ import {
   TRACKING_MULTIPLE_FILES_MODE,
 } from '../constants';
 
+import { discussionIntersectionObserverHandlerFactory } from '../utils/discussions';
 import diffsEventHub from '../event_hub';
 import { reviewStatuses } from '../utils/file_reviews';
 import { diffsApp } from '../utils/performance';
@@ -86,6 +87,9 @@ export default {
     ALERT_MERGE_CONFLICT,
     ALERT_COLLAPSED_FILES,
   },
+  provide: {
+    discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+  },
   props: {
     endpoint: {
       type: String,

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

+function normalize(processable) {
+  const { entry } = processable;
+  const offset = entry.rootBounds.bottom - entry.boundingClientRect.top;
+  const direction =
+    offset < 0 ? 'Up' : 'Down'; /* eslint-disable-line @gitlab/require-i18n-strings */
+
+  return {
+    ...processable,
+    entry: {
+      time: entry.time,
+      type: entry.isIntersecting ? 'intersection' : `scroll${direction}`,
+    },
+  };
+}
+
+function sort({ entry: alpha }, { entry: beta }) {
+  const diff = alpha.time - beta.time;
+  let order = 0;
+
+  if (diff < 0) {
+    order = -1;
+  } else if (diff > 0) {
+    order = 1;
+  } else if (alpha.type === 'intersection' && beta.type === 'scrollUp') {
+    order = 2;
+  } else if (alpha.type === 'scrollUp' && beta.type === 'intersection') {
+    order = -2;
+  }
+
+  return order;
+}
+
+function filter(entry) {
+  return entry.type !== 'scrollDown';
+}
+
+export function discussionIntersectionObserverHandlerFactory() {
+  let unprocessed = [];
+  let timer = null;
+
+  return (processable) => {
+    unprocessed.push(processable);
+
+    if (timer) {
+      clearTimeout(timer);
+    }
+
+    timer = setTimeout(() => {
+      unprocessed
+        .map(normalize)
+        .filter(filter)
+        .sort(sort)
+        .forEach((discussionObservationContainer) => {
+          const {
+            entry: { type },
+            currentDiscussion,
+            isFirstUnresolved,
+            isDiffsPage,
+            functions: { setCurrentDiscussionId, getPreviousUnresolvedDiscussionId },
+          } = discussionObservationContainer;
+
+          if (type === 'intersection') {
+            setCurrentDiscussionId(currentDiscussion.id);
+          } else if (type === 'scrollUp') {
+            setCurrentDiscussionId(
+              isFirstUnresolved
+                ? null
+                : getPreviousUnresolvedDiscussionId(currentDiscussion.id, isDiffsPage),
+            );
+          }
+        });
+
+      unprocessed = [];
+    }, 0);
+  };
+}

app/assets/javascripts/notes/components/discussion_notes.vue

 <script>
 import { mapGetters, mapActions } from 'vuex';
+import { GlIntersectionObserver } from '@gitlab/ui';
 import { __ } from '~/locale';
 import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue';
 import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue';
@@ -16,7 +17,9 @@ export default {
     ToggleRepliesWidget,
     NoteEditedText,
     DiscussionNotesRepliesWrapper,
+    GlIntersectionObserver,
   },
+  inject: ['discussionObserverHandler'],
   props: {
     discussion: {
       type: Object,
@@ -54,7 +57,11 @@ export default {
     },
   },
   computed: {
-    ...mapGetters(['userCanReply']),
+    ...mapGetters([
+      'userCanReply',
+      'previousUnresolvedDiscussionId',
+      'firstUnresolvedDiscussionId',
+    ]),
     hasReplies() {
       return Boolean(this.replies.length);
     },
@@ -77,9 +84,20 @@ export default {
         url: this.discussion.discussion_path,
       };
     },
+    isFirstUnresolved() {
+      return this.firstUnresolvedDiscussionId === this.discussion.id;
+    },
+  },
+  observerOptions: {
+    threshold: 0,
+    rootMargin: '0px 0px -50% 0px',
   },
   methods: {
-    ...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']),
+    ...mapActions([
+      'toggleDiscussion',
+      'setSelectedCommentPositionHover',
+      'setCurrentDiscussionId',
+    ]),
     componentName(note) {
       if (note.isPlaceholderNote) {
         if (note.placeholderType === SYSTEM_NOTE) {
@@ -110,6 +128,18 @@ export default {
         this.setSelectedCommentPositionHover();
       }
     },
+    observerTriggered(entry) {
+      this.discussionObserverHandler({
+        entry,
+        isFirstUnresolved: this.isFirstUnresolved,
+        currentDiscussion: { ...this.discussion },
+        isDiffsPage: !this.isOverviewTab,
+        functions: {
+          setCurrentDiscussionId: this.setCurrentDiscussionId,
+          getPreviousUnresolvedDiscussionId: this.previousUnresolvedDiscussionId,
+        },
+      });
+    },
   },
 };
 </script>
@@ -122,33 +152,35 @@ export default {
       @mouseleave="handleMouseLeave(discussion)"
     >
       <template v-if="shouldGroupReplies">
-        <component
-          :is="componentName(firstNote)"
-          :note="componentData(firstNote)"
-          :line="line || diffLine"
-          :discussion-file="discussion.diff_file"
-          :commit="commit"
-          :help-page-path="helpPagePath"
-          :show-reply-button="userCanReply"
-          :discussion-root="true"
-          :discussion-resolve-path="discussion.resolve_path"
-          :is-overview-tab="isOverviewTab"
-          @handleDeleteNote="$emit('deleteNote')"
-          @startReplying="$emit('startReplying')"
-        >
-          <template #discussion-resolved-text>
-            <note-edited-text
-              v-if="discussion.resolved"
-              :edited-at="discussion.resolved_at"
-              :edited-by="discussion.resolved_by"
-              :action-text="resolvedText"
-              class-name="discussion-headline-light js-discussion-headline discussion-resolved-text"
-            />
-          </template>
-          <template #avatar-badge>
-            <slot name="avatar-badge"></slot>
-          </template>
-        </component>
+        <gl-intersection-observer :options="$options.observerOptions" @update="observerTriggered">
+          <component
+            :is="componentName(firstNote)"
+            :note="componentData(firstNote)"
+            :line="line || diffLine"
+            :discussion-file="discussion.diff_file"
+            :commit="commit"
+            :help-page-path="helpPagePath"
+            :show-reply-button="userCanReply"
+            :discussion-root="true"
+            :discussion-resolve-path="discussion.resolve_path"
+            :is-overview-tab="isOverviewTab"
+            @handleDeleteNote="$emit('deleteNote')"
+            @startReplying="$emit('startReplying')"
+          >
+            <template #discussion-resolved-text>
+              <note-edited-text
+                v-if="discussion.resolved"
+                :edited-at="discussion.resolved_at"
+                :edited-by="discussion.resolved_by"
+                :action-text="resolvedText"
+                class-name="discussion-headline-light js-discussion-headline discussion-resolved-text"
+              />
+            </template>
+            <template #avatar-badge>
+              <slot name="avatar-badge"></slot>
+            </template>
+          </component>
+        </gl-intersection-observer>
         <discussion-notes-replies-wrapper :is-diff-discussion="discussion.diff_discussion">
           <toggle-replies-widget
             v-if="hasReplies"

spec/frontend/diffs/components/diff_discussions_spec.js

 import { GlIcon } from '@gitlab/ui';
 import { mount, createLocalVue } from '@vue/test-utils';
 import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
+import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions';
 import { createStore } from '~/mr_notes/stores';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import NoteableDiscussion from '~/notes/components/noteable_discussion.vue';
@@ -19,6 +20,9 @@ describe('DiffDiscussions', () => {
     store = createStore();
     wrapper = mount(localVue.extend(DiffDiscussions), {
       store,
+      provide: {
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: {
         discussions: getDiscussionsMockData(),
         ...props,

spec/frontend/diffs/utils/discussions_spec.js

+import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions';
+
+describe('Diff Discussions Utils', () => {
+  describe('discussionIntersectionObserverHandlerFactory', () => {
+    it('creates a handler function', () => {
+      expect(discussionIntersectionObserverHandlerFactory()).toBeInstanceOf(Function);
+    });
+
+    describe('intersection observer handler', () => {
+      const functions = {
+        setCurrentDiscussionId: jest.fn(),
+        getPreviousUnresolvedDiscussionId: jest.fn().mockImplementation((id) => {
+          return Number(id) - 1;
+        }),
+      };
+      const defaultProcessableWrapper = {
+        entry: {
+          time: 0,
+          isIntersecting: true,
+          rootBounds: {
+            bottom: 0,
+          },
+          boundingClientRect: {
+            top: 0,
+          },
+        },
+        currentDiscussion: {
+          id: 1,
+        },
+        isFirstUnresolved: false,
+        isDiffsPage: true,
+      };
+      let handler;
+      let getMock;
+      let setMock;
+
+      beforeEach(() => {
+        functions.setCurrentDiscussionId.mockClear();
+        functions.getPreviousUnresolvedDiscussionId.mockClear();
+
+        defaultProcessableWrapper.functions = functions;
+
+        setMock = functions.setCurrentDiscussionId.mock;
+        getMock = functions.getPreviousUnresolvedDiscussionId.mock;
+        handler = discussionIntersectionObserverHandlerFactory();
+      });
+
+      it('debounces multiple simultaneous requests into one queue', () => {
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+
+        expect(setTimeout).toHaveBeenCalledTimes(4);
+        expect(clearTimeout).toHaveBeenCalledTimes(3);
+
+        // By only advancing to one timer, we ensure it's all being batched into one queue
+        jest.advanceTimersToNextTimer();
+
+        expect(functions.setCurrentDiscussionId).toHaveBeenCalledTimes(4);
+      });
+
+      it('properly processes, sorts and executes the correct actions for a set of observed intersections', () => {
+        handler(defaultProcessableWrapper);
+        handler({
+          // This observation is here to be filtered out because it's a scrollDown
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            isIntersecting: false,
+            boundingClientRect: { top: 10 },
+            rootBounds: { bottom: 100 },
+          },
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 101,
+            isIntersecting: false,
+            rootBounds: { bottom: -100 },
+          },
+          currentDiscussion: { id: 20 },
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 100,
+            isIntersecting: false,
+            boundingClientRect: { top: 100 },
+          },
+          currentDiscussion: { id: 30 },
+          isDiffsPage: false,
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          isFirstUnresolved: true,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 100,
+            isIntersecting: false,
+            boundingClientRect: { top: 200 },
+          },
+        });
+
+        jest.advanceTimersToNextTimer();
+
+        expect(setMock.calls.length).toBe(4);
+        expect(setMock.calls[0]).toEqual([1]);
+        expect(setMock.calls[1]).toEqual([29]);
+        expect(setMock.calls[2]).toEqual([null]);
+        expect(setMock.calls[3]).toEqual([19]);
+
+        expect(getMock.calls.length).toBe(2);
+        expect(getMock.calls[0]).toEqual([30, false]);
+        expect(getMock.calls[1]).toEqual([20, true]);
+
+        [
+          setMock.invocationCallOrder[0],
+          getMock.invocationCallOrder[0],
+          setMock.invocationCallOrder[1],
+          setMock.invocationCallOrder[2],
+          getMock.invocationCallOrder[1],
+          setMock.invocationCallOrder[3],
+        ].forEach((order, idx, list) => {
+          // Compare each invocation sequence to the one before it (except the first one)
+          expect(list[idx - 1] || -1).toBeLessThan(order);
+        });
+      });
+    });
+  });
+});

spec/frontend/notes/components/discussion_notes_spec.js

 import { getByRole } from '@testing-library/dom';
 import { shallowMount, mount } from '@vue/test-utils';
 import '~/behaviors/markdown/render_gfm';
+import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import NoteableNote from '~/notes/components/noteable_note.vue';
 import { SYSTEM_NOTE } from '~/notes/constants';
@@ -26,6 +27,9 @@ describe('DiscussionNotes', () => {
   const createComponent = (props, mountingMethod = shallowMount) => {
     wrapper = mountingMethod(DiscussionNotes, {
       store,
+      provide: {
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: {
         discussion: discussionMock,
         isExpanded: false,

spec/frontend/notes/components/noteable_discussion_spec.js

@@ -3,6 +3,7 @@ import { nextTick } from 'vue';
 import discussionWithTwoUnresolvedNotes from 'test_fixtures/merge_requests/resolved_diff_discussion.json';
 import { trimText } from 'helpers/text_helper';
 import mockDiffFile from 'jest/diffs/mock_data/diff_file';
+import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
 import ResolveWithIssueButton from '~/notes/components/discussion_resolve_with_issue_button.vue';
@@ -31,6 +32,9 @@ describe('noteable_discussion component', () => {
 
     wrapper = mount(NoteableDiscussion, {
       store,
+      provide: {
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: { discussion: discussionMock },
     });
   });
@@ -167,6 +171,9 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
+          provide: {
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData: { discussion: discussionMock },
         });
       });
@@ -185,6 +192,9 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
+          provide: {
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData: { discussion: discussionMock },
         });
       });

spec/frontend/notes/components/notes_app_spec.js

@@ -7,6 +7,7 @@ import DraftNote from '~/batch_comments/components/draft_note.vue';
 import batchComments from '~/batch_comments/stores/modules/batch_comments';
 import axios from '~/lib/utils/axios_utils';
 import * as urlUtility from '~/lib/utils/url_utility';
+import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions';
 import CommentForm from '~/notes/components/comment_form.vue';
 import NotesApp from '~/notes/components/notes_app.vue';
 import * as constants from '~/notes/constants';
@@ -76,6 +77,9 @@ describe('note_app', () => {
           </div>`,
         },
         {
+          provide: {
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData,
           store,
         },