dohooo / react-native-reanimated-carousel

🎠 React Native swiper/carousel component, fully implemented using reanimated v2, support to iOS/Android/Web. (Swiper/Carousel)
https://react-native-reanimated-carousel.vercel.app
MIT License
2.87k stars 330 forks source link

fix: `onGestureEnd` -> `endWithSpring` uses outdated data via `useSharedValue` #574

Closed nmassey closed 6 months ago

nmassey commented 8 months ago

What: the bug

When endWithSpring is called, it is using the previous values for scrollEndVelocity and scrollEndTranslation (rather than the values just set in onGestureEnd). This seems to cause strange carousel behavior for the user: the function isn't using the values from the most recent pan.

From the user's standpoint, the problem is very obvious (and potentially frustrating) when the user pans one direction, then reverses direction for their next pan. (Please see video demonstration below.)

Why

This is due to updates to shared values created via useSharedValue() updating asynchronously (see https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue#remarks ).

What: proposed fix

Rather than relying on the shared data variable between function calls, let's explicitly pass the values as arguments to endWithSpring.

May resolve the following issues

  1. https://github.com/dohooo/react-native-reanimated-carousel/issues/535
  2. https://github.com/dohooo/react-native-reanimated-carousel/issues/568
  3. 579

Screengrab video recordings

buggy behavior on 4.0.0-alpha.10 (unpatched)

Notice, panning the same direction works OK, but panning alternate direction fails.

https://github.com/dohooo/react-native-reanimated-carousel/assets/589004/af7ce523-57d0-4fbf-a338-741b55056c7a

improved behavior with patch

https://github.com/dohooo/react-native-reanimated-carousel/assets/589004/55fad5c5-d702-48c8-8956-06613b014e0c

changeset-bot[bot] commented 8 months ago

⚠️ No Changeset found

Latest commit: 86da6acb823ec2deaa71f4cb90d392d1cccf48b5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 8 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-reanimated-carousel ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 23, 2024 6:39am
vricosti commented 7 months ago

When will it be released ? What is missing ?

sieeniu commented 6 months ago

when it would be merged?

dohooo commented 6 months ago

@nmassey Thanks! https://github.com/dohooo/react-native-reanimated-carousel/pull/601