Skip to content

Commit

Permalink
fix item disappearing with scroll in VirtualizedList (facebook#47965)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#47965

Changelog: [General] [Changed] - fix item disappearing with scroll in VirtualizedList

It was caused because the function `computeWindowedRenderLimits` collapsed the current window size to just 1 element. So, users start scroll current window increase from {left:0, right:5 } -> {left:0, right:6 } and after some edge cases the window collapsed to `{left:6, right:6 }` which cause to remove all elements and recreate them later. As a result users have a lot of lags and blank pages.

The diff fixes the collapsing window size to 1 element. Also fix other decreasing `left` position even if windowSize more than current amount of elements.

Reviewed By: NickGerleman

Differential Revision: D66334188

fbshipit-source-id: 2162d00d03d64ab6325c0492d87449051e68a4e9
  • Loading branch information
Tom910 authored and facebook-github-bot committed Nov 26, 2024
1 parent 016f445 commit df7b6ae
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,15 @@ const definitions: FeatureFlagDefinitions = {
purpose: 'release',
},
},
fixVirtualizeListCollapseWindowSize: {
defaultValue: false,
metadata: {
dateAdded: '2024-11-22',
description:
'Fixing an edge case where the current window size is not properly calculated with fast scrolling. Window size collapsed to 1 element even if windowSize more than the current amount of elements',
purpose: 'experimentation',
},
},
isLayoutAnimationEnabled: {
defaultValue: true,
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<42e14ee5e2201b2fcbf9fe550e5165a6>>
* @generated SignedSource<<38ad29621eeb29a9f82735dc187c13d4>>
* @flow strict
*/

Expand Down Expand Up @@ -36,6 +36,7 @@ export type ReactNativeFeatureFlagsJsOnly = {
enableAnimatedAllowlist: Getter<boolean>,
enableAnimatedClearImmediateFix: Getter<boolean>,
enableAnimatedPropsMemo: Getter<boolean>,
fixVirtualizeListCollapseWindowSize: Getter<boolean>,
isLayoutAnimationEnabled: Getter<boolean>,
shouldSkipStateUpdatesForLoopingAnimations: Getter<boolean>,
shouldUseAnimatedObjectForTransform: Getter<boolean>,
Expand Down Expand Up @@ -143,6 +144,11 @@ export const enableAnimatedClearImmediateFix: Getter<boolean> = createJavaScript
*/
export const enableAnimatedPropsMemo: Getter<boolean> = createJavaScriptFlagGetter('enableAnimatedPropsMemo', true);

/**
* Fixing an edge case where the current window size is not properly calculated with fast scrolling. Window size collapsed to 1 element even if windowSize more than the current amount of elements
*/
export const fixVirtualizeListCollapseWindowSize: Getter<boolean> = createJavaScriptFlagGetter('fixVirtualizeListCollapseWindowSize', false);

/**
* Function used to enable / disabled Layout Animations in React Native.
*/
Expand Down
16 changes: 14 additions & 2 deletions packages/virtualized-lists/Lists/VirtualizeUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type ListMetricsAggregator, {
CellMetricProps,
} from './ListMetricsAggregator';

import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags';

/**
* Used to find the indices of the frames that overlap the given offsets. Useful for finding the
* items that bound different windows of content, such as the visible area or the buffered overscan
Expand Down Expand Up @@ -176,10 +178,20 @@ export function computeWindowedRenderLimits(
break;
}
const maxNewCells = newCellCount >= maxToRenderPerBatch;
const firstWillAddMore = first <= prev.first || first > prev.last;

let firstWillAddMore;
let lastWillAddMore;

if (ReactNativeFeatureFlags.fixVirtualizeListCollapseWindowSize()) {
firstWillAddMore = first <= prev.first;
lastWillAddMore = last >= prev.last;
} else {
firstWillAddMore = first <= prev.first || first > prev.last;
lastWillAddMore = last >= prev.last || last < prev.first;
}

const firstShouldIncrement =
first > overscanFirst && (!maxNewCells || !firstWillAddMore);
const lastWillAddMore = last >= prev.last || last < prev.first;
const lastShouldIncrement =
last < overscanLast && (!maxNewCells || !lastWillAddMore);
if (maxNewCells && !firstShouldIncrement && !lastShouldIncrement) {
Expand Down
160 changes: 159 additions & 1 deletion packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@

'use strict';

import {elementsThatOverlapOffsets, newRangeCount} from '../VirtualizeUtils';
import ListMetricsAggregator from '../ListMetricsAggregator';
import {
computeWindowedRenderLimits,
elementsThatOverlapOffsets,
newRangeCount,
} from '../VirtualizeUtils';
import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags';

describe('newRangeCount', function () {
it('handles subset', function () {
Expand Down Expand Up @@ -116,3 +122,155 @@ function fakeProps(length) {
getItemCount: () => length,
};
}

describe('computeWindowedRenderLimits', function () {
const defaultProps = {
getItemCount: () => 10,
getItem: (_, index) => ({key: `test_${index}`}),
getItemLayout: (_, index) => {
return {index, length: 100, offset: index * 100};
},
};

const defaultScrollMetrics = {
dt: 16,
dOffset: 0,
offset: 0,
timestamp: 0,
velocity: 0,
visibleLength: 500,
zoomScale: 1,
};

it('renders all items when list is small', function () {
const props = {
...defaultProps,
getItemCount: () => 3,
};
const result = computeWindowedRenderLimits(
props,
5,
10,
{first: 0, last: 2},
new ListMetricsAggregator(),
defaultScrollMetrics,
);
expect(result).toEqual({first: 0, last: 2});
});

it('handles overflow cases when window size suddenly collapses', function () {
ReactNativeFeatureFlags.override({
fixVirtualizeListCollapseWindowSize: () => true,
});

const listMetricsAggregator = new ListMetricsAggregator();
listMetricsAggregator._contentLength = 2713.60009765625;

const offsets = [
{index: 0, length: 275, offset: 0},
{index: 1, length: 352, offset: 275},
{index: 2, length: 326, offset: 627},
{index: 3, length: 352, offset: 953},
{index: 4, length: 293, offset: 1305},
{index: 5, length: 293, offset: 1598},
{index: 6, length: 293, offset: 1891},
{index: 7, length: 293, offset: 2184},
];

expect(
computeWindowedRenderLimits(
{
...defaultProps,
getItemCount: () => 8,
getItemLayout: (_, index) => {
return offsets[index];
},
},
1,
31,
{first: 0, last: 5},
listMetricsAggregator,
{
dt: 949,
dOffset: 879.2000732421875,
offset: 2073.60009765625,
timestamp: 1732180589708,
velocity: 0.9264489707504611,
visibleLength: 640,
},
),
).toEqual({first: 0, last: 6});
});

it('handles reaching the end of the list', function () {
const listMetricsAggregator = new ListMetricsAggregator();
listMetricsAggregator._contentLength = 1000;

const offsets = Array.from({length: 10}, (_, index) => ({
index,
length: 100,
offset: index * 100,
}));

expect(
computeWindowedRenderLimits(
{
...defaultProps,
getItemLayout: (_, index) => offsets[index],
},
2,
5,
{first: 5, last: 9},
listMetricsAggregator,
{
dt: 100,
dOffset: 100,
offset: 900,
timestamp: 1000,
velocity: 1,
visibleLength: 300,
},
),
).toEqual({first: 3, last: 9});
});

it('respects maxToRenderPerBatch when adding new cells', function () {
const scrollMetrics = {
...defaultScrollMetrics,
offset: 0,
dOffset: 0,
velocity: 0,
};
const prev = {first: 0, last: 2};
const result = computeWindowedRenderLimits(
defaultProps,
2, // maxToRenderPerBatch
5, // windowSize
prev,
new ListMetricsAggregator(),
scrollMetrics,
);
expect(result).toEqual({first: 0, last: 4});
});

it('handles case where overscanFirst and overscanLast encompass entire list', function () {
const props = {
...defaultProps,
getItemCount: () => 5,
};
const scrollMetrics = {
...defaultScrollMetrics,
offset: 0,
visibleLength: 1000,
};
const result = computeWindowedRenderLimits(
props,
5,
10, // windowSize large enough to cover entire list
{first: 0, last: 4},
new ListMetricsAggregator(),
scrollMetrics,
);
expect(result).toEqual({first: 0, last: 4});
});
});

0 comments on commit df7b6ae

Please sign in to comment.