Merge branch '346182-revert-71508-and-74009' into 'master'

Revert "Merge branch 'tor/feature/actual-jump-to-next-diffs' into 'master'"

See merge request gitlab-org/gitlab!74948

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

@@ -44,7 +44,6 @@ 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';
@@ -87,9 +86,6 @@ 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';
@@ -17,9 +16,7 @@ export default {
     ToggleRepliesWidget,
     NoteEditedText,
     DiscussionNotesRepliesWrapper,
-    GlIntersectionObserver,
   },
-  inject: ['discussionObserverHandler'],
   props: {
     discussion: {
       type: Object,
@@ -57,11 +54,7 @@ export default {
     },
   },
   computed: {
-    ...mapGetters([
-      'userCanReply',
-      'previousUnresolvedDiscussionId',
-      'firstUnresolvedDiscussionId',
-    ]),
+    ...mapGetters(['userCanReply']),
     hasReplies() {
       return Boolean(this.replies.length);
     },
@@ -84,20 +77,9 @@ 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',
-      'setCurrentDiscussionId',
-    ]),
+    ...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']),
     componentName(note) {
       if (note.isPlaceholderNote) {
         if (note.placeholderType === SYSTEM_NOTE) {
@@ -128,18 +110,6 @@ 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>
@@ -152,35 +122,33 @@ export default {
       @mouseleave="handleMouseLeave(discussion)"
     >
       <template v-if="shouldGroupReplies">
-        <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>
+        <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>
         <discussion-notes-replies-wrapper :is-diff-discussion="discussion.diff_discussion">
           <toggle-replies-widget
             v-if="hasReplies"

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

@@ -8,7 +8,6 @@ import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item
 import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import draftNote from '../../batch_comments/components/draft_note.vue';
-import { discussionIntersectionObserverHandlerFactory } from '../../diffs/utils/discussions';
 import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
 import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
 import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
@@ -39,9 +38,6 @@ export default {
     TimelineEntryItem,
   },
   mixins: [glFeatureFlagsMixin()],
-  provide: {
-    discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
-  },
   props: {
     noteableData: {
       type: Object,

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';
@@ -20,9 +19,6 @@ 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';
@@ -27,9 +26,6 @@ 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,7 +3,6 @@ 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';
@@ -32,9 +31,6 @@ describe('noteable_discussion component', () => {
 
     wrapper = mount(NoteableDiscussion, {
       store,
-      provide: {
-        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
-      },
       propsData: { discussion: discussionMock },
     });
   });
@@ -171,9 +167,6 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
-          provide: {
-            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
-          },
           propsData: { discussion: discussionMock },
         });
       });
@@ -192,9 +185,6 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
-          provide: {
-            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
-          },
           propsData: { discussion: discussionMock },
         });
       });

spec/frontend/notes/components/notes_app_spec.js

@@ -9,7 +9,6 @@ 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';
@@ -79,9 +78,6 @@ describe('note_app', () => {
           </div>`,
         },
         {
-          provide: {
-            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
-          },
           propsData,
           store,
         },