Merge branch 'psi-no-note-scroll' into 'master'

Limit scrollToTargetOnResize to note hashes

See merge request gitlab-org/gitlab!81192

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

@@ -10,22 +10,30 @@ export function createResizeObserver() {
   });
 }
 
-// watches for change in size of a container element (e.g. for lazy-loaded images)
-// and scroll the target element to the top of the content area
-// stop watching after any user input. So if user opens sidebar or manually
-// scrolls the page we don't hijack their scroll position
+/**
+ * Watches for change in size of a container element (e.g. for lazy-loaded images)
+ * and scrolls the target note to the top of the content area.
+ * Stops watching after any user input. So if user opens sidebar or manually
+ * scrolls the page we don't hijack their scroll position
+ *
+ * @param {Object} options
+ * @param {string} options.targetId - id of element to scroll to
+ * @param {string} options.container - Selector of element containing target
+ *
+ * @return {ResizeObserver|null} - ResizeObserver instance if target looks like a note DOM ID
+ */
 export function scrollToTargetOnResize({
-  target = window.location.hash,
+  targetId = window.location.hash.slice(1),
   container = '#content-body',
 } = {}) {
-  if (!target) return null;
+  if (!targetId) return null;
 
   const ro = createResizeObserver();
   const containerEl = document.querySelector(container);
   let interactionListenersAdded = false;
 
   function keepTargetAtTop() {
-    const anchorEl = document.querySelector(target);
+    const anchorEl = document.getElementById(targetId);
 
     if (!anchorEl) return;
 

spec/frontend/lib/utils/resize_observer_spec.js

@@ -19,16 +19,11 @@ describe('ResizeObserver Utility', () => {
 
     jest.spyOn(document.documentElement, 'scrollTo');
 
-    setFixtures(`<div id="content-body"><div class="target">element to scroll to</div></div>`);
+    setFixtures(`<div id="content-body"><div id="note_1234">note to scroll to</div></div>`);
 
-    const target = document.querySelector('.target');
+    const target = document.querySelector('#note_1234');
 
     jest.spyOn(target, 'getBoundingClientRect').mockReturnValue({ top: 200 });
-
-    observer = scrollToTargetOnResize({
-      target: '.target',
-      container: '#content-body',
-    });
   });
 
   afterEach(() => {
@@ -38,21 +33,22 @@ describe('ResizeObserver Utility', () => {
   describe('Observer behavior', () => {
     it('returns null for empty target', () => {
       observer = scrollToTargetOnResize({
-        target: '',
+        targetId: '',
         container: '#content-body',
       });
 
       expect(observer).toBe(null);
     });
 
-    it('returns ResizeObserver instance', () => {
-      expect(observer).toBeInstanceOf(ResizeObserver);
-    });
+    it('does not scroll if target does not exist', () => {
+      observer = scrollToTargetOnResize({
+        targetId: 'some_imaginary_id',
+        container: '#content-body',
+      });
 
-    it('scrolls body so anchor is just below sticky header (contentTop)', () => {
       triggerResize();
 
-      expect(document.documentElement.scrollTo).toHaveBeenCalledWith({ top: 110 });
+      expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
     });
 
     const interactionEvents = ['mousedown', 'touchstart', 'keydown', 'wheel'];
@@ -64,5 +60,24 @@ describe('ResizeObserver Utility', () => {
 
       expect(document.documentElement.scrollTo).not.toHaveBeenCalledWith();
     });
+
+    describe('with existing target', () => {
+      beforeEach(() => {
+        observer = scrollToTargetOnResize({
+          targetId: 'note_1234',
+          container: '#content-body',
+        });
+      });
+
+      it('returns ResizeObserver instance', () => {
+        expect(observer).toBeInstanceOf(ResizeObserver);
+      });
+
+      it('scrolls body so anchor is just below sticky header (contentTop)', () => {
+        triggerResize();
+
+        expect(document.documentElement.scrollTo).toHaveBeenCalledWith({ top: 110 });
+      });
+    });
   });
 });