Skip to content

Commit

Permalink
Fix invariant violation when maintainVisibleContentPosition adjustm…
Browse files Browse the repository at this point in the history 
…ent moves window before list start (facebook#38655)

Summary:
Pull Request resolved: facebook#38655

facebook#35993 added logic in VirtualizedList to support `maintainVisibleContentPosition`. This logic makes sure that a previously visible cell being used as an anchor remains rendered after new content is added.

The strategy here is to calculate the difference in previous and new positions of the anchor, and move the render window to its new location during item change. `minIndexForVisible` is used as this anchor.

When an item change moves the anchor to a position below `minIndexForVisible`, shifting the render window may result in a window which starts before zero. This fixes up `_constrainToItemCount()` to handle this.

Changelog:
[General][Fixed] - Fix invariant violation when `maintainVisibleContentPosition` adjustment moves window before list start

Reviewed By: yungsters

Differential Revision: D47846165

fbshipit-source-id: 8a36f66fdad321acb255745dad85618d28c54dba
  • Loading branch information
@NickGerleman @billnbell3
NickGerleman authored and billnbell3 committed Jul 29, 2023
1 parent 8cac67f commit 721d9eb
Show file tree
Hide file tree
Showing 3 changed files with 321 additions and 3 deletions.
10 changes: 7 additions & 3 deletions packages/virtualized-lists/Lists/VirtualizedList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -833,15 +833,19 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
props: Props,
): {first: number, last: number} {
const itemCount = props.getItemCount(props.data);
const last = Math.min(itemCount - 1, cells.last);
const lastPossibleCellIndex = itemCount - 1;

// Constraining `last` may significantly shrink the window. Adjust `first`
// to expand the window if the new `last` results in a new window smaller
// than the number of cells rendered per batch.
const maxToRenderPerBatch = maxToRenderPerBatchOrDefault(
props.maxToRenderPerBatch,
);
const maxFirst = Math.max(0, lastPossibleCellIndex - maxToRenderPerBatch);

return {
first: clamp(0, itemCount - 1 - maxToRenderPerBatch, cells.first),
last,
first: clamp(0, cells.first, maxFirst),
last: Math.min(lastPossibleCellIndex, cells.last),
};
}

Expand Down
47 changes: 47 additions & 0 deletions packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2224,6 +2224,53 @@ it('handles maintainVisibleContentPosition', () => {
expect(component).toMatchSnapshot();
});

it('handles maintainVisibleContentPosition when anchor moves before minIndexForVisible', () => {
const items = generateItems(20);
const ITEM_HEIGHT = 10;

// Render a list with `minIndexForVisible: 1`
let component;
ReactTestRenderer.act(() => {
component = ReactTestRenderer.create(
<VirtualizedList
initialNumToRender={1}
windowSize={1}
maintainVisibleContentPosition={{minIndexForVisible: 1}}
{...baseItemProps(items)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});

ReactTestRenderer.act(() => {
simulateLayout(component, {
viewport: {width: 10, height: 50},
content: {width: 10, height: items.length * ITEM_HEIGHT},
});

performAllBatches();
});

expect(component).toMatchSnapshot();

// Remove the first item to shift the previous anchor to be before
// `minIndexForVisible`.
const [, ...restItems] = items;
ReactTestRenderer.act(() => {
component.update(
<VirtualizedList
initialNumToRender={1}
windowSize={1}
maintainVisibleContentPosition={{minIndexForVisible: 1}}
{...baseItemProps(restItems)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
});

expect(component).toMatchSnapshot();
});

function generateItems(count, startKey = 0) {
return Array(count)
.fill()
Expand Down
267 changes: 267 additions & 0 deletions packages/virtualized-lists/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3659,6 +3659,273 @@ exports[`handles maintainVisibleContentPosition 3`] = `
</RCTScrollView>
`;

exports[`handles maintainVisibleContentPosition when anchor moves before minIndexForVisible 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={1}
maintainVisibleContentPosition={
Object {
"minIndexForVisible": 1,
}
}
onContentSizeChange={[Function]}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
renderItem={[Function]}
scrollEventThrottle={50}
stickyHeaderIndices={Array []}
windowSize={1}
>
<View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={0}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={1}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={2}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={3}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={4}
/>
</View>
<View
style={
Object {
"height": 150,
}
}
/>
</View>
</RCTScrollView>
`;

exports[`handles maintainVisibleContentPosition when anchor moves before minIndexForVisible 2`] = `
<RCTScrollView
data={
Array [
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={1}
maintainVisibleContentPosition={
Object {
"minIndexForVisible": 1,
}
}
onContentSizeChange={[Function]}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
renderItem={[Function]}
scrollEventThrottle={50}
stickyHeaderIndices={Array []}
windowSize={1}
>
<View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={1}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={2}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={3}
/>
</View>
<View
onFocusCapture={[Function]}
style={null}
>
<MockCellItem
value={4}
/>
</View>
<View
style={
Object {
"height": 150,
}
}
/>
</View>
</RCTScrollView>
`;

exports[`initially renders nothing when initialNumToRender is 0 1`] = `
<RCTScrollView
data={
Expand Down

0 comments on commit 721d9eb

Please sign in to comment.