fix: adjust virtualizer indexes when scrolling at the end #9142
Conversation
There was a problem hiding this comment.
Since the issue occurs on resize, I wonder if this should rather be fixed by overriding iron-list's _resizeHandler:
_resizeHandler() {
super._resizeHandler();
const lastIndexVisible = this.adjustedLastVisibleIndex === this.size - 1;
const emptySpace = this._physicalTop - this._scrollPosition;
if (lastIndexVisible && emptySpace > 0) {
const idxAdjustment = Math.ceil(emptySpace / this._physicalAverage);
this._virtualStart = Math.max(0, this._virtualStart - idxAdjustment);
this._physicalStart = Math.max(0, this._physicalStart - idxAdjustment);
// Scroll to end for smoother resize
super.scrollToIndex(this._virtualCount - 1);
}
}EDIT: Since _resizeHandler also invokes on item resize, could also consider adding a dedicated ResizeObserver to scrollTarget for this purpose (maybe it's overoptimizing).
There was a problem hiding this comment.
This is a pretty clean fix. Updated the PR to use this approach. However, separating the resize handling to apply the fix only for the scroll target still reproduces the issue under some cases. Therefore, I left the resize observer a single one that applies the fix to both.
There was a problem hiding this comment.
This change seems to introduce a new issue when resizing a grid. It keeps adjusting the indexes in a loop, decreasing the scrollable area until the scroll position snaps back to the start. It looks like this happens when there is still some empty space at the top that could be filled with another item. This is likely cause by __fixInvalidItemPositioning, which adjusts the scroll position by a single pixel, potentially in a loop.
Some other observations:
- The new workaround doesn't always add enough items to fill up the empty space, it looks like the topmost item is missing sometimes. That seems to be the cause for the issue above.
- There already is a
__fixInvalidItemPositioningwhich seems to detect a similar case with items being missing at either top or bottom, but the fix it applies isn't enough to make new items show up. However maybe it would be more appropriate to try to make that method work properly rather than add another workaround in a different place. - It's not clear to my why we need to use the scroll delta as part of the calculation. I would expect that there would be some way to detect how much space is empty, and use that space to determine how many items need to be added.
__fixInvalidItemPositioninghas some calculations that might help there.
Bildschirmaufnahme.2025-06-17.um.11.08.13.mov
Reproduction
<script type="module">
// PolymerElement based imports
import '@vaadin/grid/all-imports';
import '@vaadin/dialog';
const grid = document.querySelector('vaadin-grid');
grid.items = [...Array(1000)].map((_, i) => ({
name: `Item ${i}`,
}));
const scrollButton = document.getElementById('scroll');
scrollButton.addEventListener('click', () => {
// Scroll to the end of the grid
grid.scrollToIndex(1000);
});
const dialog = document.querySelector('vaadin-dialog');
const dialogContent = document.getElementById('dialog-content');
dialog.renderer = (root) => {
root.innerHTML = '';
root.appendChild(dialogContent);
}
dialog.opened = true;
</script>
<vaadin-dialog resizable draggable></vaadin-dialog>
<div id="dialog-content" style="height: 100%; display: flex; flex-direction: column; gap: 10px;">
<vaadin-grid item-id-path="name" style="flex: 1">
<vaadin-grid-column path="name" width="200px" flex-shrink="0"></vaadin-grid-column>
<vaadin-grid-column path="name" width="200px" flex-shrink="0"></vaadin-grid-column>
<vaadin-grid-column path="name" width="200px" flex-shrink="0"></vaadin-grid-column>
</vaadin-grid>
<button id="scroll" style="flex: 0">Scroll to end</button>
</div>There was a problem hiding this comment.
This endless loop around __fixInvalidItemPositioning under some conditions was present before the fix already. This change seemed to fix it for me. But probably not all the cases as per your finding. This workaround/fix is based on the index adjustment logic iron list core _scrollHandler, which handles large jumps. The delta calculation comes from there basically. I will reexamine __fixInvalidItemPositioning and also consider Tomi's suggestions above and try to come up with a more stable fix.
There was a problem hiding this comment.
Updated the fix based on the suggestions and tried it with multiple setups. I cannot reproduce a loop anymore. The fix resides in an overridden _resizeHandler. While this can still be considered a workaround, I could not find a better fix around __fixInvalidItemPositioning.
| this._virtualStart = Math.max(0, this._virtualStart - idxAdjustment); | ||
| this._physicalStart = Math.max(0, this._physicalStart - idxAdjustment); | ||
| // Scroll to end for smoother resize | ||
| super.scrollToIndex(this._virtualCount - 1); |
There was a problem hiding this comment.
While this works well for virtualizer generally:
Kapture.2025-06-19.at.09.15.07.mp4
Resizing grid still looks strange
Kapture.2025-06-19.at.09.20.59.mp4
This could possibly be fixed by calling this.scrollTarget.scrollTop = this.scrollTarget.scrollHeight - this.scrollTarget.clientHeight; after the scrollToIndex call.
There was a problem hiding this comment.
Yep, it seems to make the scrolling smoother. Updated the PR.
|
* fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling
* fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling
* fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling
* fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling
) * fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
) * fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
* fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling
) * fix: adjust virtualizer indexes when scrolling at the end * fix: do not get reusables when there are no physical items * fix: fix jumpy scrolling at virtualizer end * refactor: do not modify delta * refactor: revert changes in original path * refactor: move line back * test: add grid container resize test and modify an existing test * refactor: move the fix to resize handler and simplify index adjustment * refactor: update scroll top in order to have smooth scrolling Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
Description
The indexes in the virtualizer are not properly updated when
This case is not properly handled in the iron list core or the adapter. The result is the items not being properly rendered and some loops occurring based on resize timing.
This PR introduces an index adjustment for this case, overriding
_resizeHandlerfrom the iron list core.Before fix:
withoutFix.mov
After fix:
withFix.mov
Fixes #7307
Type of change
Checklist