Skip to content

Conversation

@SteveSandersonMS
Copy link
Member

Fix for #37245

This is a really small and simple change to Virtualize.cs. We just detect the invalid offset and clamp it back to the valid range. It shouldn't affect any case where the offset is valid.

Most of this PR is just the E2E test coverage.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Oct 4, 2021
@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner October 4, 2021 10:39
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

LGTM

@SteveSandersonMS
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2021

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1303305899

@SteveSandersonMS SteveSandersonMS merged commit fb05933 into main Oct 4, 2021
@SteveSandersonMS SteveSandersonMS deleted the stevesa/fix-virtualization-dataset-length-changes branch October 4, 2021 15:57
@ghost ghost added this to the 7.0-preview1 milestone Oct 4, 2021
mkArtakMSFT pushed a commit that referenced this pull request Oct 5, 2021
…7245 (#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants