Various virtualization improvements. Fixes #25535

[MackinnonBuck]

This PR addresses issues with <Virtualize>:

Ask-mode template

@SteveSandersonMS editing the PR description from here:

Description

There are some glitches in the new Virtualize component that will ship for the first time in RC1:

• Item size calculations can be wrong by a subpixel amount, which accumulates and produces offsets in scrolling
• When items don't have @key, scrolling can get stuck in a loop

Additionally, there wasn't a way to control the number of items fetched to use as a buffer around the visible region, which resulted in no control over the frequency of rendering.

Customer Impact

If we don't merge this, customers will very likely run into one of the above glitches almost immediately. Then they won't be able to try out <Virtualize> properly and we won't get useful feedback about it or know if our fixes are correct.

Regression?

No, this is a new feature

Risk

In the worst case, this might theoretically make <Virtualize> behave badly in a different as-yet unknown way.

Addresses #25535

[SteveSandersonMS]

I've also implemented a check to see if the calculated item size is within some margin of the user-provided item size

This seems a bit quirky. Maybe it is the right design, but I'd be interested to discuss it. What I'm concerned about is that commonly, developers think they know what the item size is, but they are wrong by a small margin of error, perhaps because of the element margin being added onto its height. My hope had been that we can calculate the actual height and thereby free developers from the consequences of being slightly wrong, whereas the rule in this PR means we couldn't help them in this way.

I understand it's hard to determine the exact height but I'm unsure how bad are the consequences of us getting it wrong by a < 1px fraction or if there are any other approaches you've considered. Let's discuss when possible!

[SteveSandersonMS]

OK, I've been looking into this, and was able to reproduce the problem with fractional heights by using the following test case in FetchData.ts:

@if (forecasts == null)
{
    <p><em>Loading...</em></p>
}
else
{
    <div style="overflow-y: auto; height: 200px; border: 1px solid red;">
        <Virtualize Items="forecasts" Context="forecast">
            <div style="border: 0.2px solid green" @key="forecast">
                <span>@forecast.Date.ToShortDateString()</span>
                <span>@forecast.TemperatureC</span>
                <span>@forecast.TemperatureF</span>
                <span>@forecast.Summary</span>
            </div>
        </Virtualize>
    </div>
}

I also changed the underlying data source to supply 500 items. Without your fix that uses getBoundingClientRect, the behavior goes wrong when scrolling quickly because it misjudges its location within the scroll range because the rounding errors add up. In the example above, it just stops adding new items after a certain point, so the user scrolls into a blank white region.

Then, after adding your fix that uses getBoundingClientRect, the problem goes away. I could no longer repro any weirdness in the above example.

So, my remaining two questions are:

• Why would it be better to use the developer's own provided ItemSize value? It's hard to see how the developer would be able to give a more accurate value than you're calculating at the subpixel level. Your value is more accurate than what's displayed in the browser dev tools, which round to 2dp.
  • Do you have any repro scenario where things still go wrong after applying the getBoundingClientRect fix?
• Do we care about supporting the case where the developer fails to provide @key? I found that, if not providing that, the scrolling sometimes goes mad and continues scrolling even after I release the mouse wheel. It's a bit worrying because I don't have an exact reason for why it happens or why @key fixes it, or if there might be some case where @key doesn't fix it.
  • My best guess is there may be some heuristic within the browser whereby when using wheel scrolling (or perhaps with drag scrolling too), it picks an end point that lines up with one of the existing elements, and continues scrolling until that point is reached. But if we're not using @key, then it never reaches that point because we keep moving the target element further in the direction you're already scrolling. If this guess is correct, it explains why @key fixes it (because the target element gets removed from the DOM and the process ends). However this guess doesn't predict any good solution other than @key.
    • For my own future reference: we did consider forcing the equivalent of @key by wrapping each item inside a region with sequence number given by the index into the collection. However this behaves badly if items are inserted/removed, as it incorrectly would match/mismatch items against the earlier output. So it's better to force developers to give a @key manually corresponding to the actual collection entry or its ID.
  • Personally I think we could consider it a reasonable requirement that you must use @key for good performance and reliable scrolling. Certainly it's the most common case. It would just be great if we could avoid the weirdness in the non-key case too, or somehow detect the failure to use @key, but don't have a good suggestion for doing either right now.

[MackinnonBuck]

Thanks for the detailed follow-up, @SteveSandersonMS!

Why would it be better to use the developer's own provided ItemSize value?

I do recall that the getBoundingClientRect fix would appear to be off by a tenth of a pixel at times, so I left in the logic to snap that value to ItemSize if it's that close. I don't remember if I observed any usability issues with the getBoundingClientRect solution if I left out the rounding logic, so I'll investigate this further. If I'm not able to recreate the issue without the rounding fix, I think it's fine to remove that bit of logic.

Do we care about supporting the case where the developer fails to provide @key?

What's funny is that @danroth27 experienced this exact issue, showed me the strange behavior he was getting, and even I didn't initially recognize that the issue was the lack of a @key. I think it's something easy to forget, so we would definitely have to document it in a visible way if we decide to make it a requirement. It also might be a little confusing that PlaceholderTemplate doesn't require a @key while ItemContent does. Overall, I think it's fine if we require a @key unless there's an easy way to do without it.

[MackinnonBuck]

Quick update: It does look like removing the custom rounding logic results in the scrollbar being slightly out of sync with the mouse. I didn't notice any strange behavior when using the scroll wheel, just when clicking and dragging the scrollbar. I think it would be nice to be able to avoid using ItemSize beyond the initial render, but it might require an entirely different technique.

If we could wrap each item in a <div>, we could solve both the @key problem and item size calculation because we could just make the @key be the item context, and just calculate the height of a single item the first time one is rendered. However, this breaks things for things like tables, where it's illegal to have <div>s in certain places.

[ghost]

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

[SteveSandersonMS]

@mkArtakMSFT This is rebased to 5.0-rc2 now. Could you please merge it?

[Pilchie]

Approved for 5.0 RC2.