Skip to content

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
@changeset-bot
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

@vercel
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.

@BrodaNoel
Copy link
Contributor

@nmassey any news on this?

@nmassey
Copy link
Contributor Author

nmassey commented Nov 3, 2024

hey @BrodaNoel - I've been using this patch for my project (via yarn patch), but it appears that it's not merged in to main code for other users (yet?).

Although I love the UX for my purposes, I'm not sure what the maintainer thinks about this feature. 🤔🤷‍♂️

@BrodaNoel
Copy link
Contributor

I don't think this is just "a UX improvement". This is actually a fix to an important UX bug.

The behavior you implemented here it is actually always the expected behavior. Instagram, Facebook, Twitter, everybody uses in this way, because the user can always "regret" of moving to the next photo, so they should be able to stay in the current one.

@dohooo the changeset is already done as you requested. Probably ready to merge now

@dohooo
Copy link
Owner

dohooo commented Nov 29, 2024

Due to the conflicts, I've created another PR and will add credits for you. thank you!

#731

@dohooo dohooo closed this Nov 29, 2024
@dohooo
Copy link
Owner

dohooo commented Nov 29, 2024

Released #732

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.

3 participants