From 2a3649b2eb1530acd62d718a180f06b4e85e30b0 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 27 Mar 2024 15:40:29 +0200 Subject: [PATCH 1/3] fix: update position on visual viewport scroll and resize --- .../src/vaadin-overlay-position-mixin.js | 12 ++++++--- .../test/position-mixin-listeners.test.js | 27 +++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/overlay/src/vaadin-overlay-position-mixin.js b/packages/overlay/src/vaadin-overlay-position-mixin.js index c2244e8e077..194eafa67d5 100644 --- a/packages/overlay/src/vaadin-overlay-position-mixin.js +++ b/packages/overlay/src/vaadin-overlay-position-mixin.js @@ -146,7 +146,8 @@ export const PositionMixin = (superClass) => /** @private */ __addUpdatePositionEventListeners() { - window.addEventListener('resize', this._updatePosition); + window.visualViewport.addEventListener('resize', this._updatePosition); + window.visualViewport.addEventListener('scroll', this.__onScroll, true); this.__positionTargetAncestorRootNodes = getAncestorRootNodes(this.positionTarget); this.__positionTargetAncestorRootNodes.forEach((node) => { @@ -156,7 +157,8 @@ export const PositionMixin = (superClass) => /** @private */ __removeUpdatePositionEventListeners() { - window.removeEventListener('resize', this._updatePosition); + window.visualViewport.removeEventListener('resize', this._updatePosition); + window.visualViewport.removeEventListener('scroll', this.__onScroll, true); if (this.__positionTargetAncestorRootNodes) { this.__positionTargetAncestorRootNodes.forEach((node) => { @@ -204,9 +206,11 @@ export const PositionMixin = (superClass) => /** @private */ __onScroll(e) { // If the scroll event occurred inside the overlay, ignore it. - if (!this.contains(e.target)) { - this._updatePosition(); + if (e.target instanceof Node && this.contains(e.target)) { + return; } + + this._updatePosition(); } _updatePosition() { diff --git a/packages/overlay/test/position-mixin-listeners.test.js b/packages/overlay/test/position-mixin-listeners.test.js index 0eb05870943..4c31aa2eb88 100644 --- a/packages/overlay/test/position-mixin-listeners.test.js +++ b/packages/overlay/test/position-mixin-listeners.test.js @@ -68,8 +68,8 @@ describe('position mixin listeners', () => { updatePositionSpy.resetHistory(); }); - it('should not update position on resize', () => { - resize(window); + it('should not update position on visual viewport resize', () => { + resize(window.visualViewport); expect(updatePositionSpy.called).to.be.false; }); @@ -84,10 +84,10 @@ describe('position mixin listeners', () => { expect(updatePositionSpy.called).to.be.false; }); - it('should update position on resize after assigning a position target', () => { + it('should update position on visual viewport resize after assigning a position target', () => { overlay.positionTarget = target; updatePositionSpy.resetHistory(); - resize(window); + resize(window.visualViewport); expect(updatePositionSpy.called).to.be.true; }); @@ -126,14 +126,25 @@ describe('position mixin listeners', () => { updatePositionSpy.resetHistory(); }); - it('should update position on resize', () => { - resize(window); + it('should update position on visual viewport resize', () => { + resize(window.visualViewport); expect(updatePositionSpy.called).to.be.true; }); - it('should not update position on resize when closed', () => { + it('should not update position on visual viewport resize when closed', () => { overlay.opened = false; - resize(window); + resize(window.visualViewport); + expect(updatePositionSpy.called).to.be.false; + }); + + it('should update position on visual viewport scroll', () => { + scroll(window.visualViewport); + expect(updatePositionSpy.called).to.be.true; + }); + + it('should not update position on visual viewport scroll when closed', () => { + overlay.opened = false; + scroll(window.visualViewport); expect(updatePositionSpy.called).to.be.false; }); From 1c4361c4284c1d225c33488784d663d21965ed67 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 27 Mar 2024 15:58:17 +0200 Subject: [PATCH 2/3] wip --- .../src/vaadin-overlay-position-mixin.js | 4 +++- .../test/position-mixin-listeners.test.js | 20 +++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/overlay/src/vaadin-overlay-position-mixin.js b/packages/overlay/src/vaadin-overlay-position-mixin.js index 194eafa67d5..db74210e814 100644 --- a/packages/overlay/src/vaadin-overlay-position-mixin.js +++ b/packages/overlay/src/vaadin-overlay-position-mixin.js @@ -149,7 +149,9 @@ export const PositionMixin = (superClass) => window.visualViewport.addEventListener('resize', this._updatePosition); window.visualViewport.addEventListener('scroll', this.__onScroll, true); - this.__positionTargetAncestorRootNodes = getAncestorRootNodes(this.positionTarget); + this.__positionTargetAncestorRootNodes = getAncestorRootNodes(this.positionTarget).filter( + (node) => node !== document, + ); 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 index 4c31aa2eb88..4b341ba073b 100644 --- a/packages/overlay/test/position-mixin-listeners.test.js +++ b/packages/overlay/test/position-mixin-listeners.test.js @@ -91,10 +91,10 @@ describe('position mixin listeners', () => { expect(updatePositionSpy.called).to.be.true; }); - it('should update position on document scroll after assigning a position target', () => { + it('should update position on visual viewport scroll after assigning a position target', () => { overlay.positionTarget = target; updatePositionSpy.resetHistory(); - scroll(document); + scroll(window.visualViewport); expect(updatePositionSpy.called).to.be.true; }); @@ -161,13 +161,13 @@ describe('position mixin listeners', () => { expect(updatePositionSpy.called).to.be.false; }); - ['document', 'ancestor'].forEach((name) => { + ['visual viewport', 'ancestor'].forEach((name) => { describe(name, () => { let scrollableNode; beforeEach(() => { - if (name === 'document') { - scrollableNode = document; + if (name === 'visual viewport') { + scrollableNode = window.visualViewport; } if (name === 'ancestor') { scrollableNode = wrapper.shadowRoot.querySelector('#scrollable'); @@ -219,8 +219,8 @@ describe('position mixin listeners', () => { updatePositionSpy.resetHistory(); }); - it('should update position on document scroll', () => { - scroll(document); + it('should update position on visual viewport scroll', () => { + scroll(window.visualViewport); expect(updatePositionSpy.called).to.be.true; }); @@ -245,8 +245,8 @@ describe('position mixin listeners', () => { newWrapper.appendChild(target); }); - it('should update position on document scroll after re-opened', async () => { - scroll(document); + it('should update position on visual viewport scroll after re-opened', async () => { + scroll(window.visualViewport); expect(updatePositionSpy.called).to.be.true; overlay.opened = false; @@ -254,7 +254,7 @@ describe('position mixin listeners', () => { await nextFrame(); updatePositionSpy.resetHistory(); - scroll(document); + scroll(window.visualViewport); expect(updatePositionSpy.called).to.be.true; }); From 5d30427d91ce861cbfc161f7bd6116b1b82c31cf Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 27 Mar 2024 16:02:05 +0200 Subject: [PATCH 3/3] wip --- packages/overlay/test/position-mixin-listeners.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/overlay/test/position-mixin-listeners.test.js b/packages/overlay/test/position-mixin-listeners.test.js index 4b341ba073b..5c6431feb26 100644 --- a/packages/overlay/test/position-mixin-listeners.test.js +++ b/packages/overlay/test/position-mixin-listeners.test.js @@ -126,6 +126,16 @@ describe('position mixin listeners', () => { updatePositionSpy.resetHistory(); }); + it('should not update position on window resize', () => { + resize(window); + expect(updatePositionSpy.called).to.be.false; + }); + + it('should not update position on document scroll', () => { + scroll(document); + expect(updatePositionSpy.called).to.be.false; + }); + it('should update position on visual viewport resize', () => { resize(window.visualViewport); expect(updatePositionSpy.called).to.be.true;