Fix invariant violation when using viewability callbacks with horizontal RTL FlatList on Paper #39335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D49072963 |
…tal RTL FlatList on Paper (facebook#39335) Summary: In RTL we must have scrollview content length in order to resolve cell metrics. This means that on Paper, where layout events are bottom up, we cannot immediately calculate viewability in response to cell metric changes, as we may not yet have an accurate length of laid out list content. This change makes us defer calculation of viewability changes in this case via `setTimeout()`, to run in a single batch after the next layout events are fired. --- We need container dimensions to resolve the right-edge relative child directions. It is tricky to do this at a guaranteed right time with onLayout model on Paper. When we are laying out children, the first layout on Paper looks like: 1. Child is laid out 2. Container is laid out However, we will never see container onLayout if a child layout does not change the dimensions of the parent container. This will be the common case of subsequent child layouts, where the spacer size was accurate. I.e. we may or may not ever see content laid out, but can only rely on having both offsets be up to date if we trigger calculation after the container layout would have happened. This is not an issue on Fabric where layout events are fired top-down, or for the most common cases of VirtualizedList, where we run calculations in idle batches that will happen after layout is set, but ends up causing problems for two scenarios I didn't originally account for: We may recalculate cells to render immediately instead scheduling a later update if the list thinks blanking is immediate (high priority render). This means we cannot do an immediate update in response to cell layout, but we can in response to events batched after layout events are all dispatched, or in worst case delay in Paper RTL. We do not batch/schedule viewability calculations in response to cell layout in the same wasy as we do for calculating cells to render, but do need them to trigger recalculation. The way less hacky, but much more invasive solution, that would simplify a lot of this, would be to include parentWidth and parentHeight in onLayout events for Paper (and Fabric for consistency), so that we don't need to rely on event ordering, or sometimes not firing. I thought this would be too much at first, if we didn't have other use-cases, but am more and more tempted to tear down a lot of what we have here to do that instead, since this is not going to be able to rely on useLayoutEffect or IntersectionObserver in today's VirtualizedList because it will need to support Paper for the forseeable future.. Changelog: [General][Fixed] - Fix invariant violation when using viewability callbacks with horizontal RTL FlatList on Paper Reviewed By: yungsters Differential Revision: D49072963
|
This pull request was exported from Phabricator. Differential Revision: D49072963 |
NickGerleman
force-pushed
the
export-D49072963
branch
from
ae6c6b1 to
e27d358
Compare
|
This pull request has been merged in a2fb46e. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
In RTL we must have scrollview content length in order to resolve cell metrics. This means that on Paper, where layout events are bottom up, we cannot immediately calculate viewability in response to cell metric changes, as we may not yet have an accurate length of laid out list content.
This change makes us defer calculation of viewability changes in this case via
setTimeout(), to run in a single batch after the next layout events are fired.Changelog:
[General][Fixed] - Fix invariant violation when using viewability callbacks with horizontal RTL FlatList on Paper
Differential Revision: D49072963