Improve heuristic that triggers onStartReached #44287
Closed
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
CLA Signed
6 tasks
facebook-github-bot
added
the
Shared with Meta
54 tasks
52 tasks
NickGerleman
reviewed
May 6, 2024
| @@ -1552,16 +1552,16 @@ class VirtualizedList extends StateSafePureComponent<Props, State> { | |||
| onStartReached != null && | |||
| this.state.cellsAroundViewport.first === 0 && | |||
| isWithinStartThreshold && | |||
| this._listMetrics.getContentLength() !== this._sentStartForContentLength | |||
| this.state.firstVisibleItemKey !== this._sentStartForFirstVisibleItemKey | |||
There was a problem hiding this comment.
Comment above is now out of date it looks like
There was a problem hiding this comment.
Fixed, and only if onEndReached is not being executed that part was also out of date from a previous change.
|
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
react-native-bot
added
the
Stale
|
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
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:
onStartReachedis called more than it should be since our heuristic to not re-trigger it is based on the list content size. This has worked fine foronEndReached, but foronStartReachedit causes some issues.On initial mount we receive different content size updates. I assume this is normal because of virtualization and the need to measure content. However for
onStartReachedsince we usually start at scroll position 0 we are already in the threshold so any change to content size causes us to trigger aonStartReachedevent.For example using this code we get 2
onStartReachedcalls on mount. On top of this every time we add items at the end of the listonStartReachedis called another time.To improve this I suggest using
firstVisibleItemKeythat we use formaintainVisibleContentPositionto track if we should re-triggeronStartReached. This means that it will be only re-triggered if new items are added at the start of the list or if we leave the threshold.Changelog:
[GENERAL] [FIXED] - Improve heuristic that triggers onStartReached
Test Plan:
Tested using this code. See that
onStartReachedis only called once on mount and then again only if leaving the threshold or adding new items at the start of the list. Adding items at the end of the list should also not triggeronStartReached.