fix: seed animatedScrollY when the sticky scroll handler attaches late - #522
Open
giaBaoJS wants to merge 1 commit into
Open
fix: seed animatedScrollY when the sticky scroll handler attaches late#522giaBaoJS wants to merge 1 commit into
giaBaoJS wants to merge 1 commit into
Conversation
`useStickyScrollHandler` only attaches its `Animated.event` while `stickyHeaderIndices` is non-empty, and that event is the only writer of `ctx.animatedScrollY`. When a list mounts with empty data and the indices arrive later, every scroll event before the attach reaches only the plain JS handler and is dropped, so `animatedScrollY` still holds its initial `0`. `PositionViewSticky` derives `translateY` purely from an interpolation of that value, with `stickyStart = position - stickyHeaderConfig.offset`. On iOS with `contentInsetAdjustmentBehavior="automatic"` the true rest offset is `-headerInset`, reported in a scroll event at mount, so a stuck `0` places the index-0 sticky header exactly one inset too low until the user scrolls. Seed `animatedScrollY` from `state.lastNativeScroll` on the transition into the animated engine, which is the raw `contentOffset` the `Animated.event` would have written. Seeding only on that transition keeps `Animated.event` the sole owner afterwards, so a later `stickyHeaderIndices` change cannot overwrite a live value with a JS offset that lags during a scroll. RTL horizontal lists are skipped because `lastNativeScroll` is a logical offset there, not the raw one. Fixes LegendApp#512
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.
Fixes #512
Problem
A list mounts with empty data under
contentInsetAdjustmentBehavior="automatic". WhenstickyHeaderIndicesarrives later (data from an async query), the index-0 sticky header paints exactly one header-inset too low, overlapping the row below it, until any scroll corrects it.Root cause
ctx.animatedScrollYis created ascreateAnimatedValue(0)(src/state/state.tsx:170→src/platform/Animated.native.tsx:5), and theAnimated.eventbuilt inuseStickyScrollHandleris its only writer.That handler is gated on
stickyHeaderIndices?.length(src/platform/useStickyScrollHandler.native.ts:16), so while the indices are empty the scroll events go to the plain JS handler and nothing writes the animated value. iOS reports the inset-adjustment rest offset (-headerInset) in a scroll event at mount — exactly in that window — so it is dropped.PositionViewStickyderivestranslateYpurely from an interpolation ofanimatedScrollYwithstickyStart = position + headerSize + stylePaddingTop + alignItemsAtEndPadding - stickyHeaderConfig.offset(src/components/PositionView.native.tsx:107,122-127). For item 0 withoffset = 96that givesstickyStart = -96, and the interpolation is linear with slope 1 from(stickyStart → position):0→translateY = 0 + (0 − (−96)) = 96— one inset too low−96→translateY = 0— correctFix
Seed
animatedScrollYfromstate.lastNativeScrollon the transition into the animated engine.state.lastNativeScrollis assigned from the rawcontentOffsetinsrc/core/onScroll.ts:89, i.e. exactly the value theAnimated.eventwould have written had it been attached.Two deliberate constraints:
stickyHeaderIndices?.join(","), so a sectioned list whose indices change during a scroll would otherwise re-seed fromlastNativeScroll, which can lag the native offset (JSonScrollis throttled and has several early returns, e.g.onScroll.ts:57,85). KeepingAnimated.eventthe sole owner once attached avoids introducing that jitter. Covered by the second new test.lastNativeScrollhas been converted to a logical offset for those (onScroll.ts:77-80→toLogicalHorizontalOffset), so it is not the raw valueAnimated.eventwrites. Better to leave those on the existing behaviour than to seed a wrong number.The seed cannot double-apply:
setValueis an assignment, and every subsequentAnimated.eventwrite is also an assignment of the authoritative native offset.Why not "attach whenever
stickyHeaderIndicesis defined"The issue suggests that as an alternative. It does not fix the reported case: the repro is
undefined → [0], so the handler is still absent for the whole window in which iOS reports the inset adjustment. It would only help a[] → [0]transition. Seeding covers both.Scope
src/integrations/reanimated.tsxis untouched. It uses a separatestickyScrollOffsetshared value fed byuseScrollViewOffset(reanimated.tsx:90), which reads the live offset off the scroll view ref rather than accumulating from events, so it does not have the stale-zero problem.src/platform/useStickyScrollHandler.ts(web) is untouched — it is a no-op passthrough.as unknown as Animated.Valuecast (needed becausetsc:srcresolves@/platform/Animatedto the web shim whereAnimatedValue = number) from theAnimated.eventconfig up to the destructure, so both uses share it.Verification
bun test— 1585 pass / 0 fail (baseline onmainwas 1583; +2 new).src/platform/useStickyScrollHandler.native.tsreverted tomain, both new tests fail withExpected number of calls: 1 / Received number of calls: 0— the hook never callssetValueat all today. Restoring the fix returns 4/4 pass in that file.bun run lint— clean (Biome, 450 files).bun run tsc:src— clean.bun run build— succeeds.Tests added to
__tests__/platform/useStickyScrollHandler.native.test.tsx:stickyHeaderIndices={undefined}, setstate.lastNativeScroll = -96, re-render with[0]→animatedScrollY.setValue(-96)called exactly once.[0], then change to[0, 5]with a differentlastNativeScroll→ still only the one initial seed, no re-seed.The interpolation numbers above are arithmetic on
PositionView.native.tsx:122-127, not a rendered measurement —react-nativeis fully mocked in this suite (__tests__/setup.ts:51), so a realAnimated.Valueinterpolation cannot be evaluated there.iOS simulator confirmation
Verified the user-visible symptom end to end on an iPhone 17 Pro simulator (iOS 26.3), using
example/with a throwaway screen that mirrors the repro: transparent header,contentInsetAdjustmentBehavior="automatic",stickyHeaderConfig={{ offset: 96 }}, anddata+stickyHeaderIndicesboth going from empty/undefinedto populated/[0]after 1.5s. Metro resolves@legendapp/list/*straight to../src, so the same build was used for both runs and only the hook file was swapped.Accessibility-frame positions of the sticky header row, normalized against the 874pt screen:
STICKY HEADER 0yRow 0ymainThe displacement on
mainis0.262 − 0.152 = 0.110, i.e.0.110 × 874 ≈ 96.1pt— exactly the one header inset the issue describes. A ~1pt scroll onmainsnapped it into the correct position, matching the reported "any scroll corrects it" behaviour. The screen was reached by a fresh mount both times (navigate away, deep-link back in).