react-native
react-native copied to clipboard
facebook
Nested horizontal virtualized list viewability
Summary
This Pull Request fixes issues with viewability events in the Virtualized List component when nesting in opposing directions.
Essentially, the VirtualizedList is currently only considering same-direction viewability. To fix that and stop the viewability computations from running on nested Flatlists, I needed a way to allow nested Flatlists to access the parent's currently visible elements. By storing the list's currently viewable items in-memory, I was able to reach into the parent's context and pass that data to the ViewabilityHelper of all of its children.
See #35180
Changelog
[General] [Fixed] - Nested horizontal virtualized list viewability events
Test Plan
Working on adding some jest tests but there's some interesting stuff happening with react-test-renderer in the virtualized list related to viewability (likely need to trigger some onLayout events). Doesn't seem easy to test at this point.
https://user-images.githubusercontent.com/22749569/200435788-31abe231-3fc5-498d-979e-aa4f3f12798b.mp4
Hi @vincentvella!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 7,066,562 | +385 |
| android | hermes | armeabi-v7a | 6,438,939 | +378 |
| android | hermes | x86 | 7,481,872 | +377 |
| android | hermes | x86_64 | 7,341,278 | +389 |
| android | jsc | arm64-v8a | 8,932,216 | +221 |
| android | jsc | armeabi-v7a | 7,666,557 | +218 |
| android | jsc | x86 | 8,992,587 | +221 |
| android | jsc | x86_64 | 9,471,338 | +217 |
Base commit: 7a327d967357ecf04ddd64b6f6661dd1e7f0ea22 Branch: main
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| ios | - | universal | n/a | -- |
Base commit: 7a327d967357ecf04ddd64b6f6661dd1e7f0ea22 Branch: main
PR build artifact for e5a9d0adebefe7584964ef3916a3c5d56930a081 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.
since the inner list can still be arbitrarily offset within the cell to not be visible itself.
@NickGerleman is there any chance you have an example of what you're describing above? It'll be useful to help diagnose the issues with this PR. For my particular use-case this solves the issue I'm attempting to get past so I might use this as a temporary patch until your version/strategy is merged.
My thought process was if an inner cell was no longer visible or became not visible at a time post-render it'd fire one of the layout events and the list would recalculate its positioning but I'm interested to see if there's an edge case I'd missed
In the below example, cell 2 of the outer vertical list is visible, but none of the children in the nested list are yet visible in the y axis. So deriving visibility from whether the list is in a visible cell doesn't work quite generically.
A solution you can use to do that accurately is to instead thread both main-axis and cross-axis position down the child contexts. That allows individual lists to know their viewport coverage in both axis. It means requirements like "must be 20% visible" can also apply to the cross-axis as well.
Last week has been hectic, but I can export the change as a GitHub PR and retest it soon. Part of why I was holding off is that this change will need more UTs than what I had added, or the nested RNTester example you discovered.
This makes total sense @NickGerleman , I'm happy to help with any unit testing if you are able to export the PR to Github! I attempted to do something similar but I was attempting to reach up from the child to get the scroll positioning. I think it may have only been grabbing the outermost parent because it wasn't firing the onViewableItemsChanged appropriately anymore.