Skip to content

Virtualize: Refresh when estimated item size changes. Fixes #25915 #26268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Virtualize: Refresh when estimated item size changes. Fixes #25915 #26268

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
745b12d
Refresh when estimated item size changes. Fixes #25915
SteveSandersonMS Sep 24, 2020
303c2c1
Remove typo
SteveSandersonMS Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 29 additions & 12 deletions src/Components/Web/src/Virtualization/Virtualize.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,44 +250,54 @@ private string GetSpacerStyle(int itemsInSpacer)

void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(float spacerSize, float spacerSeparation, float containerSize)
{
CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsBefore, out var visibleItemCapacity);
CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsBefore, out var visibleItemCapacity, out var itemSizeChanged);

UpdateItemDistribution(itemsBefore, visibleItemCapacity);
UpdateItemDistribution(itemsBefore, visibleItemCapacity, itemSizeChanged);
}

void IVirtualizeJsCallbacks.OnAfterSpacerVisible(float spacerSize, float spacerSeparation, float containerSize)
{
CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity);
CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity, out var itemSizeChanged);

var itemsBefore = Math.Max(0, _itemCount - itemsAfter - visibleItemCapacity);

UpdateItemDistribution(itemsBefore, visibleItemCapacity);
UpdateItemDistribution(itemsBefore, visibleItemCapacity, itemSizeChanged);
}

private void CalcualteItemDistribution(
float spacerSize,
float spacerSeparation,
float containerSize,
out int itemsInSpacer,
out int visibleItemCapacity)
out int visibleItemCapacity,
out bool itemSizeChanged)
{
if (_lastRenderedItemCount > 0)
if (spacerSeparation > 0 && _lastRenderedItemCount > 0)
{
var previousItemSize = _itemSize;
_itemSize = (spacerSeparation - (_lastRenderedPlaceholderCount * _itemSize)) / _lastRenderedItemCount;
}

if (_itemSize <= 0)
if (_itemSize <= 0)
{
// At this point, something unusual has occurred, likely due to misuse of this component.
// Reset the calculated item size to the user-provided item size.
_itemSize = ItemSize;
}

// We don't want to create an infinite rendering loop due to floating point precision limits,
// so only re-render if the item size change is big enough
itemSizeChanged = Math.Abs(_itemSize - previousItemSize) > 0.5f;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bit I'm a bit nervous about. Might there be some edge case where we create an infinite loop? That is, on every render we compute a slightly different item size, perhaps alternating between two values.

I'm pretty sure I could cause such a problem deliberately if I wanted by putting in some pathological CSS that varies the sizes weirdly. Whether or not that scenario happens to innocent developers acting in good faith is harder to guess.

}
else
{
// At this point, something unusual has occurred, likely due to misuse of this component.
// Reset the calculated item size to the user-provided item size.
_itemSize = ItemSize;
itemSizeChanged = false;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the refactoring here isn't strictly necessary, but I've tidied up the operations so it only deals with the "item size changed" steps in cases where we might be assigning new values to _itemSize. These refactorings are very safe AFAIK.


itemsInSpacer = Math.Max(0, (int)Math.Floor(spacerSize / _itemSize) - OverscanCount);
visibleItemCapacity = (int)Math.Ceiling(containerSize / _itemSize) + 2 * OverscanCount;
}

private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity)
private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity, bool itemSizeChanged)
{
if (itemsBefore != _itemsBefore || visibleItemCapacity != _visibleItemCapacity)
{
Expand All @@ -300,6 +310,13 @@ private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity)
StateHasChanged();
}
}
else if (itemSizeChanged)
{
// Even if the range of visible items hasn't changed, it's possible that we adjusted our
// estimate of the item height. In this case we need to re-render to update the spacer
// dimensions.
StateHasChanged();
}
}

private async ValueTask RefreshDataCoreAsync(bool renderOnSuccess)
Expand Down