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 VirtualizedList saves items with offsets equal to or below zero and lengths equal to zero #35415

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 in this PR.

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

These solutions involve adding or modifying VirtualizedList.js but they improve drastically the scroll experience on Web without causing any regression 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 Third Part solution to fix the 'Flatlist with expensive items breaks scroll' issue in react-native-web.

VirtualizedList saves offsets equal to or below zero and lengths equal to zero

As explained in the Second Part PR, a High Priority update cancels every Low Priority update. A canceled Low Priority update will make onLayout returns offset and height equal to zero for the items this update pretended to mount. These zero values will be saved in _frames, associated with the canceled item, even if that item was properly measured on previous updates.

_frames object with offsets equal to zero will cause a miss calculation to get items for the virtual area. Because it does not make sense that items in higher positions than the bottom of the list have an offset equal to or below zero.

Solution:

Skip the action to save values on our _frames and trigger another update if any of the following conditions are met:

  1. zero for offset and the item measured is not the first item of the FlatList.
  2. offset below zero
  3. length equal to zero

Screen Shot 2022-11-18 at 19 19 40

Changelog

[GENERAL] [FIXED] prevent VirtualizedList saves offsets equal to or below zero and lengths equal to zero

Test Plan

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,208 -232
android hermes armeabi-v7a 6,470,283 -210
android hermes x86 7,519,702 -193
android hermes x86_64 7,378,363 -210
android jsc arm64-v8a 8,967,143 -166
android jsc armeabi-v7a 7,697,986 -166
android jsc x86 9,029,302 -116
android jsc x86_64 9,507,121 -142

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 7018ce7 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 7018ce7 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

A canceled Low Priority update will make onLayout returns offset and height equal to zero for the items this update pretended to mount.

Could you explain more here? If we never mount the items, where does this onLayout callback come from? I would think the mounting would happen in the eventual high pri update?

@LucioChavezFuentes
Copy link
Author

A canceled Low Priority update will make onLayout returns offset and height equal to zero for the items this update pretended to mount.

Could you explain more here? If we never mount the items, where does this onLayout callback come from? I would think the mounting would happen in the eventual high pri update?

Thank you @NickGerleman for the review.

That's my understanding of what happens to canceled Low Priority and I interpreted it as the reason for onLayout getting zero values for dimensions. I am open to being corrected, I am still understanding VirtualizedList. Cells of canceled Low Pri never execute onLayout?

@NickGerleman
Copy link
Contributor

A canceled Low Priority update will make onLayout returns offset and height equal to zero for the items this update pretended to mount.

Could you explain more here? If we never mount the items, where does this onLayout callback come from? I would think the mounting would happen in the eventual high pri update?

Thank you @NickGerleman for the review.

That's my understanding of what happens to canceled Low Priority and I interpreted it as the reason for onLayout getting zero values for dimensions. I am open to being corrected, I am still understanding VirtualizedList. Cells of canceled Low Pri never execute onLayout?

My understanding is that they they should not, since if the low priority update is canceled, the new CellRenderer's the update would create (and add onLayout to) are never added. They are instead first added in the high priority update then.

@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