meliorence / react-native-snap-carousel

Swiper/carousel component for React Native featuring previews, multiple layouts, parallax images, performant handling of huge numbers of items, and more. Compatible with Android & iOS.
BSD 3-Clause "New" or "Revised" License
10.36k stars 2.29k forks source link

Multiple small items not scrolling with momentum on iOS [Needs Fixing on latest version 3.8.4] #614

Open summerkiflain opened 5 years ago

summerkiflain commented 5 years ago

Is this a bug report, a feature request, or a question?

Yes, A Bug report.

Have you followed the required steps before opening a bug report?

Have you made sure that it wasn't a React Native bug?

Not sure.

Is the bug specific to iOS or Android? Or can it be reproduced on both platforms?

Only happening on iOS, working fine on Android, tested on simulator and physical devices.

Is the bug reproductible in a production environment (not a debug one)?

Yes happening in production env also.

Environment

I have been using expo environment to develop, its on the latest expo version sdk 35, and i have tested with the latest snap carousel version. react and react native versions are latest that are supported by the expo sdk.

Environment: React: 16.8.3 React native: https://github.com/expo/react-native/archive/sdk-35.0.0.tar.gz react-native-snap-carousel: 3.8.3

Target Platform: iOS (12.4.2)

Expected Behavior

Scrolling multiple item should be smooth like the android system.

Actual Behavior

Scrolling multiple small items horizontally with momentum is only scrolling 1 item, sometimes it looks like that its going to scroll multiple items but then it either doesn't snap to the center of the carousel or just returns back to the last item which was next to the previously centered item.

Reproducible Demo

https://snack.expo.io/@summersyed/snap-carousel-momentum-scroll-bug

Steps to Reproduce

  1. just try to scroll carousel in the above link to either left or right side, using swing gesture. which should scroll multiple item with one swipe.
  2. see that it only scrolls 1 item at a time.
  3. try this couple of times to check it carefully.

although bug is apparent on the iOS simulator provided with expo, you can check it out more clearly on an actual iOS physical device, I have tested it on iPhone 6 and iPhone X, both are not able to scroll multiple items on dev and prod version of the app.

bd-arc commented 5 years ago

Hi @summerkiflain,

You should take a look at #189, as well as this note from the doc.

Note that a prop enableMomentum is exposed.

summerkiflain commented 5 years ago

@bd-arc I have read the #189, there is no solution to the problem which I mentioned and shown in the expo example, note from the doc doesn't mention plugin not working correctly on iOS, its mostly about android and flatlist's underlying problems. I am already using enableMomentum prop, and I have seen see Momentum section in the docs too, which gives hints on using threshold throttle and other options, but none of them are fixing the issue in expo example, please take a look into expo link I provided above and let me know if there is any work around to fix the problem until react native fix their own bugs in the meantime. I have put a lot of time in posting this issue. please help me.

bd-arc commented 4 years ago

@summerkiflain Well, I don't understand what's going on with your own example.

Here's a quick one I made: https://snack.expo.io/BJKN_2coB As you can see, everything is working as intended.

Two other things to note:

  1. You should never do loopClonesPerSide={data.length}. This means you're basically multiplying the number of rendered slides by 3!
  2. Your key prop in _renderCover() wasn't on the top level component, and therefore useless.
summerkiflain commented 4 years ago

@bd-arc Thanks for the helpful expo snack, I can confirm that snap-carousel is working fine for above issue pre v3.8.0, I have reverted my dependency to v3.7.5 and its working fine, but there is a bug where snapToItem in not working correctly on android devices when used with enableMomentum true. which is fixed on v3.8.3, I need that too P.S i would like to stay on the latest version, anyway you can check what went wrong on v3.8.0 that caused such behavior on iOS devices? thanks again.

JoaoPedroRodrigues commented 4 years ago

This is an awesome package, really is! But it seems like momentum isn't even working on iPhone, just put the first example on the example folder to work, added enableMomentum={true} and nothing changes.

summerkiflain commented 4 years ago

@JoaoPedroRodrigues for now just revert your react-native-snap-carousel dependency to version 3.7.5 for momentum to work on iOS, its not working fine for me on latest release 3.8.4 also.

JoaoPedroRodrigues commented 4 years ago

I tried reverting, even in version 3.7.5 it doesn't seem to work.

summerkiflain commented 4 years ago

@JoaoPedroRodrigues checkout the https://snack.expo.io/BJKN_2coB (posted by bd-arc) expo snack in iOS device, its working totally fine and for me its also working with v3.7.5, may be there is something else wrong with your configuration, you can try to create your own expo snack and post a new issue if its still not working for you.

JoaoPedroRodrigues commented 4 years ago

@summerkiflain I tried that snack out, my case is not with small sized items tho, not sure if that's what is making it behave differently. I haven't created an expo snack yet because I'm basically running the first example on the example folder. Thank you for your help tho!

summerkiflain commented 4 years ago

@bd-arc Any update on this, this is not resolved on the latest version 3.8.4, I have been using 3.7.5 for quite sometime now, please reopen this ticket. Thanks.

summerkiflain commented 4 years ago

@bd-arc I have narrowed down the issue to this commit: https://github.com/archriss/react-native-snap-carousel/commit/406c8f0963d232d0cb9c8df8daf0ade8eb8bf6e0 fix(Carousel): make sure that autoplay is properly restarted after a touchStart event

bd-arc commented 4 years ago

Thanks @summerkiflain, that's useful!

I assume the culprit is this. Two different features seem to contradict themselves here: autoplay & momentum.

I won't be able to look into it further for the time being, but if you want to investigate that'll be appreciated ;-)

summerkiflain commented 4 years ago

Hi @bd-arc, I really don't use autoplay feature at-all, I'll think about fixing the code for users who don't use autoplay and create a new PR. will see what can be done for users using autoplay feature later.

rogerkerse commented 4 years ago

Hi @bd-arc, I really don't use autoplay feature at-all, I'll think about fixing the code for users who don't use autoplay and create a new PR. will see what can be done for users using autoplay feature later.

Have you managed to get it working? I ran into same issue, that with 3.8.4 enableMomentum is not working and I cannot go onto older version since then having some other initial item doesn't work properly 😕

summerkiflain commented 4 years ago

Have you managed to get it working? I ran into same issue, that with 3.8.4 enableMomentum is not working and I cannot go onto older version since then having some other initial item doesn't work properly 😕

Couldn't get around to fix it for PR, I just change the line 1324 to onTouchEnd: this._onTouchEnd in Carousel.js to fix momentum issue, I don't use autoscroll so I don't care if its not working.

dohooo commented 3 years ago

Sorry, please allow me to advertise for my open source library! ~ I think this library react-native-reanimated-carousel will solve your problem. It is a high performance and very simple component, complete with React-Native reanimated 2