Skip to content

Commit

Permalink
fix: update position on visual viewport scroll and resize (#7279)
Browse files Browse the repository at this point in the history
  • Loading branch information
vursen authored and vaadin-bot committed Apr 3, 2024
1 parent 60f1c0e commit 121bb78
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 22 deletions.
16 changes: 11 additions & 5 deletions packages/overlay/src/vaadin-overlay-position-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,21 @@ 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 = getAncestorRootNodes(this.positionTarget).filter(
(node) => node !== document,
);
this.__positionTargetAncestorRootNodes.forEach((node) => {
node.addEventListener('scroll', this.__onScroll, true);
});
}

/** @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) => {
Expand Down Expand Up @@ -204,9 +208,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() {
Expand Down
55 changes: 38 additions & 17 deletions packages/overlay/test/position-mixin-listeners.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand All @@ -84,17 +84,17 @@ 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;
});

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;
});

Expand Down Expand Up @@ -126,14 +126,35 @@ describe('position mixin listeners', () => {
updatePositionSpy.resetHistory();
});

it('should update position on resize', () => {
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;
});

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;
});

Expand All @@ -150,13 +171,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');
Expand Down Expand Up @@ -208,8 +229,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;
});

Expand All @@ -234,16 +255,16 @@ 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;
overlay.opened = true;
await nextFrame();
updatePositionSpy.resetHistory();

scroll(document);
scroll(window.visualViewport);
expect(updatePositionSpy.called).to.be.true;
});

Expand Down

0 comments on commit 121bb78

Please sign in to comment.