Repository navigation
fix(scroll): let scrollToIndex reach content after the last item - #2529
Open
cleobnvntra wants to merge 1 commit into
Open
cleobnvntra wants to merge 1 commit into
cleobnvntra wants to merge 1 commit into
Conversation
scrollToIndex clamped its target offset to a maximum computed from the header and the items only, ignoring ListFooterComponent and contentContainerStyle padding. Near the end of the list, viewPosition was ignored even though the scroll view had room to scroll further. Clamp against the content size reported by the scroll view instead, falling back to the layout size until it is reported (or when the reported size is stale and smaller). onContentSizeChange is still forwarded to the user's handler. Fixes Shopify#1923
Author
|
I have signed the CLA! |
This branch has not been deployed
This file contains hidden or 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
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.
Description
Fixes #1923
scrollToIndexclamps its final offset withgetMaxScrollOffset(), which computes the scrollable length from the header and the items only. Content after the items —ListFooterComponentandcontentContainerStylepadding — is not counted, so near the end of the listviewPositionis ignored even though the scroll view can scroll further (you can scroll there manually). This is also why addingpaddingBottomdoesn't help, while adding a spacer item todatadoes.This PR clamps against the content size reported by the scroll view (
onContentSizeChange), which includes everything after the items:RecyclerViewManagerstores the reported content size andgetMaxScrollOffset()usesmax(layout size, reported size). Before a size is reported, or if the reported size is stale and smaller (e.g. right after items are appended), it behaves exactly as before.RecyclerViewpasses its ownonContentSizeChangeto the scroll view and forwards the call to the user's handler.Lists without content after the items are unaffected: the reported size equals the layout size.
getMaxScrollOffset()is only used byscrollToIndex, so the change is limited toscrollToIndex,scrollToItemand thescrollToIndexstep ofscrollToEnd.Reviewers' hat-rack 🎩
getMaxScrollOffset()inRecyclerViewManager.ts: taking the larger of the layout size and the reported size. If data shrinks, the reported size can be larger than the real content until the nextonContentSizeChange, so in that window the target can be past the real end. Happy to change this if you'd prefer a different trade-off.onContentSizeChangeinRecyclerView.tsxis set after{...rest}and before{...overrideProps}, so a handler passed throughoverridePropsstill takes precedence (and the clamp then falls back to the previous behaviour).To reproduce: a list with a
ListFooterComponentorcontentContainerStyle={{ paddingBottom }}, thenscrollToIndex({ index: <one of the last items>, viewPosition: 0.5 }). Before: stops short of centering. After: the item is centered. Without footer/padding, the behaviour is unchanged (it stops at the end of the list).Screenshots or videos
Footer (300px) +
paddingBottom(200px),scrollToIndex({ index, viewPosition: 0.5 }). Red line = viewport center.Test plan
RecyclerView.test.tsx(vertical, horizontal, fallback before a size is reported, stale/smaller size, invalid sizes, user handler forwarding). The two main cases fail onmain(Expected: 1500.5, Received: 1101) and pass with the fix.yarn test): 206/206yarn type-check)yarn lint)fixture/web) with a footer +paddingBottom: last items now center; without them, behaviour unchanged