Skip to content

fix(scroll): withhold native MVCP from snapping lists on Android - #2456

Open
dennytosp wants to merge 1 commit into
Shopify:mainfrom
dennytosp:fix/snap-mvcp-drift-correction
Open

fix(scroll): withhold native MVCP from snapping lists on Android#2456
dennytosp wants to merge 1 commit into
Shopify:mainfrom
dennytosp:fix/snap-mvcp-drift-correction

Conversation

@dennytosp

@dennytosp dennytosp commented Aug 20, 2026

Copy link
Copy Markdown

Fixes #2427

Problem

On Android, a backward swipe on a horizontal snapping carousel flies past the intended card and lands on index 0, once the list has scrolled far enough that recycling has started. Forward swiping looks fine, iOS never reproduces it, and maintainVisibleContentPosition={{ disabled: true }} makes it go away.

Every link below is in the RN sources:

  1. FlashList hands the native maintainVisibleContentPosition to the ScrollView (RecyclerView.tsx), which puts Android's MaintainVisibleScrollPositionHelper in charge.
  2. That helper re-anchors on every layout change of the first visible child, and a recycling list repositions its cells constantly — so it fires throughout a fling. Each time: scrollToPreservingMomentum(scrollX + deltaX, …).
  3. scrollToPreservingMomentum calls recreateFlingAnimation(x, Integer.MAX_VALUE). Its own comment says what that does: "If we have any pending custom flings (e.g. from animated scrollTo, or flinging to a snap point), cancel them." It cancels the snap animator and re-flings the OverScroller with maxX = Integer.MAX_VALUE.
  4. The velocity carried over is not the user's. flingAndSnap boosts it first — velocityX -= (int) ((targetOffset - smallerOffset) * 10.0), commented "The default animator requires boost on initial velocity as when snapping velocity can feel sluggish for slow swipes" — and that is safe only because the same call clamps it with minX == maxX == targetOffset ("setting both minX and maxX to the same value will guarantee that we scroll to it").

Step 3 keeps the 10x velocity and throws away the clamp that made it safe. A backward swipe runs unbounded down to minX = 0.

Fix

The helper watches the cells, not FlashList's ScrollAnchor, so there is nothing to suppress on the JS side — no guard around applyOffsetCorrection can help (I verified this the hard way; see below). Withholding the native prop is what stops it.

export function isSnappingList(props) {
  return (
    props.pagingEnabled === true ||
    props.snapToInterval != null ||
    props.snapToOffsets != null
  );
}

That predicate is copied from ScrollView.js, which is what turns the native snapping path on for Android:

pagingEnabled: Platform.select({
  android:
    this.props.pagingEnabled === true ||
    this.props.snapToInterval != null ||
    this.props.snapToOffsets != null,
})

flingAndSnap is only reachable when that flag is true, so matching it covers exactly the affected lists. Checking snapToInterval alone would miss plain pagingEnabled carousels.

Gated behind a new PlatformConfig.nativeMvcpBreaksSnapFling, true only on Android — iOS does not have this failure mode and keeps the prop.

Prepending keeps working, only the mechanism changes: when the native prop is withheld, applyOffsetCorrection drives the scroll position through scrollTo instead of nudging the anchor — the same path web already uses. Verified on device (below).

Device verification

Pixel 9 Pro emulator, RN 0.84, Fabric, using the Snap Carousel Repro fixture screen added here (snapToInterval, 20 cards, "settled on" readout):

build before backward swipe after
main card 13 card 1, offset 0
main (repeat) card 9 card 1, offset 0
this branch card 17 card 16, offset 5860 ✅
this branch, 4 more backward swipes card 16 card 9 ✅

Prepend regression, Horizontal MVCP screen with a snapToInterval added: tapped Prepend 5 twice (95 → 100 → 105 items), visible cards stayed on 16/17/18 both times.

Tests

snapping.test.ts covers the predicate (including snapToInterval: 0, empty snapToOffsets, and null values, which have to behave the way ScrollView.js treats them), and RecyclerView.test.tsx asserts on the prop the ScrollView actually receives for snapping / non-snapping / unaffected-platform cases.

Both layers were mutation-tested rather than assumed:

mutation result
un-wire the gate in RecyclerView.tsx, leave the helper correct exactly the two "withholds it…" tests fail
drop pagingEnabled from the predicate the predicate test and the pagingEnabled wiring test fail
yarn test --forceExit   203 passed
yarn type-check         clean
yarn lint               clean

Note on the earlier revision of this PR

The first version of this PR guarded applyOffsetCorrection so it would not nudge ScrollAnchor on a snapping list. It had passing tests and it did not work: on device the bug reproduced unchanged. Instrumenting ScrollAnchor.scrollBy showed it is called zero times during this scenario — the anchor is never involved. The analysis in the issue is right about recreateFlingAnimation, the 10x boost and the lost clamp, but the trigger is the cells moving, not the anchor. That is why this revision moves the fix to the prop and why the tests now assert on the prop rather than on the correction.

@dennytosp
dennytosp force-pushed the fix/snap-mvcp-drift-correction branch from 61d0636 to b45e50c Compare August 20, 2026 05:33
@dennytosp
dennytosp marked this pull request as draft August 20, 2026 05:56
A backward swipe on a horizontal snapping carousel on Android flies past
the intended card and lands on index 0, once the list has scrolled far
enough for recycling to start.

FlashList hands the native maintainVisibleContentPosition to the
ScrollView, which puts Android's MaintainVisibleScrollPositionHelper in
charge. That helper re-anchors on every layout change of the first visible
child, and a recycling list repositions its cells constantly, so it fires
throughout a fling. Each time it calls scrollToPreservingMomentum ->
recreateFlingAnimation(x, Integer.MAX_VALUE), which cancels the in-flight
snap animator and re-flings the OverScroller with no upper bound.

The velocity it carries over is not the user's. flingAndSnap boosts it by
(distance to snap point) * 10 so a slow swipe still reaches its target,
which is safe only because the same call clamps it with
minX == maxX == targetOffset. Unclamped, that boosted velocity runs to the
end of its range - offset 0 for a backward swipe.

The helper watches the cells, not FlashList's ScrollAnchor, so there is
nothing to suppress on the JS side. Withhold the prop instead, for the
lists ScrollView.js marks as snapping on Android - pagingEnabled,
snapToInterval or snapToOffsets - and let their offset corrections drive
the scroll position through scrollTo, the same path web already uses.
Prepending to a snapping list keeps working; only the mechanism changes.

Verified on a Pixel 9 Pro emulator with the added fixture screen: card 17
-> one backward swipe -> card 16 with the fix, card 1 without it.

Fixes Shopify#2427
@dennytosp
dennytosp force-pushed the fix/snap-mvcp-drift-correction branch from b45e50c to 072a492 Compare August 20, 2026 06:11
@dennytosp dennytosp changed the title fix(scroll): skip MVCP drift corrections on snapping lists fix(scroll): withhold native MVCP from snapping lists on Android Aug 20, 2026
@dennytosp
dennytosp marked this pull request as ready for review August 20, 2026 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android: horizontal snapToInterval carousel over-snaps to index 0 on backward swipe (maintainVisibleContentPosition re-flings unbounded mid-snap)

1 participant