Skip to content

Commit fcbb66b

Browse files
vaadin-botvursenclaude
authored
fix: update scroll container height on virtualizer size change (#12267) (#12268)
This PR cherry-picks changes from the original PR #12267 to branch 25.2. --- #### Original PR description > When the virtualizer size changes, the scroll container height needs to be updated. This used to happen reliably only because the virtualizer called `scrollToIndex` unconditionally in that situation, and that call force-updated the height as a side effect. However, that call was made conditional in #11196, and no longer happens when no scroll position needs to be restored. Without it, the amortization logic in `_updateScrollerSize` can skip the height update if the change is smaller than the viewport, so the scroll container ends up with a stale height. > > This PR fixes that by making the size setter force the height update with an explicit `_updateScrollerSize(true)` call. This also covers the `allRowsVisible` growth scenario from #12146, so the complexity introduced there is no longer needed and has been removed. The regression tests from that PR still pass. > > Fixes vaadin/flow-components#9802 Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ff14786 commit fcbb66b

3 files changed

Lines changed: 10 additions & 19 deletions

File tree

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ export class IronListAdapter {
2222
reorderElements,
2323
elementsContainer,
2424
__disableHeightPlaceholder,
25-
__alwaysUpdateScrollerSize,
2625
}) {
2726
this.isAttached = true;
2827
this._vidxOffset = 0;
@@ -38,12 +37,6 @@ export class IronListAdapter {
3837
// elements with a non-zero height. Not for public use.
3938
this.__disableHeightPlaceholder = __disableHeightPlaceholder ?? false;
4039

41-
// Internal option: a predicate that, when it returns true, makes the scroller
42-
// height always be applied instead of amortized (see `_updateScrollerSize`).
43-
// Used by components whose height tracks the content exactly (e.g. the grid's
44-
// `allRowsVisible` mode). Not for public use.
45-
this.__alwaysUpdateScrollerSize = __alwaysUpdateScrollerSize;
46-
4740
// Iron-list uses this value to determine how many pages of elements to render
4841
this._maxPages = 1.3;
4942

@@ -223,11 +216,6 @@ export class IronListAdapter {
223216
this.__afterElementsUpdated(updatedElements);
224217
}
225218

226-
/** @override */
227-
_updateScrollerSize(forceUpdate) {
228-
super._updateScrollerSize(forceUpdate || !!this.__alwaysUpdateScrollerSize?.());
229-
}
230-
231219
/**
232220
* Updates the height for a given set of items.
233221
*
@@ -426,6 +414,8 @@ export class IronListAdapter {
426414
requestAnimationFrame(() => this._resizeHandler());
427415
}
428416

417+
this._updateScrollerSize(true);
418+
429419
// Re-render items once the scroll position has been restored.
430420
// This call also updates the cached scrollTarget height and
431421
// rechecks whether more virtual elements are needed, since the

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,14 @@ describe('virtualizer', () => {
234234
expect(item.getBoundingClientRect().top).to.be.closeTo(scrollTarget.getBoundingClientRect().top - 10, 1);
235235
});
236236

237+
it('should update scroll container height on size change', () => {
238+
virtualizer.size += 5;
239+
expect(elementsContainer.offsetHeight).to.equal(virtualizer.size * 30);
240+
241+
virtualizer.size -= 5;
242+
expect(elementsContainer.offsetHeight).to.equal(virtualizer.size * 30);
243+
});
244+
237245
it('should not call updateElement when size increase does not affect visible indexes', () => {
238246
const updateElement = sinon.spy((el, index) => {
239247
el.textContent = `item-${index}`;

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,6 @@ export const GridMixin = (superClass) =>
247247
// otherwise be triggered by this logic because it reads the row height
248248
// right after updating the rows' content.
249249
__disableHeightPlaceholder: true,
250-
// The virtualizer amortizes scroller height updates to avoid reflows while
251-
// scrolling. In `allRowsVisible` mode the grid has no scrolling and its
252-
// height must track the content exactly, so tell the virtualizer to always
253-
// apply the scroller height. Otherwise the items container can be left at a
254-
// stale, too-small height and clip rows when the grid grows (e.g. when
255-
// expanding a tree grid from a small size).
256-
__alwaysUpdateScrollerSize: () => this.allRowsVisible,
257250
});
258251

259252
this._tooltipController = new TooltipController(this);

0 commit comments

Comments
 (0)