Prevent VirtualizedList to save offsets equal to zero if item is not the first item of list (4th of 4 problems that cause #1254) #2415
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
…the first item of list
|
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 10fcaa5:
|
roryabraham
suggested changes
Nov 8, 2022
There was a problem hiding this comment.
I believe this change also belongs in the upstream react-native, not react-native-web. You can apply the same change in the same place, right here in _onCellLayout
| @@ -1323,6 +1323,14 @@ class VirtualizedList extends React.PureComponent<Props, State> { | |||
| inLayout: true, | |||
| }; | |||
| const curr = this._frames[cellKey]; | |||
|
|
|||
| // Measures on discarded Low Priority updates will return zero for dimensions | |||
| // no cell should have an offset of zero on frames, except for first cell | |||
There was a problem hiding this comment.
Suggested change
| // no cell should have an offset of zero on frames, except for first cell | |
| // no cell should have an offset of zero on frames, except for first cell, | |
| // so if that's the case, re-schedule another update without saving the incorrect layout |
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.
I found 4 problems that cause the Inverted VirtualizedList to go wild (#1254). This PR will explain and fix the 4th of 4 problems found.
If we fix the following 4 problems, issue #1254 will be fixed and FlatList’s scroll will be more stable:
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.
$lead_spacer expands scroll artificially when Inverted VirtualizedList is mounting new items —> Problem Explanation and Solution in this PR.
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 in this PR.
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 below.
Video of Inverted FlatList with all problems fixed:
2022-10-23.17-49-22.mp4
4th Problem
VirtualizedList gets offsets equal to zero for items that are not the list's first item (this happens also for normal lists, 4th of 4 problems that cause #1254)
As explained in the 3rd PR, a High Priority update cancels every Low Priority update. A canceled Low Priority update will make
onLayoutreturns 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._framesobject 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 zero.Solution:
Skip the action to save values on our
_framesand trigger another update if two conditions happen: 1)onLayoutreturns zero for offset and 2) the item measured is not the first item of the FlatList.react-native-web/src/vendor/react-native/VirtualizedList/index.js → Inside:
_onCellLayoutfunction.