Skip to content

Commit 73ad651

Browse files
genkikondofacebook-github-bot
authored andcommitted
Fix bounds calculation with initialScrollIndex
Summary: Sometimes this._scrollMetrics.offset is 0 even after initial scroll is triggered, because the offset is updated only upon _onScroll, which may not have been called in time for the next computation of the render limits. Thus, there is a window of time where computeWindowedRenderLimits calculates undesired render limits as it uses the offset. This results in 2 extra rerenders of the VirtualizedList if an initial scroll offset is applied, as the render limits shifts from the expected bounds (calculated using initialScrollIndex), to the 0 offset bounds (calculated using computeWindowedRenderLimits due to offset = 0), back to the expected bounds (onScroll triggers recalculation of render limits via _updateCellsToRender). This issue was introduced in https://www.internalfb.com/diff/D35503114 (facebook@c5c1798) We cannot rely on this._hasDoneInitialScroll to indicate that scrolling *actually* finished (specifically, that _onScroll was called). Instead, we want to recalculate the windowed render limits if any of the following hold: - initialScrollIndex is undefined or is 0 - initialScrollIndex > 0 AND scrolling is complete - initialScrollIndex > 0 AND the end of the list is visible (this handles the case where the list is shorter than the visible area) <- this is the case that https://www.internalfb.com/diff/D35503114 (facebook@c5c1798) attempted to address Changelog: [Internal][Fixed] - Fix issue where VirtualizedList rerenders multiple times and flickers when initialScrollIndex is set Reviewed By: JoshuaGross Differential Revision: D36328891 fbshipit-source-id: aba26aa06b24f6976657dd1e9f95bb666f60d9a6
1 parent 2e49332 commit 73ad651

File tree

2 files changed

+19
-7
lines changed

2 files changed

+19
-7
lines changed

Libraries/Lists/VirtualizedList.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,7 +1202,8 @@ class VirtualizedList extends React.PureComponent<Props, State> {
12021202
},
12031203
} = {};
12041204
_footerLength = 0;
1205-
_hasDoneInitialScroll = false;
1205+
// Used for preventing scrollToIndex from being called multiple times for initialScrollIndex
1206+
_hasTriggeredInitialScrollToIndex = false;
12061207
_hasInteracted = false;
12071208
_hasMore = false;
12081209
_hasWarned: {[string]: boolean} = {};
@@ -1539,15 +1540,15 @@ class VirtualizedList extends React.PureComponent<Props, State> {
15391540
height > 0 &&
15401541
this.props.initialScrollIndex != null &&
15411542
this.props.initialScrollIndex > 0 &&
1542-
!this._hasDoneInitialScroll
1543+
!this._hasTriggeredInitialScrollToIndex
15431544
) {
15441545
if (this.props.contentOffset == null) {
15451546
this.scrollToIndex({
15461547
animated: false,
15471548
index: this.props.initialScrollIndex,
15481549
});
15491550
}
1550-
this._hasDoneInitialScroll = true;
1551+
this._hasTriggeredInitialScrollToIndex = true;
15511552
}
15521553
if (this.props.onContentSizeChange) {
15531554
this.props.onContentSizeChange(width, height);
@@ -1758,6 +1759,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
17581759
| $TEMPORARY$object<{first: number, last: number}>
17591760
);
17601761
const {contentLength, offset, visibleLength} = this._scrollMetrics;
1762+
const distanceFromEnd = contentLength - visibleLength - offset;
17611763
if (!isVirtualizationDisabled) {
17621764
// If we run this with bogus data, we'll force-render window {first: 0, last: 0},
17631765
// and wipe out the initialNumToRender rendered elements.
@@ -1768,7 +1770,17 @@ class VirtualizedList extends React.PureComponent<Props, State> {
17681770
// we'll wipe out the initialNumToRender rendered elements starting at initialScrollIndex.
17691771
// So let's wait until we've scrolled the view to the right place. And until then,
17701772
// we will trust the initialScrollIndex suggestion.
1771-
if (!this.props.initialScrollIndex || this._hasDoneInitialScroll) {
1773+
1774+
// Thus, we want to recalculate the windowed render limits if any of the following hold:
1775+
// - initialScrollIndex is undefined or is 0
1776+
// - initialScrollIndex > 0 AND scrolling is complete
1777+
// - initialScrollIndex > 0 AND the end of the list is visible (this handles the case
1778+
// where the list is shorter than the visible area)
1779+
if (
1780+
!this.props.initialScrollIndex ||
1781+
this._scrollMetrics.offset ||
1782+
Math.abs(distanceFromEnd) < Number.EPSILON
1783+
) {
17721784
newState = computeWindowedRenderLimits(
17731785
this.props.data,
17741786
this.props.getItemCount,
@@ -1781,7 +1793,6 @@ class VirtualizedList extends React.PureComponent<Props, State> {
17811793
}
17821794
}
17831795
} else {
1784-
const distanceFromEnd = contentLength - visibleLength - offset;
17851796
const renderAhead =
17861797
distanceFromEnd < onEndReachedThreshold * visibleLength
17871798
? maxToRenderPerBatchOrDefault(this.props.maxToRenderPerBatch)

Libraries/Lists/__tests__/VirtualizedList-test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,7 @@ it('adjusts render area with non-zero initialScrollIndex', () => {
885885
viewport: {width: 10, height: 50},
886886
content: {width: 10, height: 200},
887887
});
888+
simulateScroll(component, {x: 0, y: 10}); // simulate scroll offset for initialScrollIndex
888889

889890
performAllBatches();
890891
});
@@ -914,8 +915,8 @@ it('renders new items when data is updated with non-zero initialScrollIndex', ()
914915

915916
ReactTestRenderer.act(() => {
916917
simulateLayout(component, {
917-
viewport: {width: 10, height: 50},
918-
content: {width: 10, height: 200},
918+
viewport: {width: 10, height: 20},
919+
content: {width: 10, height: 20},
919920
});
920921
performAllBatches();
921922
});

0 commit comments

Comments
 (0)