Add latest changes from gitlab-org/gitlab@master

4 files changed, 103 insertions, 24 deletions

diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index eedcb0c09d4..cdb8e9abab8 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -1,6 +1,7 @@
 <script>
 import { GlTooltipDirective, GlButton, GlButtonGroup } from '@gitlab/ui';
 import { mapGetters, mapActions } from 'vuex';
+import { throttle } from 'lodash';
 import { __ } from '~/locale';
 import discussionNavigation from '../mixins/discussion_navigation';
 
@@ -19,6 +20,12 @@ export default {
       required: true,
     },
   },
+  data() {
+    return {
+      jumpNext: throttle(this.jumpToNextDiscussion, 500),
+      jumpPrevious: throttle(this.jumpToPreviousDiscussion, 500),
+    };
+  },
   computed: {
     ...mapGetters([
       'getNoteableData',
@@ -54,6 +61,7 @@ export default {
 <template>
   <div
     v-if="resolvableDiscussionsCount > 0"
+    id="discussionCounter"
     ref="discussionCounter"
     class="gl-display-flex discussions-counter"
   >
@@ -74,7 +82,7 @@ export default {
         {{ n__('%d unresolved thread', '%d unresolved threads', unresolvedDiscussionsCount) }}
         <gl-button-group class="gl-ml-3">
           <gl-button
-            v-gl-tooltip.hover
+            v-gl-tooltip:discussionCounter.hover.bottom
             :title="__('Go to previous unresolved thread')"
             :aria-label="__('Go to previous unresolved thread')"
             class="discussion-previous-btn gl-rounded-base! gl-px-2!"
@@ -83,10 +91,10 @@ export default {
             data-track-property="click_previous_unresolved_thread_top"
             icon="chevron-lg-up"
             category="tertiary"
-            @click="jumpToPreviousDiscussion"
+            @click="jumpPrevious"
           />
           <gl-button
-            v-gl-tooltip.hover
+            v-gl-tooltip:discussionCounter.hover.bottom
             :title="__('Go to next unresolved thread')"
             :aria-label="__('Go to next unresolved thread')"
             class="discussion-next-btn gl-rounded-base! gl-px-2!"
@@ -95,7 +103,7 @@ export default {
             data-track-property="click_next_unresolved_thread_top"
             icon="chevron-lg-down"
             category="tertiary"
-            @click="jumpToNextDiscussion"
+            @click="jumpNext"
           />
         </gl-button-group>
       </template>
diff --git a/app/assets/javascripts/notes/components/discussion_navigator.vue b/app/assets/javascripts/notes/components/discussion_navigator.vue
index c1e39f31bbb..03bdc7a2cc6 100644
--- a/app/assets/javascripts/notes/components/discussion_navigator.vue
+++ b/app/assets/javascripts/notes/components/discussion_navigator.vue
@@ -1,6 +1,7 @@
 <script>
 /* global Mousetrap */
 import 'mousetrap';
+import { throttle } from 'lodash';
 import {
   keysFor,
   MR_NEXT_UNRESOLVED_DISCUSSION,
@@ -11,12 +12,18 @@ import discussionNavigation from '~/notes/mixins/discussion_navigation';
 
 export default {
   mixins: [discussionNavigation],
+  data() {
+    return {
+      jumpToNext: throttle(() => this.jumpToNextDiscussion({ behavior: 'auto' }), 200),
+      jumpToPrevious: throttle(() => this.jumpToPreviousDiscussion({ behavior: 'auto' }), 200),
+    };
+  },
   created() {
     eventHub.$on('jumpToFirstUnresolvedDiscussion', this.jumpToFirstUnresolvedDiscussion);
   },
   mounted() {
-    Mousetrap.bind(keysFor(MR_NEXT_UNRESOLVED_DISCUSSION), this.jumpToNextDiscussion);
-    Mousetrap.bind(keysFor(MR_PREVIOUS_UNRESOLVED_DISCUSSION), this.jumpToPreviousDiscussion);
+    Mousetrap.bind(keysFor(MR_NEXT_UNRESOLVED_DISCUSSION), this.jumpToNext);
+    Mousetrap.bind(keysFor(MR_PREVIOUS_UNRESOLVED_DISCUSSION), this.jumpToPrevious);
   },
   beforeDestroy() {
     Mousetrap.unbind(keysFor(MR_NEXT_UNRESOLVED_DISCUSSION));
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index c5d174ed890..ca880802b5d 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -269,6 +269,7 @@ export default {
     <div class="timeline-content">
       <div
         :data-discussion-id="discussion.id"
+        :data-discussion-resolved="discussion.resolved"
         class="discussion js-discussion-container"
         data-qa-selector="discussion_content"
       >
diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js
index 45df91796fc..10c897d2d2a 100644
--- a/app/assets/javascripts/notes/mixins/discussion_navigation.js
+++ b/app/assets/javascripts/notes/mixins/discussion_navigation.js
@@ -1,5 +1,5 @@
 import { mapGetters, mapActions, mapState } from 'vuex';
-import { scrollToElementWithContext, scrollToElement } from '~/lib/utils/common_utils';
+import { scrollToElementWithContext, scrollToElement, contentTop } from '~/lib/utils/common_utils';
 import { updateHistory } from '~/lib/utils/url_utility';
 import eventHub from '../event_hub';
 
@@ -94,8 +94,6 @@ function jumpToDiscussion(self, discussion) {
 
     if (activeTab === 'diffs' && isDiffDiscussion) {
       diffsJump(self, id, firstNoteId);
-    } else if (activeTab === 'show') {
-      discussionJump(self, id);
     } else {
       switchToDiscussionsTabAndJumpTo(self, id);
     }
@@ -105,11 +103,10 @@ function jumpToDiscussion(self, discussion) {
 /**
  * @param {object} self Component instance with mixin applied
  * @param {function} fn Which function used to get the target discussion's id
- * @param {string} [discussionId=this.currentDiscussionId] Current discussion id, will be null if discussions have not been traversed yet
  */
-function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId) {
+function handleDiscussionJump(self, fn) {
   const isDiffView = window.mrTabs.currentAction === 'diffs';
-  const targetId = fn(discussionId, isDiffView);
+  const targetId = fn(self.currentDiscussionId, isDiffView);
   const discussion = self.getDiscussion(targetId);
   const discussionFilePath = discussion?.diff_file?.file_path;
 
@@ -127,6 +124,70 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId)
   });
 }
 
+function getAllDiscussionElements() {
+  return Array.from(
+    document.querySelectorAll('[data-discussion-id]:not([data-discussion-resolved])'),
+  );
+}
+
+function hasReachedPageEnd() {
+  return document.body.scrollHeight <= Math.ceil(window.scrollY + window.innerHeight);
+}
+
+function findNextClosestVisibleDiscussion(discussionElements) {
+  const offsetHeight = contentTop();
+  let isActive;
+  const index = discussionElements.findIndex((element) => {
+    const { y } = element.getBoundingClientRect();
+    const visibleHorizontalOffset = Math.ceil(y) - offsetHeight;
+    // handle rect rounding errors
+    isActive = visibleHorizontalOffset < 2;
+    return visibleHorizontalOffset >= 0;
+  });
+  return [discussionElements[index], index, isActive];
+}
+
+function getNextDiscussion() {
+  const discussionElements = getAllDiscussionElements();
+  const firstDiscussion = discussionElements[0];
+  if (hasReachedPageEnd()) {
+    return firstDiscussion;
+  }
+  const [nextClosestDiscussion, index, isActive] = findNextClosestVisibleDiscussion(
+    discussionElements,
+  );
+  if (nextClosestDiscussion && !isActive) {
+    return nextClosestDiscussion;
+  }
+  const nextDiscussion = discussionElements[index + 1];
+  if (!nextClosestDiscussion || !nextDiscussion) {
+    return firstDiscussion;
+  }
+  return nextDiscussion;
+}
+
+function getPreviousDiscussion() {
+  const discussionElements = getAllDiscussionElements();
+  const lastDiscussion = discussionElements[discussionElements.length - 1];
+  const [, index] = findNextClosestVisibleDiscussion(discussionElements);
+  const previousDiscussion = discussionElements[index - 1];
+  if (previousDiscussion) {
+    return previousDiscussion;
+  }
+  return lastDiscussion;
+}
+
+function handleJumpForBothPages(getDiscussion, ctx, fn, scrollOptions) {
+  if (window.mrTabs.currentAction !== 'show') {
+    handleDiscussionJump(ctx, fn);
+  } else {
+    const discussion = getDiscussion();
+    const id = discussion.dataset.discussionId;
+    ctx.expandDiscussion({ discussionId: id });
+    scrollToElement(discussion, scrollOptions);
+  }
+}
+
 export default {
   computed: {
     ...mapGetters([
@@ -142,12 +203,22 @@ export default {
     ...mapActions(['expandDiscussion', 'setCurrentDiscussionId']),
     ...mapActions('diffs', ['scrollToFile']),
 
-    jumpToNextDiscussion() {
-      handleDiscussionJump(this, this.nextUnresolvedDiscussionId);
+    jumpToNextDiscussion(scrollOptions) {
+      handleJumpForBothPages(
+        getNextDiscussion,
+        this,
+        this.nextUnresolvedDiscussionId,
+        scrollOptions,
+      );
     },
 
-    jumpToPreviousDiscussion() {
-      handleDiscussionJump(this, this.previousUnresolvedDiscussionId);
+    jumpToPreviousDiscussion(scrollOptions) {
+      handleJumpForBothPages(
+        getPreviousDiscussion,
+        this,
+        this.previousUnresolvedDiscussionId,
+        scrollOptions,
+      );
     },
 
     jumpToFirstUnresolvedDiscussion() {
@@ -157,13 +228,5 @@ export default {
         })
         .catch(() => {});
     },
-
-    /**
-     * Go to the next discussion from the given discussionId
-     * @param {String} discussionId The id we are jumping from
-     */
-    jumpToNextRelativeDiscussion(discussionId) {
-      handleDiscussionJump(this, this.nextUnresolvedDiscussionId, discussionId);
-    },
   },
 };