try(pinch): Wave 1 — snapshot + defensive reset + robust arrival + deferred-reenable scroll-lock#29
Closed
anton-patrushev wants to merge 2 commits into
Closed
Conversation
Switch pinchScrollDelta math from the gesture-start offset snapshot to the live ScrollView offset. On iOS where scroll is stable the two are equal; on Android where finger movement can leak into native scroll alongside the pinch, the live offset absorbs the drift and keeps the focal-anchor formula self-consistent. In onEnd, re-anchor startOffsetY to the live offset before scrollTo so the auto-compensate residual starts at 0 and the transition lands without a visible jump. Always recompute targetOffset from focal-anchor math (the prior pinchScrollDelta-shortcut was tied to snapshot semantics). Drop the iOS scroll-lock + Android skip — no longer needed once the math is drift-tolerant.
…l-lock with deferred re-enable Four Android-targeted fixes layered on the existing drift-absorb branch: 1. Revert onUpdate to snapshot semantics. Feeding scrollOffsetLive into the per-frame translateY amplified Android scroll-event burstiness (samples arrive sparsely with multi-px steps) into visible shake. 2. onBegin defensively resets pinchEndTarget = NaN and isSettling = false. Previously a stale armed target from a prior gesture (or a dropped spring callback after cancelAnimation) could leak the residual term into the next gesture, producing cross-gesture jumps. 3. Arrival reaction widened from |s - t| < 0.5 to (crossed-through OR within 2px), AND skips the same UI tick the target was armed. On Android scroll samples can step from before to past the target in one emission, never landing inside a tight tolerance — the prior logic would then never clear and the residual would stay armed. 4. Re-introduce the iOS-only-effective scroll-lock and make it work on Android via a setTimeout(0) on re-enable. The lock disables vertical scroll during pinch; re-enabling on next tick post-pinch lets the Android touch dispatcher see a clean boundary instead of being wedged mid-touch.
4 tasks
Owner
Author
|
Superseded by platform-split fix graduated into PR #28. Closing. |
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.
Experimental Wave 1 — Android pinch hardening, layered on the original drift-absorb branch. Base: `fix/timeline-height-anim-style` (PR #28). Replaces the prior drift-absorb attempt with a different set of fixes targeting separately-identified Android failure modes.
Why this branch
The drift-absorb experiment (PR #29's prior state) used `scrollOffsetLive` directly in the per-frame `pinchScrollDelta` formula. A deep-dive investigation identified that this amplifies Android scroll-event burstiness — Android scroll samples arrive sparsely and step several pixels at once, so `scrollOffsetLive` lurches between samples and that quantised noise lands 1:1 in the inner-scale `translateY`. The cure was worse than the disease on Android.
This Wave 1 takes a different approach: keep snapshot semantics, prevent drift via a re-introduced scroll-lock, and make the gesture-end machinery robust against Android's quirks.
Changes (4 layered fixes)
1. Revert `onUpdate` to snapshot semantics
`pinchScrollDelta = startOffsetY - newOffsetY` (was `scrollOffsetLive - newOffsetY`). The live-offset approach amplified Android scroll burstiness into per-frame shake; snapshot keeps the formula stable.
2. `onBegin` defensively resets gesture-end state
`pinchEndTarget.value = NaN` and `isSettling.value = false` at the start of each gesture. Previously, a stale armed target from a prior gesture whose arrival reaction never resolved (Android sparse scroll events can step past the tolerance window without landing inside it) would leak the residual term into the next gesture, producing a cross-gesture jump. Same for `isSettling` after a `cancelAnimation` drops the spring's completion callback on Reanimated v4 + Android.
3. Arrival reaction: skip same-frame-arming + crossed-through detection
4. Re-introduce scroll-lock with Android-safe re-enable
Re-add `scrollEnabled` state + `useAnimatedReaction` on `isPinching`, but route the re-enable through a `setTimeout(0)` instead of a direct synchronous setState. The prior attempt had to be skipped on Android because flipping `scrollEnabled` from false→true mid-touch wedged the ScrollView's native touch dispatcher (vertical scroll dead until next fresh `ACTION_DOWN`). Deferring re-enable to the next JS tick gives Android's dispatcher a clean event boundary, so the lock works on both platforms.
Test plan