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.32k stars 2.28k forks source link

onSnapToItem & onBeforeSnapToItem do not fire reliably on left / right edges on Android #425

Open taylorkline opened 5 years ago

taylorkline commented 5 years ago

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

Bug report

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

(Check the step you've followed - replace the space character between the square brackets ([]) by an x.)

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

Yes.

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

Android only, I believe

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

Yes.

Environment

Env:

Target platform:

Expected Behavior

Expected onSnapToItem to be fired reliably for every index.

Actual Behavior

onSnapToItem does not fire if scrolling all the way to the left or right edges.

gfycat video (look at the bottom for the Active index display)

Reproducible Demo

https://snack.expo.io/SJ6vPURhm

Steps to Reproduce

From the example, scroll to a middle slide on Android.

Scroll all the way to the left or right and release.

The state index is not updated.

bd-arc commented 5 years ago

Hey @jkdf2,

Have you tried to get rid of containerCustomStyle={{ paddingLeft: 36 }}? Because it can mess with the active slide calculations...

taylorkline commented 5 years ago

Hi @bd-arc,

Great suggestion. Unfortunately, removing the containerCustomStyle does not change the behavior.

Here is a Snack that shows the issue remains.

taylorkline commented 5 years ago

It would appear that the issue is rooted in _onScroll's itemReached being incorrectly set to false if the finger is dragged all the way to the left / right slides on Android.

bd-arc commented 5 years ago

@jkdf2 Thanks for the heads up! I'll look into it as soon as possible.

antonzy commented 5 years ago

@bd-arc Is there a workaround for the meanwhile?

saharbit commented 5 years ago

I am also facing the same problem.

bd-arc commented 5 years ago

Hi guys,

Does PR #443 solve the bug for you?

taylorkline commented 5 years ago

@bd-arc No change. Are you sure you tagged the right PR? I'm not seeing how that PR relates to this issue.

NikiTsv commented 5 years ago

@bd-arc I have the same problem. Really blocking thingy and I can't find a proper Swiper that works with many items like this awesome component. Here is some debugging information: I have 3 items (indexes).

  1. When snapping from item 0 (first) to item 1 (mid) both onSnapToItem and onBeforeSnapToItem are called with correct index.
  2. When snapping from item 1 (mid) to item 2 (last) only onBeforeSnapToItem is called.
  3. When snapping from item 2 (last) to item 1 (mid) nothing is called.
  4. When snapping from item 1 (mid) to item 0 (first) - both events are called.

When using onViewableItemsChanged instead sometimes the "ViewableItems" become 2 instead of 3.

Do you have any info if this is going to be resolved because I might need to find some other component? ps. thanks for your contribution I couldn't have ever created such a nice component

flaviaferreira commented 5 years ago

Hi @jkdf2 I had the same problem. I set the enableMomentum property to true and it worked for me.

vinhtq commented 5 years ago

I also have the same issue? Did any one have workaround for this?

yulolimum commented 5 years ago

I ran into this issue as well. Spent a few minutes logging various things inside the package and here's what i found (in case someone finds it useful). The reason this is not an issue on iOS (or when enableMomentum prop is enabled on Android ) is because there is over-scroll (scrollview/flatlist scrolls beyond the content). This triggers a re-render of the View and the first/last items snap.

When enableMomentum is disabled on Android (default), there's no over-scroll. So when all the logic in the Carousel resolves and determines the next snap-to item, the View does not re-render because there is nothing to trigger the re-render. If there was over-scroll (like on iOS), scrolling just one pixel past the edges would re-render and snap the items. You can reproduce this yourself by manually setting the ScrollView offset to 1 when you reach 0. This triggers and update and items snap and callbacks are fired.

I added a very quick and hacky fix to my project by manually calling snapTo depending on the ScrollViews scroll offset.

image

I am also attaching a patch-file for the latest library version in case this helps someone.

In both the screenshot and the patch file, the "fix" is for vertical carousels only and may cause performance issues; however, it's been working out so far.

carousel-demo

bd-arc commented 5 years ago

@yulolimum Thanks a lot for the valuable information!

I'll see what I can do with it in order to implement a built-in fix ;-)

Tsiniloiv commented 5 years ago

Any progress on this? I'm experiencing the same issue. Yuolimum's fix helps, but it's still not super reliable.

EDIT: For the folks that are experiencing this issue, a possible workaround is to set swipeThreshold to 0. I tried it just now, and my carousel seems to be behaving much better now.

austinthetaco commented 5 years ago

@bd-arc any progress on that fix?

bd-arc commented 5 years ago

@austinthetaco Unfortunately no. I haven't been able to work on the plugin recently due to work overload.

If someone wants to jump in, now's the chance!

christopher-18 commented 4 years ago

I was also facing the similar issue with horizontal carousel. I needed the equal spacing from left , right of the first cards as well as tin between the cards. In between the card spacing was achieved as mentioned in the document, and for aligning the leftmost and right most card spacing from screen was done by setting the contentContainerCustomStyle and setting these values, which ultimately lead to changing the carousel logic , therefore snapToIndex was not triggering.

So i ended up adding onBeforeSnapToIndex along with it as well as enableMomentum true. This is how it worked for me.

<Carousel
                        data={this.state.bottomCarousel.dataValue}
                        renderItem={this._renderItemForBottomCarousel}
                        sliderWidth={screenWidth}
                        itemWidth={bottomCardItemWidth}
                        inactiveSlideScale={2}
                        inactiveSlideOpacity={1}
                        contentContainerCustomStyle={{
                            left:0, // Left spacing for the very first card
                            paddingRight:0 //right spacing for very last card 
                        }}
                        onBeforeSnapToItem={(index) => this.setState({ bottomActiveSlide: index })}
                        onSnapToItem={(index) => this.setState({ bottomActiveSlide: index })}
                        enableMomentum={true}
                    />

Hope this will help someone.

dohooo commented 2 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