fix: listen to document scroll in addition to the visual viewport (#7298

) (#7299) Co-authored-by: Serhii Kulykov <[email protected]>
vaadin · Apr 4, 2024 · 612df29 · 612df29
1 parent 83f9384
commit 612df29
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 6 deletions.
diff --git a/packages/overlay/src/vaadin-overlay-position-mixin.js b/packages/overlay/src/vaadin-overlay-position-mixin.js
@@ -149,9 +149,7 @@ export const PositionMixin = (superClass) =>
       window.visualViewport.addEventListener('resize', this._updatePosition);
       window.visualViewport.addEventListener('scroll', this.__onScroll, true);
 
-      this.__positionTargetAncestorRootNodes = getAncestorRootNodes(this.positionTarget).filter(
-        (node) => node !== document,
-      );
+      this.__positionTargetAncestorRootNodes = getAncestorRootNodes(this.positionTarget);
       this.__positionTargetAncestorRootNodes.forEach((node) => {
         node.addEventListener('scroll', this.__onScroll, true);
       });

diff --git a/packages/overlay/test/position-mixin-listeners.test.js b/packages/overlay/test/position-mixin-listeners.test.js
@@ -78,6 +78,11 @@ describe('position mixin listeners', () => {
       expect(updatePositionSpy.called).to.be.false;
     });
 
+    it('should not update position on visual viewport scroll', () => {
+      scroll(window.visualViewport);
+      expect(updatePositionSpy.called).to.be.false;
+    });
+
     it('should not update position on ancestor scroll', () => {
       const scrollableAncestor = wrapper.shadowRoot.querySelector('#scrollable');
       scroll(scrollableAncestor);
@@ -98,6 +103,13 @@ describe('position mixin listeners', () => {
       expect(updatePositionSpy.called).to.be.true;
     });
 
+    it('should update position on document scroll after assigning a position target', () => {
+      overlay.positionTarget = target;
+      updatePositionSpy.resetHistory();
+      scroll(document);
+      expect(updatePositionSpy.called).to.be.true;
+    });
+
     it('should update position on ancestor scroll after assigning a position target', () => {
       overlay.positionTarget = target;
       updatePositionSpy.resetHistory();
@@ -131,9 +143,9 @@ describe('position mixin listeners', () => {
       expect(updatePositionSpy.called).to.be.false;
     });
 
-    it('should not update position on document scroll', () => {
+    it('should update position on document scroll', () => {
       scroll(document);
-      expect(updatePositionSpy.called).to.be.false;
+      expect(updatePositionSpy.called).to.be.true;
     });
 
     it('should update position on visual viewport resize', () => {
@@ -152,6 +164,12 @@ describe('position mixin listeners', () => {
       expect(updatePositionSpy.called).to.be.true;
     });
 
+    it('should not update position on document scroll when closed', () => {
+      overlay.opened = false;
+      scroll(document);
+      expect(updatePositionSpy.called).to.be.false;
+    });
+
     it('should not update position on visual viewport scroll when closed', () => {
       overlay.opened = false;
       scroll(window.visualViewport);
@@ -171,11 +189,14 @@ describe('position mixin listeners', () => {
       expect(updatePositionSpy.called).to.be.false;
     });
 
-    ['visual viewport', 'ancestor'].forEach((name) => {
+    ['document', 'visual viewport', 'ancestor'].forEach((name) => {
       describe(name, () => {
         let scrollableNode;
 
         beforeEach(() => {
+          if (name === 'document') {
+            scrollableNode = document;
+          }
           if (name === 'visual viewport') {
             scrollableNode = window.visualViewport;
           }
@@ -229,6 +250,11 @@ describe('position mixin listeners', () => {
         updatePositionSpy.resetHistory();
       });
 
+      it('should update position on document scroll', () => {
+        scroll(document);
+        expect(updatePositionSpy.called).to.be.true;
+      });
+
       it('should update position on visual viewport scroll', () => {
         scroll(window.visualViewport);
         expect(updatePositionSpy.called).to.be.true;
@@ -255,6 +281,19 @@ describe('position mixin listeners', () => {
         newWrapper.appendChild(target);
       });
 
+      it('should update position on document scroll after re-opened', async () => {
+        scroll(document);
+        expect(updatePositionSpy.called).to.be.true;
+
+        overlay.opened = false;
+        overlay.opened = true;
+        await nextFrame();
+        updatePositionSpy.resetHistory();
+
+        scroll(document);
+        expect(updatePositionSpy.called).to.be.true;
+      });
+
       it('should update position on visual viewport scroll after re-opened', async () => {
         scroll(window.visualViewport);
         expect(updatePositionSpy.called).to.be.true;