Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix VirtualizedList with initialScrollIndex not rendering all elements when data is updated (#33558)

Browse Source 
Summary:
Fixes https://github.com/facebook/react-native/issues/33529 (note that I reproduced the bug on iOS too).

The bug comes from the fact that we were using `this._scrollMetrics.offset` to determine if the initial scroll was done. But sometimes it equals 0 even after the initial scroll is done, for example when the content does not fill the list. So I replaced it with `this._hasDoneInitialScroll`.
I believe that `this._hasDoneInitialScroll` was not used in the first place because it was introduced later (3 years ago vs 5 years ago for the original code).

The replacement correctly fixes the broken test case and the example given in the issue.

Then I had to update two test cases (rename the first and remove the second), that shows explicitly the broken behavior:
we have to simulate the initial scroll for the content to be adjusted, so when the content does not fill the view and the scroll cannot be executed, the content is not adjusted.

## Changelog

[General] [Fix] - Fix VirtualizedList with initialScrollIndex not rendering all elements when data is updated

Pull Request resolved: https://github.com/facebook/react-native/pull/33558

Test Plan:
- I added a broken test case based on the issue
- I tested with the RNTesterApp using the code example given in the issue

Reviewed By: ryancat

Differential Revision: D35503114

Pulled By: yungsters

fbshipit-source-id: 67bb75d7cf1ebac0d59127d0d45afbaa3167dcf3
This commit is contained in:
Antoine Doubovetzky
2022-04-20 10:26:50 -07:00
committed by Facebook GitHub Bot
parent 2c52131f5e
commit c5c17985da
3 changed files with 121 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Libraries/Lists/VirtualizedList.js
+1 -1
View File
@@ -1754,7 +1754,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
// we'll wipe out the initialNumToRender rendered elements starting at initialScrollIndex.
// So let's wait until we've scrolled the view to the right place. And until then,
// we will trust the initialScrollIndex suggestion.
if (!this.props.initialScrollIndex || this._scrollMetrics.offset) {
if (!this.props.initialScrollIndex || this._hasDoneInitialScroll) {
newState = computeWindowedRenderLimits(
this.props.data,
this.props.getItemCount,
Libraries/Lists/__tests__/VirtualizedList-test.js
+50 -35
View File
@@ -862,40 +862,7 @@ it('does not adjust render area until content area layed out', () => {
expect(component).toMatchSnapshot();
});
it('does not adjust render area with non-zero initialScrollIndex until scrolled', () => {
const items = generateItems(20);
const ITEM_HEIGHT = 10;
let component;
ReactTestRenderer.act(() => {
component = ReactTestRenderer.create(
<VirtualizedList
initialNumToRender={5}
initialScrollIndex={1}
windowSize={10}
maxToRenderPerBatch={10}
{...baseItemProps(items)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});
ReactTestRenderer.act(() => {
simulateLayout(component, {
viewport: {width: 10, height: 50},
content: {width: 10, height: 200},
});
performAllBatches();
});
// Layout information from before the time we scroll to initial index may not
// correspond to the area "initialScrollIndex" points to. Expect only the 5
// initial items (starting at initialScrollIndex) to be rendered after
// processing all batch work, even though the windowSize allows for more.
expect(component).toMatchSnapshot();
});
it('adjusts render area with non-zero initialScrollIndex after scrolled', () => {
it('adjusts render area with non-zero initialScrollIndex', () => {
const items = generateItems(20);
const ITEM_HEIGHT = 10;
@@ -919,7 +886,6 @@ it('adjusts render area with non-zero initialScrollIndex after scrolled', () =>
content: {width: 10, height: 200},
});
simulateScroll(component, {x: 0, y: 10});
performAllBatches();
});
@@ -928,6 +894,55 @@ it('adjusts render area with non-zero initialScrollIndex after scrolled', () =>
expect(component).toMatchSnapshot();
});
it('renders new items when data is updated with non-zero initialScrollIndex', () => {
const items = generateItems(2);
const ITEM_HEIGHT = 10;
let component;
ReactTestRenderer.act(() => {
component = ReactTestRenderer.create(
<VirtualizedList
initialNumToRender={5}
initialScrollIndex={1}
windowSize={10}
maxToRenderPerBatch={10}
{...baseItemProps(items)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});
ReactTestRenderer.act(() => {
simulateLayout(component, {
viewport: {width: 10, height: 50},
content: {width: 10, height: 200},
});
performAllBatches();
});
const newItems = generateItems(4);
ReactTestRenderer.act(() => {
component.update(
<VirtualizedList
initialNumToRender={5}
initialScrollIndex={1}
windowSize={10}
maxToRenderPerBatch={10}
{...baseItemProps(newItems)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});
ReactTestRenderer.act(() => {
performAllBatches();
});
// We expect all the items to be rendered
expect(component).toMatchSnapshot();
});
it('renders initialNumToRender cells when virtualization disabled', () => {
const items = generateItems(10);
const ITEM_HEIGHT = 10;
Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
+70 -139
View File
@@ -1600,7 +1600,7 @@ exports[`VirtualizedList warns if both renderItem or ListItemComponent are speci
</RCTScrollView>
`;
exports[`adjusts render area with non-zero initialScrollIndex after scrolled 1`] = `
exports[`adjusts render area with non-zero initialScrollIndex 1`] = `
<RCTScrollView
data={
Array [
@@ -2207,144 +2207,6 @@ exports[`does not adjust render area until content area layed out 1`] = `
</RCTScrollView>
`;
exports[`does not adjust render area with non-zero initialScrollIndex until scrolled 1`] = `
<RCTScrollView
data={
Array [
Object {
"key": 0,
},
Object {
"key": 1,
},
Object {
"key": 2,
},
Object {
"key": 3,
},
Object {
"key": 4,
},
Object {
"key": 5,
},
Object {
"key": 6,
},
Object {
"key": 7,
},
Object {
"key": 8,
},
Object {
"key": 9,
},
Object {
"key": 10,
},
Object {
"key": 11,
},
Object {
"key": 12,
},
Object {
"key": 13,
},
Object {
"key": 14,
},
Object {
"key": 15,
},
Object {
"key": 16,
},
Object {
"key": 17,
},
Object {
"key": 18,
},
Object {
"key": 19,
},
]
}
getItem={[Function]}
getItemCount={[Function]}
getItemLayout={[Function]}
initialNumToRender={5}
initialScrollIndex={1}
maxToRenderPerBatch={10}
onContentSizeChange={[Function]}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
renderItem={[Function]}
scrollEventThrottle={50}
stickyHeaderIndices={Array []}
windowSize={10}
>
<View>
<View
style={
Object {
"height": 10,
}
}
/>
<View
style={null}
>
<MockCellItem
value={1}
/>
</View>
<View
style={null}
>
<MockCellItem
value={2}
/>
</View>
<View
style={null}
>
<MockCellItem
value={3}
/>
</View>
<View
style={null}
>
<MockCellItem
value={4}
/>
</View>
<View
style={null}
>
<MockCellItem
value={5}
/>
</View>
<View
style={
Object {
"height": 140,
}
}
/>
</View>
</RCTScrollView>
`;
exports[`does not over-render when there is less than initialNumToRender cells 1`] = `
<RCTScrollView
data={
@@ -3250,6 +3112,75 @@ exports[`renders items before initialScrollIndex on first batch tick when virtua
</RCTScrollView>
`;
exports[`renders new items when data is updated with non-zero initialScrollIndex 1`] = `
<RCTScrollView
data={
Array [
Object {
"key": 0,
},
Object {
"key": 1,
},
Object {
"key": 2,
},
Object {
"key": 3,
},
]
}
getItem={[Function]}
getItemCount={[Function]}
getItemLayout={[Function]}
initialNumToRender={5}
initialScrollIndex={1}
maxToRenderPerBatch={10}
onContentSizeChange={[Function]}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
renderItem={[Function]}
scrollEventThrottle={50}
stickyHeaderIndices={Array []}
windowSize={10}
>
<View>
<View
style={null}
>
<MockCellItem
value={0}
/>
</View>
<View
style={null}
>
<MockCellItem
value={1}
/>
</View>
<View
style={null}
>
<MockCellItem
value={2}
/>
</View>
<View
style={null}
>
<MockCellItem
value={3}
/>
</View>
</View>
</RCTScrollView>
`;
exports[`renders no spacers up to initialScrollIndex on first render when virtualization disabled 1`] = `
<RCTScrollView
data={