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

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

Open
5 tasks done
taylorkline opened this issue Nov 6, 2018 · 18 comments
Open
5 tasks done

Comments

@taylorkline
Copy link

taylorkline commented Nov 6, 2018

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:

  • React: 16.6.0-alpha.8af6728

  • react-native: 0.57.4

  • react-native-snap-carousel: 3.7.5

Target platform:

  • Android 7.1

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
Copy link
Contributor

bd-arc commented Nov 6, 2018

Hey @jkdf2,

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

@taylorkline
Copy link
Author

Hi @bd-arc,

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

Here is a Snack that shows the issue remains.

@taylorkline
Copy link
Author

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
Copy link
Contributor

bd-arc commented Nov 7, 2018

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

@antonzy
Copy link

antonzy commented Dec 20, 2018

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

@saharbit
Copy link

I am also facing the same problem.

@bd-arc
Copy link
Contributor

bd-arc commented Jan 10, 2019

Hi guys,

Does PR #443 solve the bug for you?

@taylorkline
Copy link
Author

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

@NikiTsv
Copy link

NikiTsv commented Feb 3, 2019

@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
Copy link

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

@vinhtq
Copy link

vinhtq commented Mar 12, 2019

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

@yulolimum
Copy link

yulolimum commented May 22, 2019

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
Copy link
Contributor

bd-arc commented May 27, 2019

@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
Copy link

Tsiniloiv commented Jul 4, 2019

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
Copy link

@bd-arc any progress on that fix?

@bd-arc
Copy link
Contributor

bd-arc commented Aug 2, 2019

@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
Copy link

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

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests