Fix last spacer constrain logic in VirtualizedList

[janicduplessis]

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

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).

[janicduplessis]

cc @NickGerleman

[NickGerleman]

I think this makes sense.

Wonder if there other ways than MVCP adjustment we get into this state of "holes" in the measurements. I think that works fine most of the time under MVCP though (since the logic is supposed to avoid viewport shift on estimated layout change before viewport).

[facebook-github-bot]

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[janicduplessis]

Yea I don’t think this logic is perfect since it is possible to shift outside the window if added items are very different than the estimate still. I wonder if there is a better way to handle this.

[NickGerleman]

Oh, one more MVCP issue on my radar was around Horizontal + RTL. I did a round of fixups to the platform scrollviews and FlatList for RTL, but MVCP wasn't working, and I didn't do the right thing to fix it. 30c7e9d

[janicduplessis]

I am currently working with Expensify to use MVCP in their app so we might end up hitting these issues too.

[facebook-github-bot]

@NickGerleman merged this pull request in 3ed4bf9.