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

Prevent the tail-spacer gets negative values in VirtualizedList on scrolling toward new items with dynamic height. #35413

Conversation

LucioChavezFuentes
Copy link

@LucioChavezFuentes LucioChavezFuentes commented Nov 20, 2022

Summary

Flatlist on Web has scroll issues when used with expensive items.

I noticed this issue in an App called Expensify. It uses Flatlist for a chat of reports that supports text, images, emojis, and reports as items (and perhaps others that I am not aware of). These items have complex code to support interactions and features on them. As you scroll in the report chat in Web, especially if you scroll fast, you will notice scroll issues like scroll jumps, items appearing and disappearing, or items not showing at all.

Here's Expensify Flatlist Web current state:

Flatlist.Web.Broken.mp4

I recreate a sandbox with expensive items where you can experience the scroll issues I mentioned here.

Steps to reproduce the scroll issues in the sandbox:

  1. Scroll down for a while until you render a few items beyond the virtual area.
  2. Use the mouse and drag the scroll bar up and down.
  3. Things should start looking weird: scroll jumps, items appearing, and disappearing.

I take on the task to improve the scroll experience of react-native-web's Flatlist. I found 3 problems that cause this issue and present their corresponding solution:

  1. $lead_spacer expands scroll artificially when VirtualizedList is mounting new items —> Problem Explanation and Solution below.

  2. VirtualizedList skip items for offset measuring when the user scrolls very fast while new items are mounted and measured —> Problem Explanation and Solution in this PR.

  3. VirtualizedList gets offsets below or equal to zero for items that are not the list's first item —> Problem Explanation and Solution in this PR.

These solutions involve adding or modifying VirtualizedList.js but they improve drastically the scroll experience on Web without causing any problems on Android or iOS.

Also here's Expensify's App after solutions (plus another solution for Inverted VirtualizedLists in react-native-web):

Flatlist.Web.Good.mp4

This PR is the First Part solution to fix the 'Flatlist with expensive items breaks scroll' issue in react-native-web.

$lead_spacer expands scroll limit artificially when Inverted VirtualizedList is mounting new items. (1st of 3 problems that cause 'Flatlist with expensive items breaks scroll' )

In a FlatList with items with different heights, the scroll is limited to the last measured item. That’s because VirtualizedList (the inner FlatList's component) must measure the offset of mounted items before it tries to mount new items. In this way, all items in the virtual area are measured and the _frames object is complete.

The scroll limit should be only expanded when the user tries to scroll beyond the scroll limit (unmeasured area) and then new items are mounted and their offsets are measured and saved in _frames. When recently mounted items are measured, the scroll limit is expanded to the new latest measured item, then the user will be ready to keep scrolling to unmeasured area and repeat the process to keep expanding the scroll limit until the last item is met.

Sometimes users may “overscroll” to unmeasured areas while previously mounted items haven't been measured yet, and VirtualizedList starts skipping items for measuring and measures others that shouldn’t be measured yet, leading to a fragmented map of offsets (e.g. we have offset values for …31, 32, 33, then jumps to 37, 38…).

Why do users “overscroll”?

Flaltlist constantly mounts and unmounts items. A white space is mounted on our list called $tail_spacer. It helps us to fill with white space the bottom outside of our virtual area (for the inverted list, it fills the top), where items were previously mounted and measured but unmounted afterward, so the scroll limit sticks to our latest measured item offset.

Screen Shot 2022-10-12 at 18 02 58

$tail_spacer height depends on how many areas we had measured, but it reduces to zero when the latest measured item is mounted and the user pretends to scroll to an unmeasured area looking for more items. When $tail_spacer's height is zero, the scroll is limited and prevents the user from “overscroll”. This gives time to VirtualizedList to properly mount and measure new items to then use their offsets as our new scroll limit.

Sometimes $tail_spacer has a height greater than zero when it should be zero, allowing the user to “overscroll”.

Why sometimes does $tail_spacer has a height greater than zero when it should be zero?

On some updates, $tail_spacer gets a negative value for its height, which is invalid on Web. Invalid values will be ignored and $tail-spacer’s height will set the valid value of the previous state, instead of setting it to zero.

Solution:

The solution is simple. Set spacerSize to zero if the metrics calculation returns negative values.

Screen Shot 2022-11-18 at 14 34 26

Changelog

[GENERAL] [FIXED] prevent the tail-spacer gets negative values in VirtualizedList on scrolling toward new items with dynamic height.

Test Plan

This issue is difficult to reproduce on its own but fixing it does provide more stability used along with the other two solutions that I present with it. You can test the Flatlist with all solutions applied in this sandbox

Naturally, expensive items will take time to show but you should find no issues on scroll fast.

Also tested in Expensify's App I am working on (see video above).

No problem with iOS

iOS.Flatlist.Compressed.mp4

No problem with Android

Android.Flatlist.mp4

Thank you for reading, let me know what you think!

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,102,206 -234
android hermes armeabi-v7a 6,470,279 -214
android hermes x86 7,519,708 -187
android hermes x86_64 7,378,366 -207
android jsc arm64-v8a 8,967,124 -185
android jsc armeabi-v7a 7,697,970 -182
android jsc x86 9,029,287 -131
android jsc x86_64 9,507,101 -162

Base commit: 81e441a
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 81e441a
Branch: main

@pull-bot
Copy link

PR build artifact for def6211 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for def6211 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@NickGerleman
Copy link
Contributor

On some updates, $tail_spacer gets a negative value for its height, which is invalid on Web. Invalid values will be ignored and $tail-spacer’s height will set the valid value of the previous state, instead of setting it to zero.

This feels like the root issue. What are the metrics when this happens? Would be better to add an invariant that the math produces the correct results instead of adjusting it to be non-negative after the fact.

@LucioChavezFuentes
Copy link
Author

On some updates, $tail_spacer gets a negative value for its height, which is invalid on Web. Invalid values will be ignored and $tail-spacer’s height will set the valid value of the previous state, instead of setting it to zero.

This feels like the root issue. What are the metrics when this happens? Would be better to add an invariant that the math produces the correct results instead of adjusting it to be non-negative after the fact.

Thank you for reviewing. I will investigate further.

@LucioChavezFuentes
Copy link
Author

Just another question, in Native Platforms (Android, iOS), how does an element behave when it receives negative values for width or height?

@react-native-bot react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No CLA Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants