Merge branch '199732-show-more-context-to-jump-unresolved-button' into 'master'

Show more context in unresolved jump button

Closes #199732

See merge request gitlab-org/gitlab!32737

app/assets/javascripts/lib/utils/common_utils.js

@@ -244,22 +244,28 @@ export const contentTop = () => {
   );
 };
 
-export const scrollToElement = element => {
+export const scrollToElement = (element, options = {}) => {
   let $el = element;
   if (!(element instanceof $)) {
     $el = $(element);
   }
   const { top } = $el.offset();
+  const { offset = 0 } = options;
 
   // eslint-disable-next-line no-jquery/no-animate
   return $('body, html').animate(
     {
-      scrollTop: top - contentTop(),
+      scrollTop: top - contentTop() + offset,
     },
     200,
   );
 };
 
+export const scrollToElementWithContext = element => {
+  const offsetMultiplier = -0.1;
+  return scrollToElement(element, { offset: window.innerHeight * offsetMultiplier });
+};
+
 /**
  * Returns a function that can only be invoked once between
  * each browser screen repaint.

app/assets/javascripts/notes/mixins/discussion_navigation.js

 import { mapGetters, mapActions, mapState } from 'vuex';
-import { scrollToElement } from '~/lib/utils/common_utils';
+import { scrollToElementWithContext } from '~/lib/utils/common_utils';
 import eventHub from '../event_hub';
 
 /**
@@ -10,7 +10,7 @@ function scrollTo(selector) {
   const el = document.querySelector(selector);
 
   if (el) {
-    scrollToElement(el);
+    scrollToElementWithContext(el);
     return true;
   }
 

changelogs/unreleased/199732-show-more-context-to-jump-unresolved-button.yml

+---
+title: Show more context in unresolved jump button
+merge_request: 32737
+author:
+type: changed

spec/frontend/lib/utils/common_utils_spec.js

 import * as commonUtils from '~/lib/utils/common_utils';
+import $ from 'jquery';
 
 describe('common_utils', () => {
   describe('parseUrl', () => {
@@ -211,6 +212,59 @@ describe('common_utils', () => {
     });
   });
 
+  describe('scrollToElement*', () => {
+    let elem;
+    const windowHeight = 1000;
+    const elemTop = 100;
+
+    beforeEach(() => {
+      elem = document.createElement('div');
+      window.innerHeight = windowHeight;
+      jest.spyOn($.fn, 'animate');
+      jest.spyOn($.fn, 'offset').mockReturnValue({ top: elemTop });
+    });
+
+    afterEach(() => {
+      $.fn.animate.mockRestore();
+      $.fn.offset.mockRestore();
+    });
+
+    describe('scrollToElement', () => {
+      it('scrolls to element', () => {
+        commonUtils.scrollToElement(elem);
+        expect($.fn.animate).toHaveBeenCalledWith(
+          {
+            scrollTop: elemTop,
+          },
+          expect.any(Number),
+        );
+      });
+
+      it('scrolls to element with offset', () => {
+        const offset = 50;
+        commonUtils.scrollToElement(elem, { offset });
+        expect($.fn.animate).toHaveBeenCalledWith(
+          {
+            scrollTop: elemTop + offset,
+          },
+          expect.any(Number),
+        );
+      });
+    });
+
+    describe('scrollToElementWithContext', () => {
+      it('scrolls with context', () => {
+        commonUtils.scrollToElementWithContext();
+        expect($.fn.animate).toHaveBeenCalledWith(
+          {
+            scrollTop: elemTop - windowHeight * 0.1,
+          },
+          expect.any(Number),
+        );
+      });
+    });
+  });
+
   describe('debounceByAnimationFrame', () => {
     it('debounces a function to allow a maximum of one call per animation frame', done => {
       const spy = jest.fn();

spec/frontend/notes/mixins/discussion_navigation_spec.js

@@ -41,7 +41,7 @@ describe('Discussion navigation mixin', () => {
         .join(''),
     );
 
-    jest.spyOn(utils, 'scrollToElement');
+    jest.spyOn(utils, 'scrollToElementWithContext');
 
     expandDiscussion = jest.fn();
     const { actions, ...notesRest } = notesModule();
@@ -102,7 +102,7 @@ describe('Discussion navigation mixin', () => {
         });
 
         it('scrolls to element', () => {
-          expect(utils.scrollToElement).toHaveBeenCalledWith(
+          expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
             findDiscussion('div.discussion', expected),
           );
         });
@@ -123,11 +123,13 @@ describe('Discussion navigation mixin', () => {
         });
 
         it('scrolls when scrollToDiscussion is emitted', () => {
-          expect(utils.scrollToElement).not.toHaveBeenCalled();
+          expect(utils.scrollToElementWithContext).not.toHaveBeenCalled();
 
           eventHub.$emit('scrollToDiscussion');
 
-          expect(utils.scrollToElement).toHaveBeenCalledWith(findDiscussion('ul.notes', expected));
+          expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
+            findDiscussion('ul.notes', expected),
+          );
         });
       });
 
@@ -167,7 +169,7 @@ describe('Discussion navigation mixin', () => {
           });
 
           it('scrolls to discussion', () => {
-            expect(utils.scrollToElement).toHaveBeenCalledWith(
+            expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
               findDiscussion('div.discussion', expected),
             );
           });