Skip to content

Commit 0146d25

Browse files
Fix virtualization handling of large dataset length changes. Fixes #37245 (#37250)
Backport of #37247 to release/6.0 /cc @SteveSandersonMS ## Description/Customer Impact The `Virtualize` component exhibits pathological behavior in the following case: * User scrolls to the end of a very long list (e.g., 100000 items) * The list becomes much shorter (e.g., 100 items) Instead of jumping back to the new maximum scroll position, it incrementally steps back to it one page at a time. In the above example, if the scroll region is tall enough for 20 items, that would take (100000-100)/20 = 4995 steps. On each step it requests a new data set (possibly causing an HTTP request and DB query) and a UI re-render. Eventually this does complete, but it's extraordinarily inefficient. It's very trivial to fix this so that we move to the correct final scroll offset in a single step. ## Regression? - [ ] Yes - [X] No ## Risk - [ ] High - [ ] Medium - [X] Low * Change was verified manually and via a new E2E test * The code change is trivial: we can detect and correct the invalid scroll offset in a single step. The new logic shouldn't affect any existing case where the scroll offset was valid. ## Verification - [X] Manual (required) - [X] Automated ## Packaging changes reviewed? - [ ] Yes - [ ] No - [X] N/A Fixes #37245
1 parent 134a0a3 commit 0146d25

File tree

3 files changed

+112
-0
lines changed

3 files changed

+112
-0
lines changed

src/Components/Web/src/Virtualization/Virtualize.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,14 @@ private void CalcualteItemDistribution(
308308

309309
private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity)
310310
{
311+
// If the itemcount just changed to a lower number, and we're already scrolled past the end of the new
312+
// reduced set of items, clamp the scroll position to the new maximum
313+
if (itemsBefore + visibleItemCapacity > _itemCount)
314+
{
315+
itemsBefore = Math.Max(0, _itemCount - visibleItemCapacity);
316+
}
317+
318+
// If anything about the offset changed, re-render
311319
if (itemsBefore != _itemsBefore || visibleItemCapacity != _visibleItemCapacity)
312320
{
313321
_itemsBefore = itemsBefore;

src/Components/test/E2ETest/Tests/VirtualizationTest.cs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.AspNetCore.Testing;
1212
using OpenQA.Selenium;
1313
using OpenQA.Selenium.Support.Extensions;
14+
using OpenQA.Selenium.Support.UI;
1415
using Xunit;
1516
using Xunit.Abstractions;
1617

@@ -366,10 +367,69 @@ public void CanRefreshItemsProviderResultsInPlace()
366367
name => Assert.Equal("Person 3", name));
367368
}
368369

370+
[Fact]
371+
public void CanExpandDataSetAndRetainScrollPosition()
372+
{
373+
Browser.MountTestComponent<VirtualizationDataChanges>();
374+
var dataSetLengthSelector = new SelectElement(Browser.Exists(By.Id("large-dataset-length")));
375+
var dataSetLengthLastRendered = () => int.Parse(Browser.FindElement(By.Id("large-dataset-length-lastrendered")).Text, CultureInfo.InvariantCulture);
376+
var container = Browser.Exists(By.Id("removing-many"));
377+
378+
// Scroll to the end of a medium list
379+
dataSetLengthSelector.SelectByText("1000");
380+
Browser.Equal(1000, dataSetLengthLastRendered);
381+
Browser.True(() =>
382+
{
383+
ScrollToEnd(Browser, container);
384+
return GetPeopleNames(container).Contains("Person 1000");
385+
});
386+
387+
// Expand the data set
388+
dataSetLengthSelector.SelectByText("100000");
389+
Browser.Equal(100000, dataSetLengthLastRendered);
390+
391+
// See that the old data is still visible, because the scroll position is preserved as a pixel count,
392+
// not a scroll percentage
393+
Browser.True(() => GetPeopleNames(container).Contains("Person 1000"));
394+
}
395+
396+
[Fact]
397+
public void CanHandleDataSetShrinkingWithExistingOffsetAlreadyBeyondNewListEnd()
398+
{
399+
// Represents https://github.com/dotnet/aspnetcore/issues/37245
400+
Browser.MountTestComponent<VirtualizationDataChanges>();
401+
var dataSetLengthSelector = new SelectElement(Browser.Exists(By.Id("large-dataset-length")));
402+
var dataSetLengthLastRendered = () => int.Parse(Browser.FindElement(By.Id("large-dataset-length-lastrendered")).Text, CultureInfo.InvariantCulture);
403+
var container = Browser.Exists(By.Id("removing-many"));
404+
405+
// Scroll to the end of a very long list
406+
dataSetLengthSelector.SelectByText("100000");
407+
Browser.Equal(100000, dataSetLengthLastRendered);
408+
Browser.True(() =>
409+
{
410+
ScrollToEnd(Browser, container);
411+
return GetPeopleNames(container).Contains("Person 100000");
412+
});
413+
414+
// Now make the dataset much shorter
415+
// We should automatically have the scroll position reduced to the new maximum
416+
// Because the new data set is *so much* shorter than the previous one, if bug #37245 were still here,
417+
// this would take over 30 minutes so the test would fail
418+
dataSetLengthSelector.SelectByText("25");
419+
Browser.Equal(25, dataSetLengthLastRendered);
420+
Browser.True(() => GetPeopleNames(container).Contains("Person 25"));
421+
}
422+
369423
private string[] GetPeopleNames(IWebElement container)
370424
{
371425
var peopleElements = container.FindElements(By.CssSelector(".person span"));
372426
return peopleElements.Select(element => element.Text).ToArray();
373427
}
428+
429+
private static void ScrollToEnd(IWebDriver browser, IWebElement elem)
430+
{
431+
var js = (IJavaScriptExecutor)browser;
432+
js.ExecuteScript("arguments[0].scrollTop = arguments[0].scrollHeight", elem);
433+
}
374434
}
375435
}

