Fix `Manipulator.offset()` (#33603)

* test: add more test cases for Manipulator.offset()

* fix: Manipulator.offset() is using obsolete properties to get scroll position

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Co-authored-by: GeoSot <geo.sotis@gmail.com>

diff --git a/js/src/dom/manipulator.js b/js/src/dom/manipulator.js
index 113817beedb91e9e3add3266fecc3878f99557ad..a866993f0dad9b69b220fa7a914db30de4fd1fc0 100644 (file)
--- a/js/src/dom/manipulator.js
+++ b/js/src/dom/manipulator.js
@@ -64,8 +64,8 @@ const Manipulator = {
     const rect = element.getBoundingClientRect()
 
     return {
-      top: rect.top + document.body.scrollTop,
-      left: rect.left + document.body.scrollLeft
+      top: rect.top + window.pageYOffset,
+      left: rect.left + window.pageXOffset
     }
   },
 

diff --git a/js/tests/unit/dom/manipulator.spec.js b/js/tests/unit/dom/manipulator.spec.js
index 3d91e6f744dfb4d7c94bec1f630e1d50c4822a32..4f8e40067ea080ca434d015e9d3258ecd16a56cf 100644 (file)
--- a/js/tests/unit/dom/manipulator.spec.js
+++ b/js/tests/unit/dom/manipulator.spec.js
@@ -119,6 +119,60 @@ describe('Manipulator', () => {
       expect(offset.top).toEqual(jasmine.any(Number))
       expect(offset.left).toEqual(jasmine.any(Number))
     })
+
+    it('should return offset relative to attached element\'s offset', () => {
+      const top = 500
+      const left = 1000
+
+      fixtureEl.innerHTML = `<div style="position:absolute;top:${top}px;left:${left}px"></div>`
+
+      const div = fixtureEl.querySelector('div')
+      const offset = Manipulator.offset(div)
+      const fixtureOffset = Manipulator.offset(fixtureEl)
+
+      expect(offset).toEqual({
+        top: fixtureOffset.top + top,
+        left: fixtureOffset.left + left
+      })
+    })
+
+    it('should not change offset when viewport is scrolled', done => {
+      const top = 500
+      const left = 1000
+      const scrollY = 200
+      const scrollX = 400
+
+      fixtureEl.innerHTML = `<div style="position:absolute;top:${top}px;left:${left}px"></div>`
+
+      const div = fixtureEl.querySelector('div')
+      const offset = Manipulator.offset(div)
+
+      // append an element that forces scrollbars on the window so we can scroll
+      const { defaultView: win, body } = fixtureEl.ownerDocument
+      const forceScrollBars = document.createElement('div')
+      forceScrollBars.style.cssText = 'position:absolute;top:5000px;left:5000px;width:1px;height:1px'
+      body.appendChild(forceScrollBars)
+
+      const scrollHandler = () => {
+        expect(window.pageYOffset).toBe(scrollY)
+        expect(window.pageXOffset).toBe(scrollX)
+
+        const newOffset = Manipulator.offset(div)
+
+        expect(newOffset).toEqual({
+          top: offset.top,
+          left: offset.left
+        })
+
+        win.removeEventListener('scroll', scrollHandler)
+        body.removeChild(forceScrollBars)
+        win.scrollTo(0, 0)
+        done()
+      }
+
+      win.addEventListener('scroll', scrollHandler)
+      win.scrollTo(scrollX, scrollY)
+    })
   })
 
   describe('position', () => {