Skip to content

Commit

Permalink
Fix last spacer constrain logic in VirtualizedList (#41846)
Browse files Browse the repository at this point in the history
Summary:
The logic to constrain the last spacer size is incorrect in some cases where the spacer is the last spacer, but not the last section in the list.

For more context, the role of spacer constraining is explained in this comment:

```
// Without getItemLayout, we limit our tail spacer to the _highestMeasuredFrameIndex to
// prevent the user for hyperscrolling into un-measured area because otherwise content will
// likely jump around as it renders in above the viewport.
 ```

For example it is incorrect in the case where we have:

ITEMS
SPACER
ITEMS

In this case the spacer is not actually the tail spacer so the constraining is incorrectly appied.

This causes issues mainly when using `maintainVisibleContentPosition` since it will cause it to scroll to an incorrect position and then cause the view that was supposed to stay visible to be virtualized away.

## Changelog:

[GENERAL] [FIXED] - Fix last spacer constrain logic in VirtualizedList

Pull Request resolved: #41846

Test Plan:
Tested using https://gist.github.com/janicduplessis/b67d1fafc08ef848378263208ab93d4c in RN tester, before the change content will jump on first click on add items.

Tested using the same example and setting initial posts to 1000, then we can see our content view size is still constrained properly (see scrolling indicator as reference).

Reviewed By: yungsters

Differential Revision: D51964500

Pulled By: NickGerleman

fbshipit-source-id: 4465aa5a36c95466aef6571314973c1e2c9a0f2c
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Dec 12, 2023
1 parent b6adbf7 commit 3ed4bf9
Showing 1 changed file with 2 additions and 14 deletions.
16 changes: 2 additions & 14 deletions packages/virtualized-lists/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,6 @@ type State = {
pendingScrollUpdateCount: number,
};

function findLastWhere<T>(
arr: $ReadOnlyArray<T>,
predicate: (element: T) => boolean,
): T | null {
for (let i = arr.length - 1; i >= 0; i--) {
if (predicate(arr[i])) {
return arr[i];
}
}

return null;
}

function getScrollingThreshold(threshold: number, visibleLength: number) {
return (threshold * visibleLength) / 2;
}
Expand Down Expand Up @@ -987,7 +974,8 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
const spacerKey = this._getSpacerKey(!horizontal);

const renderRegions = this.state.renderMask.enumerateRegions();
const lastSpacer = findLastWhere(renderRegions, r => r.isSpacer);
const lastRegion = renderRegions[renderRegions.length - 1];
const lastSpacer = lastRegion?.isSpacer ? lastRegion : null;

for (const section of renderRegions) {
if (section.isSpacer) {
Expand Down

0 comments on commit 3ed4bf9

Please sign in to comment.