src/Components/test/testassets/BasicTestApp/VirtualizationDataChanges.razor

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,34 @@
3838
</Virtualize>
3939
</div>
4040

41+
<h4>Large data set with size changes</h4>
42+
<p>
43+
Data set length:
44+
<select id="large-dataset-length" value="@largeDataSetLength" @onchange="@(e => SetLargeDataSetLengthAsync(int.Parse((string)e.Value)))">
45+
<option>25</option>
46+
<option>1000</option>
47+
<option>100000</option>
48+
</select>
49+
Last rendered:
50+
<span id="large-dataset-length-lastrendered">@largeDataSetLengthLastRendered</span>
51+
</p>
52+
<div id="removing-many" style="overflow-y: auto; height: 200px; border: 1px dashed gray;">
53+
<Virtualize @ref="removeManyVirtualize" ItemsProvider="GetLargeDataSetAsync">
54+
<div @key="@context.Name" class="person" style="border-bottom: 1px solid silver; padding: 4px;">
55+
<span>@context.Name</span>
56+
</div>
57+
</Virtualize>
58+
</div>
59+
4160
@code {
4261
Virtualize<Person> asyncVirtualize;
62+
Virtualize<Person> removeManyVirtualize;
4363
List<Person> fixedPeople = Enumerable.Range(1, 3).Select(GeneratePerson).ToList();
4464
int numPeopleInItemsProvider = 3;
4565

66+
int largeDataSetLength = 25;
67+
int? largeDataSetLengthLastRendered;
68+
4669
void AddPersonToFixedList()
4770
{
4871
// When using Items (not ItemsProvider), the Virtualize component re-queries
@@ -69,6 +92,27 @@
6992
numPeopleInItemsProvider);
7093
}
7194

95+
async Task SetLargeDataSetLengthAsync(int length)
96+
{
97+
largeDataSetLength = length;
98+
await removeManyVirtualize.RefreshDataAsync();
99+
}
100+
101+
async ValueTask<ItemsProviderResult<Person>> GetLargeDataSetAsync(ItemsProviderRequest request)
102+
{
103+
await Task.Delay(500);
104+
largeDataSetLengthLastRendered = largeDataSetLength;
105+
106+
// Behave like a .Skip(startIndex).Take(count), so that if you ask for data beyond the end of the
107+
// set, you get back nothing
108+
var lastIndexExcl = Math.Min(request.StartIndex + request.Count, largeDataSetLength);
109+
var effectiveCount = lastIndexExcl - request.StartIndex;
110+
var resultItems = effectiveCount <= 0
111+
? Enumerable.Empty<Person>()
112+
: Enumerable.Range(1 + request.StartIndex, effectiveCount).Select(GeneratePerson).ToList();
113+
return new ItemsProviderResult<Person>(resultItems, largeDataSetLength);
114+
}
115+
72116
class Person
73117
{
74118
public string Name { get; set; }

0 commit comments

Comments
 (0)