Skip to content

Commit 683444f

Browse files
CopilotPureWeen
andauthored
Correct Virtualize measurement ownership edge cases
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
1 parent 5221098 commit 683444f

7 files changed

Lines changed: 447 additions & 42 deletions

File tree

src/Components/Web.JS/src/Virtualize.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -694,9 +694,7 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac
694694
const targetRect = target.getBoundingClientRect();
695695
const targetIntersectionTop = Math.max(targetRect.top, intersectionTop);
696696
const targetIntersectionBottom = Math.min(targetRect.bottom, intersectionBottom);
697-
const isZeroHeightIntersection = targetRect.height === 0 && targetIntersectionBottom === targetIntersectionTop;
698-
if (targetIntersectionBottom < targetIntersectionTop
699-
|| (targetIntersectionBottom === targetIntersectionTop && !isZeroHeightIntersection)) {
697+
if (targetIntersectionBottom < targetIntersectionTop) {
700698
continue;
701699
}
702700

@@ -791,6 +789,7 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac
791789
restoreAnchor: restoreAnchorForShift,
792790
alignToItem: alignToItemAt,
793791
beginProgrammaticScroll: beginProgrammaticScroll,
792+
reobserveSpacers,
794793
anchorSnapshot: null as { anchorItemIndex: number; anchorOffset: number; scrollTop: number } | null,
795794
onDispose: () => {
796795
mutationObserver.disconnect();
@@ -989,14 +988,19 @@ function init(dotNetHelper: DotNet.DotNetObject, spacerBefore: HTMLElement, spac
989988
}
990989

991990
const methodName = isBefore ? 'OnSpacerBeforeVisible' : 'OnSpacerAfterVisible';
992-
dotNetHelper.invokeMethodAsync(
991+
const callback = dotNetHelper.invokeMethodAsync(
993992
methodName,
994993
measurement.spacerSize,
995994
measurement.spacerSeparation,
996995
measurement.containerSize,
997996
reason,
998997
measurement.renderedWindowVersion
999998
);
999+
void Promise.resolve(callback).then(isCurrentMeasurement => {
1000+
if (isCurrentMeasurement === false) {
1001+
reobserveSpacers();
1002+
}
1003+
});
10001004
});
10011005

10021006
if (source === ScrollSource.AlignToItem) {

src/Components/Web.JS/test/Virtualize.test.ts

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ describe('Virtualize exports', () => {
1717
describe('Virtualize intersection measurements', () => {
1818
let intersectionCallback: IntersectionObserverCallback;
1919
let spacerSeparation: number;
20+
let observe: jest.Mock;
21+
let unobserve: jest.Mock;
2022

2123
beforeEach(() => {
2224
document.body.innerHTML = '';
2325
spacerSeparation = 600;
26+
observe = jest.fn();
27+
unobserve = jest.fn();
28+
invokeMethodAsync.mockReset().mockResolvedValue(true);
2429

2530
Object.defineProperty(globalThis, 'CSS', {
2631
configurable: true,
@@ -33,8 +38,8 @@ describe('Virtualize intersection measurements', () => {
3338
intersectionCallback = callback;
3439
}
3540

36-
observe() {}
37-
unobserve() {}
41+
observe(target: Element) { observe(target); }
42+
unobserve(target: Element) { unobserve(target); }
3843
disconnect() {}
3944
},
4045
});
@@ -60,7 +65,7 @@ describe('Virtualize intersection measurements', () => {
6065
jest.restoreAllMocks();
6166
});
6267

63-
const invokeMethodAsync = jest.fn();
68+
const invokeMethodAsync = jest.fn<(...args: unknown[]) => Promise<boolean>>();
6469
const dotNetHelper = {
6570
_callDispatcher: {},
6671
_id: 1,
@@ -254,6 +259,82 @@ describe('Virtualize intersection measurements', () => {
254259
2,
255260
2);
256261
});
262+
263+
test.each([
264+
['edge-adjacent nonzero spacer', -60, 10, true],
265+
['separated spacer', -61, 10, false],
266+
['edge-adjacent zero-height spacer', -50, 0, true],
267+
['overlapping spacer', -59, 10, true],
268+
])('matches threshold-zero intersection semantics for %s', (_, top, height, expectedCallback) => {
269+
const container = document.createElement('div');
270+
container.style.overflowY = 'auto';
271+
const spacerBefore = document.createElement('div');
272+
const item = document.createElement('div');
273+
const spacerAfter = document.createElement('div');
274+
spacerBefore.style.overflowY = 'visible';
275+
container.append(spacerBefore, item, spacerAfter);
276+
document.body.append(container);
277+
278+
setElementMetrics(container, rect(0, 200), 200);
279+
setElementMetrics(spacerBefore, rect(top, height), height);
280+
setElementMetrics(item, rect(20, 50), 50);
281+
setElementMetrics(spacerAfter, rect(1000, 100), 100);
282+
spacerBefore.setAttribute(renderedWindowVersionAttribute, '1');
283+
spacerAfter.setAttribute(renderedWindowVersionAttribute, '1');
284+
285+
invokeMethodAsync.mockClear();
286+
Virtualize.init(dotNetHelper, spacerBefore, spacerAfter);
287+
intersectionCallback([{
288+
target: spacerBefore,
289+
isIntersecting: expectedCallback,
290+
} as unknown as IntersectionObserverEntry], {} as IntersectionObserver);
291+
292+
expect(invokeMethodAsync).toHaveBeenCalledTimes(expectedCallback ? 1 : 0);
293+
});
294+
295+
test('reobserves spacers only when managed code rejects a stale measurement', async () => {
296+
jest.useFakeTimers();
297+
const container = document.createElement('div');
298+
container.style.overflowY = 'auto';
299+
const spacerBefore = document.createElement('div');
300+
const item = document.createElement('div');
301+
const spacerAfter = document.createElement('div');
302+
spacerBefore.style.overflowY = 'visible';
303+
container.append(spacerBefore, item, spacerAfter);
304+
document.body.append(container);
305+
306+
setElementMetrics(container, rect(0, 200), 200);
307+
setElementMetrics(spacerBefore, rect(-10, 20), 20);
308+
setElementMetrics(item, rect(10, 50), 50);
309+
setElementMetrics(spacerAfter, rect(1000, 100), 100);
310+
spacerBefore.setAttribute(renderedWindowVersionAttribute, '1');
311+
spacerAfter.setAttribute(renderedWindowVersionAttribute, '1');
312+
313+
invokeMethodAsync.mockReset().mockResolvedValueOnce(false);
314+
Virtualize.init(dotNetHelper, spacerBefore, spacerAfter);
315+
observe.mockClear();
316+
unobserve.mockClear();
317+
318+
const entry = {
319+
target: spacerBefore,
320+
isIntersecting: true,
321+
} as unknown as IntersectionObserverEntry;
322+
intersectionCallback([entry], {} as IntersectionObserver);
323+
await Promise.resolve();
324+
325+
expect(unobserve).toHaveBeenCalledTimes(2);
326+
expect(observe).toHaveBeenCalledTimes(2);
327+
328+
jest.advanceTimersByTime(50);
329+
observe.mockClear();
330+
unobserve.mockClear();
331+
invokeMethodAsync.mockResolvedValueOnce(true);
332+
intersectionCallback([entry], {} as IntersectionObserver);
333+
await Promise.resolve();
334+
335+
expect(unobserve).not.toHaveBeenCalled();
336+
expect(observe).not.toHaveBeenCalled();
337+
});
257338
});
258339

259340
function rect(top: number, height: number): DOMRect {

src/Components/Web/src/Virtualization/IVirtualizeJsCallbacks.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Microsoft.AspNetCore.Components.Web.Virtualization;
55

66
internal interface IVirtualizeJsCallbacks
77
{
8-
void OnBeforeSpacerVisible(float spacerSize, float spacerSeparation, float containerSize, SpacerVisibilityReason reason, long renderedWindowVersion);
9-
void OnAfterSpacerVisible(float spacerSize, float spacerSeparation, float containerSize, SpacerVisibilityReason reason, long renderedWindowVersion);
8+
bool OnBeforeSpacerVisible(float spacerSize, float spacerSeparation, float containerSize, SpacerVisibilityReason reason, long renderedWindowVersion);
9+
bool OnAfterSpacerVisible(float spacerSize, float spacerSeparation, float containerSize, SpacerVisibilityReason reason, long renderedWindowVersion);
1010
void OnAlignmentCompleted(VirtualizeAlignmentResult result);
1111
}

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

Lines changed: 80 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ public sealed class Virtualize<TItem> : ComponentBase, IVirtualizeJsCallbacks, I
4444

4545
internal long _renderedWindowVersion;
4646

47+
private long _contentRevision;
48+
49+
private RenderedWindowIdentity? _lastRenderedWindowIdentity;
50+
51+
private TItem[]? _lastLoadedItemsSnapshot;
52+
4753
internal float _itemSize;
4854

4955
private float _lastSetItemSize;
@@ -546,7 +552,12 @@ protected override void BuildRenderTree(RenderTreeBuilder builder)
546552
throw oldRefreshException;
547553
}
548554

549-
var renderedWindowVersion = ++_renderedWindowVersion;
555+
var renderedWindowIdentity = GetRenderedWindowIdentity();
556+
if (renderedWindowIdentity != _lastRenderedWindowIdentity)
557+
{
558+
_renderedWindowVersion++;
559+
}
560+
var renderedWindowVersion = _renderedWindowVersion;
550561

551562
builder.OpenElement(0, SpacerElement);
552563
builder.AddAttribute(1, "data-blazor-virtualize-reserved-height", GetSpacerHeightPx(_itemsBefore));
@@ -628,8 +639,26 @@ protected override void BuildRenderTree(RenderTreeBuilder builder)
628639
builder.AddElementReferenceCapture(14, elementReference => _spacerAfter = elementReference);
629640

630641
builder.CloseElement();
642+
643+
_lastRenderedWindowIdentity = GetRenderedWindowIdentity();
631644
}
632645

646+
private RenderedWindowIdentity GetRenderedWindowIdentity()
647+
=> new(
648+
_itemsBefore,
649+
_visibleItemCapacity,
650+
_unusedItemCapacity,
651+
_itemCount,
652+
_loadedItemsStartIndex,
653+
_lastRenderedItemCount,
654+
_lastRenderedPlaceholderCount,
655+
_itemSize,
656+
_totalMeasuredHeight,
657+
_measuredItemCount,
658+
_loading,
659+
_contentRevision,
660+
SpacerElement);
661+
633662
private string GetSpacerHeightPx(int itemCount)
634663
=> (itemCount * GetItemHeight()).ToString(CultureInfo.InvariantCulture);
635664

@@ -647,39 +676,39 @@ private void CancelInFlightScrollForUserInteraction()
647676
}
648677
}
649678

650-
void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(
679+
bool IVirtualizeJsCallbacks.OnBeforeSpacerVisible(
651680
float spacerSize,
652681
float spacerSeparation,
653682
float containerSize,
654683
SpacerVisibilityReason reason,
655684
long renderedWindowVersion)
656685
{
657-
if (renderedWindowVersion != _renderedWindowVersion)
658-
{
659-
return;
660-
}
661-
662686
if (_pendingAnchorRestore)
663687
{
664-
return;
688+
return true;
665689
}
666690
if (_initialIndex.Phase == InitialIndexPhase.None && InitialItemIndex > 0)
667691
{
668-
return;
692+
return true;
693+
}
694+
if (reason == SpacerVisibilityReason.UserScroll)
695+
{
696+
CancelInFlightScrollForUserInteraction();
697+
}
698+
if (renderedWindowVersion != _renderedWindowVersion)
699+
{
700+
return false;
669701
}
670702
switch (reason)
671703
{
672704
case SpacerVisibilityReason.ProgrammaticScroll:
673-
return;
674-
case SpacerVisibilityReason.UserScroll:
675-
CancelInFlightScrollForUserInteraction();
676-
break;
705+
return true;
677706
case SpacerVisibilityReason.ViewportFill:
678707
// A fill callback while our own scroll is in flight is a side effect of that scroll —
679708
// acting on it would move the target.
680709
if (_currentScrollCts is not null)
681710
{
682-
return;
711+
return true;
683712
}
684713
break;
685714
}
@@ -692,7 +721,7 @@ void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(
692721
ViewportFillDirection.Before,
693722
visibleItemCapacity,
694723
unusedItemCapacity);
695-
return;
724+
return true;
696725
}
697726

698727
// Slide window up by at least one if spacer is visible but position unchanged.
@@ -702,33 +731,33 @@ void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(
702731
}
703732

704733
UpdateItemDistribution(itemsBefore, visibleItemCapacity, unusedItemCapacity);
734+
return true;
705735
}
706736

707-
void IVirtualizeJsCallbacks.OnAfterSpacerVisible(
737+
bool IVirtualizeJsCallbacks.OnAfterSpacerVisible(
708738
float spacerSize,
709739
float spacerSeparation,
710740
float containerSize,
711741
SpacerVisibilityReason reason,
712742
long renderedWindowVersion)
713743
{
714-
if (renderedWindowVersion != _renderedWindowVersion)
715-
{
716-
return;
717-
}
718-
719744
if (_pendingAnchorRestore || reason == SpacerVisibilityReason.ProgrammaticScroll)
720745
{
721-
return;
746+
return true;
722747
}
723748
if (reason == SpacerVisibilityReason.UserScroll)
724749
{
725750
CancelInFlightScrollForUserInteraction();
726751
}
727-
else if (reason == SpacerVisibilityReason.ViewportFill && _currentScrollCts is not null)
752+
if (renderedWindowVersion != _renderedWindowVersion)
753+
{
754+
return false;
755+
}
756+
if (reason == SpacerVisibilityReason.ViewportFill && _currentScrollCts is not null)
728757
{
729758
// Bottom-spacer fill while our own scroll is in flight: the window moved but scrollTop hasn't
730759
// landed, so acting on it would undo the target. The real fill runs once the scroll completes.
731-
return;
760+
return true;
732761
}
733762
var hadNewMeasurements = CalculateItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity, out var unusedItemCapacity);
734763

@@ -738,7 +767,7 @@ void IVirtualizeJsCallbacks.OnAfterSpacerVisible(
738767
ViewportFillDirection.After,
739768
visibleItemCapacity,
740769
unusedItemCapacity);
741-
return;
770+
return true;
742771
}
743772

744773
var itemsBefore = Math.Max(0, _itemCount - itemsAfter - visibleItemCapacity);
@@ -761,6 +790,7 @@ void IVirtualizeJsCallbacks.OnAfterSpacerVisible(
761790
}
762791

763792
UpdateItemDistribution(itemsBefore, visibleItemCapacity, unusedItemCapacity);
793+
return true;
764794
}
765795

766796
void IVirtualizeJsCallbacks.OnAlignmentCompleted(VirtualizeAlignmentResult result)
@@ -1083,7 +1113,15 @@ private async ValueTask RefreshDataCoreAsync(bool renderOnSuccess, CancellationT
10831113
}
10841114

10851115
_itemCount = result.TotalItemCount;
1086-
_loadedItems = result.Items;
1116+
var loadedItems = result.Items.ToArray();
1117+
if (_itemsProvider != DefaultItemsProvider
1118+
|| _lastLoadedItemsSnapshot is null
1119+
|| !_lastLoadedItemsSnapshot.SequenceEqual(loadedItems))
1120+
{
1121+
_contentRevision++;
1122+
}
1123+
_lastLoadedItemsSnapshot = loadedItems;
1124+
_loadedItems = loadedItems;
10871125
_loadedItemsStartIndex = _itemsBefore;
10881126

10891127
// For DefaultItemsProvider, capture the first loaded item so we can detect
@@ -1106,6 +1144,7 @@ private async ValueTask RefreshDataCoreAsync(bool renderOnSuccess, CancellationT
11061144
StateHasChanged();
11071145
}
11081146
}
1147+
11091148
catch (Exception e)
11101149
{
11111150
if (e is OperationCanceledException oce && oce.CancellationToken == cancellationToken)
@@ -1230,6 +1269,21 @@ public async ValueTask DisposeAsync()
12301269
}
12311270
}
12321271

1272+
private readonly record struct RenderedWindowIdentity(
1273+
int ItemsBefore,
1274+
int VisibleItemCapacity,
1275+
int UnusedItemCapacity,
1276+
int ItemCount,
1277+
int LoadedItemsStartIndex,
1278+
int LastRenderedItemCount,
1279+
int LastRenderedPlaceholderCount,
1280+
float ItemSize,
1281+
float TotalMeasuredHeight,
1282+
int MeasuredItemCount,
1283+
bool IsLoading,
1284+
long ContentRevision,
1285+
string SpacerElement);
1286+
12331287
private enum InitialIndexPhase
12341288
{
12351289
None,

0 commit comments

Comments
 (0)