Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/fix: better slow pan: if user intent is to stay on current page, _don't_ change page #596

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nmassey
Copy link
Contributor

@nmassey nmassey commented May 4, 2024

Overview

This improves swipe behavior for my full-screen carousel use case. If a user pans very slowly and ultimately not very far, we would prefer that they stay on the current page. This still allows them to "flick" to pan to the next (or previous) screen.

This should fix the following issue:

Details

  1. refactor code slighty: calculate nextPageoutside of the if branch
  2. when calculating nextPage, use velocity * 2 instead of velocity * 0.4: I liked this factor better for my usage. This value seems to work well on all of my iOS and Android test devices
  3. fix: if the calculated nextPage is actually the current page, don't go anywhere (previous behavior was to always go to next or previous page)
  4. improve: if user has mostly panned one direction, but then reverses that pan at the end of their gesture, return to current page

Screen recording

πŸ‘Ž before patch (4.0.0-alpha.10)

Notice: panning only a little bit always brings user to the next page. This happens even if it seems that the user's intent is to stay on the current page.

Screen.Recording.2024-05-04.at.11.27.07.mov

πŸ‘Ž before patch: this version shows with some other important patches on top of 4.0.0-alpha.10: this includes #577 (worklets), #574 (useSharedValue), and #576 (panOffset)

Screen.Recording.2024-05-04.at.11.37.12.mov

βœ… with this patch (also includes #577 (worklets), #574 (useSharedValue), and #576 (panOffset))

This shows good behavior with my expected user intent.

Screen.Recording.2024-05-04.at.11.42.55.mov

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label May 4, 2024
Copy link

changeset-bot bot commented May 4, 2024

πŸ¦‹ Changeset detected

Latest commit: 65e3d0f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-native-reanimated-carousel Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

vercel bot commented May 4, 2024

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 May 6, 2024 4:27pm

@nmassey nmassey changed the title feat/fix: better slow swipe: if user intent is to stay on current page, _don't_ change feat/fix: better slow pan: if user intent is to stay on current page, _don't_ change May 4, 2024
@nmassey nmassey changed the title feat/fix: better slow pan: if user intent is to stay on current page, _don't_ change feat/fix: better slow pan: if user intent is to stay on current page, _don't_ change page May 4, 2024
@dohooo
Copy link
Owner

dohooo commented May 6, 2024

Incredible work, thank you! But before merging, could you gen a changeset for this PR?

npx changeset

@BrodaNoel
Copy link
Contributor

I have the feeling that this PR is the one who is going to fix #590 (comment)

I'm using alpha.12 and seems like the only remaining bug that I have after migrating from Expo SDK 49 to 50 (thus, alpha0 to 10), is this.

Crossed fingers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants