Skip to content

Commit 324e4ad

Browse files
committed
refactor: add hostConnected to virtualizer, use it in virtual-list and grid
1 parent 0fb7bb7 commit 324e4ad

File tree

7 files changed

+44
-21
lines changed

7 files changed

+44
-21
lines changed

packages/component-base/src/virtualizer-iron-list-adapter.js

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,6 @@ export class IronListAdapter {
5252
this.__resizeObserver.observe(this.scrollTarget);
5353
this.scrollTarget.addEventListener('scroll', () => this._scrollHandler());
5454

55-
const attachObserver = new ResizeObserver(([{ contentRect }]) => {
56-
const isHidden = contentRect.width === 0 && contentRect.height === 0;
57-
if (!isHidden && this.__scrollTargetHidden && this.scrollTarget.scrollTop !== this._scrollPosition) {
58-
// When removing element from DOM, its scroll position is lost and
59-
// virtualizer doesn't re-render when adding it to the DOM again.
60-
// Restore scroll position when the scroll target becomes visible,
61-
// which is the case e.g. when virtualizer is used inside a dialog.
62-
this.scrollTarget.scrollTop = this._scrollPosition;
63-
}
64-
65-
this.__scrollTargetHidden = isHidden;
66-
});
67-
attachObserver.observe(this.scrollTarget);
68-
6955
this._scrollLineHeight = this._getScrollLineHeight();
7056
this.scrollTarget.addEventListener('wheel', (e) => this.__onWheel(e));
7157

@@ -192,6 +178,14 @@ export class IronListAdapter {
192178
this.__afterElementsUpdated(updatedElements);
193179
}
194180

181+
hostConnected() {
182+
if (this.scrollTarget.scrollTop !== this._scrollPosition) {
183+
// Restore scroll position, which is reset when host is removed from DOM,
184+
// since virtualizer doesn't re-render when adding it to the DOM again.
185+
this.scrollTarget.scrollTop = this._scrollPosition;
186+
}
187+
}
188+
195189
/**
196190
* Updates the height for a given set of items.
197191
*

packages/component-base/src/virtualizer.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,13 @@ export class Virtualizer {
8080
flush() {
8181
this.__adapter.flush();
8282
}
83+
84+
/**
85+
* Notifies the virtualizer about its host element connected to the DOM.
86+
*
87+
* @method hostConnected
88+
*/
89+
hostConnected() {
90+
this.__adapter.hostConnected();
91+
}
8392
}

packages/component-base/test/virtualizer.test.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@vaadin/chai-plugins';
2-
import { aTimeout, fixtureSync, nextFrame, nextResize, oneEvent } from '@vaadin/testing-helpers';
2+
import { aTimeout, fixtureSync, nextFrame, oneEvent } from '@vaadin/testing-helpers';
33
import sinon from 'sinon';
44
import { Virtualizer } from '../src/virtualizer.js';
55

@@ -350,18 +350,14 @@ describe('virtualizer', () => {
350350
expect(initialCount).not.to.be.above(expectedCount);
351351
});
352352

353-
it('should preserve scroll position when moving within DOM and changing visibility', async () => {
353+
it('should restore scroll position on hostConnected after moving within DOM', async () => {
354354
scrollTarget.scrollTop = 100;
355355
await oneEvent(scrollTarget, 'scroll');
356356

357-
scrollTarget.hidden = true;
358-
await nextResize(scrollTarget);
359-
360357
const wrapper = fixtureSync('<div></div>');
361358
wrapper.appendChild(scrollTarget);
362359

363-
scrollTarget.hidden = false;
364-
await nextResize(scrollTarget);
360+
virtualizer.hostConnected();
365361

366362
expect(scrollTarget.scrollTop).to.equal(100);
367363
});

packages/grid/src/vaadin-grid-mixin.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ export const GridMixin = (superClass) =>
197197
connectedCallback() {
198198
super.connectedCallback();
199199
this.isAttached = true;
200+
this.__virtualizer.hostConnected();
200201
}
201202

202203
/** @protected */

packages/grid/test/basic.test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,16 @@ describe('basic features', () => {
131131
expect(top).to.be.greaterThan(0);
132132
});
133133

134+
it('should restore scroll position when moving within DOM', () => {
135+
grid.scrollToIndex(99);
136+
const top = grid.$.table.scrollTop;
137+
138+
const wrapper = fixtureSync('<div></div>');
139+
wrapper.appendChild(grid);
140+
141+
expect(grid.$.table.scrollTop).to.eql(top);
142+
});
143+
134144
it('reset items', () => {
135145
grid.size = 100;
136146

packages/virtual-list/src/vaadin-virtual-list-mixin.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export const VirtualListMixin = (superClass) =>
104104
connectedCallback() {
105105
super.connectedCallback();
106106
document.addEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
107+
this.__virtualizer.hostConnected();
107108
}
108109

109110
/** @protected */

packages/virtual-list/test/virtual-list.test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,17 @@ describe('virtual-list', () => {
177177
expect(list.getAttribute('overflow')).to.equal('top');
178178
});
179179
});
180+
181+
describe('scroll restoration', () => {
182+
it('should restore scroll position when moving within DOM', () => {
183+
list.scrollToIndex(50);
184+
const top = list.scrollTop;
185+
186+
const wrapper = fixtureSync('<div></div>');
187+
wrapper.appendChild(list);
188+
189+
expect(list.scrollTop).to.equal(top);
190+
});
191+
});
180192
});
181193
});

0 commit comments

Comments
 (0)