Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2265f04
:white_check_mark: (VirtualizedList) add broken test case based on is…
Apr 3, 2022
2335afb
:bug: (VirtualizedList) fix VirtualizedList with initialScrollIndex n…
Apr 3, 2022
5eb2a0a
:fire: (VirtualizedList-test) remove unrelevant test case
Apr 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Libraries/Lists/VirtualizedList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 25 additions & 10 deletions Libraries/Lists/__tests__/VirtualizedList-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +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', () => {
it('adjusts render area with non-zero initialScrollIndex', () => {
const items = generateItems(20);
const ITEM_HEIGHT = 10;

Expand All @@ -885,18 +885,17 @@ it('does not adjust render area with non-zero initialScrollIndex until scrolled'
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.
// We should expand the render area after receiving a message indcating we
// arrived at initialScrollIndex.
expect(component).toMatchSnapshot();
});

it('adjusts render area with non-zero initialScrollIndex after scrolled', () => {
const items = generateItems(20);
it('renders new items when data is updated with non-zero initialScrollIndex', () => {
const items = generateItems(2);
const ITEM_HEIGHT = 10;

let component;
Expand All @@ -918,13 +917,29 @@ it('adjusts render area with non-zero initialScrollIndex after scrolled', () =>
viewport: {width: 10, height: 50},
content: {width: 10, height: 200},
});
performAllBatches();
});

const newItems = generateItems(4);

simulateScroll(component, {x: 0, y: 10});
ReactTestRenderer.act(() => {
component.update(
<VirtualizedList
initialNumToRender={5}
initialScrollIndex={1}
windowSize={10}
maxToRenderPerBatch={10}
{...baseItemProps(newItems)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});

ReactTestRenderer.act(() => {
performAllBatches();
});

// We should expand the render area after receiving a message indcating we
// arrived at initialScrollIndex.
// We expect all the items to be rendered
expect(component).toMatchSnapshot();
});

Expand Down
209 changes: 70 additions & 139 deletions Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand Down Expand Up @@ -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={
Expand Down Expand Up @@ -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={
Expand Down