try(pinch): platform-split — Android per-frame scrollTo, iOS translate+lock#31
Closed
anton-patrushev wants to merge 1 commit into
Closed
Conversation
…e+lock Restore the original Android approach (per-frame scrollTo in onUpdate) which always worked smoothly on Android, while keeping the iOS-side translate-only path that avoids Fabric commit-order shake on iOS. The translate-only approach was originally introduced to fix iOS shake, but it never had a clean Android end-of-gesture story: on Android the ScrollView can leak pan-gesture motion concurrent with the pinch, and trying to absorb that drift (or lock scrollEnabled to prevent it) either amplified noise or wedged the touch dispatcher. Per-frame scrollTo sidesteps both problems on Android — the focal anchor maps directly into ScrollView contentOffset every frame, with no residual to reconcile on release. On iOS the existing path stays: - onUpdate writes pinchScrollDelta into innerScaleStyle's translateY - iOS-only scrollEnabled lock (in CalendarBody) prevents drift - onEnd fires scrollTo once + arms pinchEndTarget for auto-compensate Drop unused withSpring import + SPRING_DAMPING/SPRING_STIFFNESS — the rubber-band overshoot branch was dead code (zoomScale is clamped in onUpdate so finalZoomScale always equals zoomScale.value at onEnd). Also drop unused isSettling destructure for the same reason.
Owner
Author
|
Graduated into PR #28. Closing experiment. |
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
1 participant
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.
Experimental — Android pinch jump fix via platform-split. Base: `fix/timeline-height-anim-style` (PR #28).
Why this branch
Multiple prior attempts (drift-absorb, Wave 1 robust-arrival + deferred-reenable lock, Wave 2 stepped layout + scroll-source consolidation) tried to make the translate-only iOS approach work on Android too. Each hit a different failure mode — amplified scroll-event burstiness, wedged touch dispatcher on `scrollEnabled` toggle, or broke scroll outright when removing JS `onScroll`.
The original code path that worked smoothly on Android (per-frame `scrollTo` in `onUpdate`) was reverted because it caused Fabric commit-order shake on iOS, not Android. So: split by platform. Android gets back what worked for it; iOS keeps what works for iOS.
Approach
Android (`onUpdate`): `offsetY.value = newOffsetY; scrollTo(verticalListRef, 0, newOffsetY, false)`. The focal-anchor target maps directly into ScrollView contentOffset every frame, with no residual to reconcile on release. `pinchScrollDelta` stays at 0 so `innerScaleStyle.translateY` is just `zoomT`. No scroll-lock needed (we want the ScrollView to follow the pinch).
iOS (`onUpdate`): existing translate-only — writes `pinchScrollDelta = startOffsetY - newOffsetY` into the inner-scale's translateY. Combined with the iOS-only `scrollEnabled` lock (in `CalendarBody`), `scrollOffsetLive` stays stable and the gesture-end transition lands cleanly via `pinchEndTarget` + auto-compensate reaction.
onEnd split:
Cleanups
Test plan