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.

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 skips items for offset measuring when the user scrolls very fast (3rd of 4 problems that cause #1254) #2414

Conversation

LucioChavezFuentes
Copy link
Contributor

@LucioChavezFuentes LucioChavezFuentes commented Oct 23, 2022

I found 4 problems that cause the Inverted VirtualizedList to go wild (#1254). This PR will explain and fix the 3rd of 4 problems found.

If we fix the following 4 problems, issue #1254 will be fixed and FlatList’s scroll will be more stable:

  1. Inverted VirtualizedList has incorrect baseline measurement for its items’ offset —> Problem Explanation and Solution in this PR.

Update:

PR 2, 3, and 4 will fix 'Flatlist with expensive items breaks scroll' issue. PR 1 is enough to fix the Inverted Flatlist issue if your Flatlist's items are cheap to mount.


  1. $lead_spacer expands scroll artificially when Inverted VirtualizedList is mounting new items —> Problem Explanation and Solution in this PR.

  2. VirtualizedList skip items for offset measuring when the user scrolls very fast while new items are mounted and measured (this happens also for normal lists) —> Problem Explanation and Solution below.

  3. VirtualizedList gets offsets equal to zero for items that are not the list's first item (this happens also for normal lists) —> Problem Explanation and Solution in this PR.

Video of Inverted FlatList with all problems fixed:

2022-10-23.17-49-22.mp4

3rd Problem

VirtualizedList skip items for offset measuring when the user scrolls very fast while new items are mounted and measured (this happens also for normal lists, 3rd of 4 problems that cause #1254)

There are two important variables (react states of VirtualizedList) that determine the virtual area: first and last. These variables represent a range of values from the data array (which is raw information of items provided to FlatList as data prop) that we use to render items on the virtual area.

On every scroll, first and last get updated according to two priority updates: Low Priority and High Priority.

Low Priority updates help us to wait for new items to be mounted and measured. These updates are scheduled in a queue and are useful when the user is scrolling on unmeasured area.

High Priority updates help us to mount items as fast as possible. These updates occur when users scroll fast: if a High Priority update is triggered, every Low Priority update on the queue is canceled.

When the user scrolls up to unmeasured area, ideally (on every update) the prev last should be greater than the next first. This ensures that every item is measured and saved in _frames, like this:

Screen Shot 2022-10-12 at 18 06 56

But If we scroll very fast towards unmeasured area and the list is still rendering new items, a race condition happens between measuring new items in onLayout and the High Priority update. This is because onLayout (executed on unmeasured items) takes some time to return the offset value that we need after the item is mounted. If High Priority wins, the next first will be greater than prev last, and as a consequence, some items will be skipped for measuring:

Screen Shot 2022-10-12 at 17 25 01

More than one item can be skipped, which makes the problem worse. The more items are skipped to be measured, the greater the probability to experiment scroll issues (like weird jumps or white space in the visible area).

Solution:

packages/react-native-web/src/vendor/react-native/VirtualizedList/index.js
image13

1.-Description of the first line of code:

image1

We use a variable called totalCellLength, it represents the sum of heights of all items measured.
If totalCellLength is greater than scroll offset + visibleLength / 2 (half of our Visible Area) it means the measure of new items is keeping up with the scroll toward unmeasured area or that we are in an area where the cells had been already measured so we allow High Priority updates as usual:

Screen Shot 2022-10-12 at 18 13 08

If totalCellLength is not greater than scroll offset + visibleLength / 2 then VirtulizedList prevents High Priority updates, allowing Low Priority updates on queue to complete so measure of items can catch up with scroll and prevents subsequent updates where next first is greater than prev last:

Screen Shot 2022-10-12 at 18 14 41

Why visibleLength / 2?

It represents a balanced ‘bar check’ to be sure that all items are gonna be measured and mounted as fast as possible (when user scrolls quickly towards unmeasured area). I tested with other two approaches:

  • totalCellLength > offset + visibleLength
  • totalCellLength > offset

With offset + visibleLength (top of Visible Area) we set the ‘bar check’ too high: the check will be false as soon as a new item is mounted. This denies any margin to make High Priority Updates when users keep scrolling up to unmeasured area.

With totalCellLength > offset (bottom of Visible Area), the check will be true only if one new item is measured in our visible area. This will trigger more High Priority updates than it should, Low Priority updates will be canceled more often, and we lose the certainty that no items are gonna skip measure.

I tested previous scenarios and I conclude scroll offset + visibleLength / 2 is the fastest way to scroll safely toward unmeasured area.

2.-Description of the second line of code:

image11

Prevents the checking of the first line of code (allows High Priority updates as usual) if the FlatList doesn't have enough rendered items to fill both the Visible Area and the initialNumToRender items (the number of items at the beginning of FlatList that never unmount). This preserves a fast list initialization.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 56b17af:

Sandbox Source
react-native-web-examples Configuration

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change also belongs in the upstream React Native repo, not react-native-web, and remains valid after the refactor.

Unless I'm mistaken, these same changes can be applied in the place, ie: right here in _scheduleCellsToRenderUpdate

) ? (
isScrollOffsetMeasured
) : (
this._averageCellLength
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should coerce to Boolean:

Suggested change
this._averageCellLength
Boolean(this._averageCellLength)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flatlist with expensive items breaks scroll
2 participants