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

snapToItem/snapToNext not working on Android devices when called from callbacks inside step #552

Open kgorol opened 5 years ago

kgorol 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?

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.

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

I think so.

Environment

Expo with v33 SDK

Expected Behavior

It should go to specified page or to the next.

Actual Behavior

On iOS device it is working fine, but on Android nothing happens.

Reproducible Demo

https://snack.expo.io/SJxF4GwlB

Steps to Reproduce

  1. Run on Android
  2. Try to snapToItem from inside single page view.
bd-arc commented 5 years ago

Hi @kgorol,

Can you tell me if this works properly with version 3.7.5 of the plugin?

kgorol commented 5 years ago

Thanks for replying @bd-arc,

Works like a charm on 3.7.5 :)

sansetty2510 commented 5 years ago

Still it's not working for me

calz10 commented 5 years ago
setTimeout(() => this.carouselComponent.snapToNext(), 250)

adding timeout in the current version works for me :) less than 250 will not snap to next

pedro-lb commented 5 years ago
setTimeout(() => this.carouselComponent.snapToNext(), 250)

adding timeout in the current version works for me :) less than 250 will not snap to next

works for me! thanks :)

tannguyen16 commented 5 years ago

setTimeout(() => this.carouselComponent.snapToNext(), 250)

adding timeout in the current version works for me :) less than 250 will not snap to next

Works for me too! Thanks!

Inovassist-dev commented 5 years ago

I think we shouldn't let this end here... We have just one workaround, but nothing concrete yet. Any update coming @bd-arc?

bd-arc commented 5 years ago

@Inovassist-dev I definitely agree that we need to get to the root of it.

The problem is that I need to know first and foremost if this is a bug introduced in 3.8.0 or something else. Hence my initial question.

But so far I've received contradictory answers...

Inovassist-dev commented 5 years ago

Yeah, I understand... Maybe I can help you! I'll check last versions in order to see if it's a problem in the latest one! Hope this helps!

abegehr commented 5 years ago

Have the same issue running on an iOS device: snapToItem(), snapToPrev(), and snapToNext() don't work. I spent a long time trying to debug on 3.8.0 with no success, except for @calz10's workaround (which is suboptimal since it adds a noticeable delay). @bd-arc, downgrading to 3.7.5 does not fix the problem for me. Also not after resetting all caches. What else can I try? How can we get this to work again?

QQizi commented 5 years ago

The problem is here. When using snapToNext() or snapToPrev(), _snapToItem is triggered again with the old index

daaashleywch commented 5 years ago

Also not working for me on versions 3.2 - 3.8, adding timeout does work, but the fix is far too slow and hacky to be used. Would like to know how this could be fixed as well.

daaashleywch commented 5 years ago

@QQizi any thoughts on how to fix?

daaashleywch commented 5 years ago

@QQizi @kgorol Actually got this to work by removing only iOS check for momentum bug fix here Now click through and swipe work for me on Android.

zabojad commented 5 years ago

@bd-arc I was using version at commit 85980e5177a75d393a1b38480800753282148c50 until today and did not have this problem. I just updated to latest master version (commit 1aebb185036813b1104ea01fc6229b43f10f4d9c) and I am observing this problem. So yes, there is a regression between those two commits.

sultan-arshi commented 5 years ago

+1

ALexandreM75013 commented 5 years ago

I encounter the same problem on Android (not tested on IOS) I am using react-native 0.60.5 and 3.8.1 (I also tried with 3.7.5 and it still didn't worked)

However, it was working with exactly the same code on react-native 0.59.4 and 3.7.5. So it seems the upgrade of react-native version 0.59.4 to 0.60.5 could be linked.

bd-arc commented 5 years ago

@AlexandreMaistret Thank you for the helpful feedback!

Unfortunately your conclusion is the absolute worst case scenario... And I don't see anything that is supposed to affect ScrollView and FlatList in version 0.60's release notes...

ALexandreM75013 commented 5 years ago

@bd-arc I know it is quite a really bad scenario because I looked also the releases notes of 0.60.5and nothing seems to possibly interfere with thoses functions. I just tried again just now to be sure because I was also very surprised, and same situation : works on 0.59.4 and and not with 0.60.5 with exactly the same code and both 3.7.5.

The workaround with setTimeout works thought, but it is not usable since you can feel the delay and it feels really laggy then.

Just to give you more details on the code: I am calling snapToNext() or snapToPrev() directly from the item with TouchableWithoutFeedback. The touchable event is called and go without any problem to snapToNext() function (I checked with logs) but then nothing, it does not go to the next item. Also it does not work with snapToItem() too. I tried with and without Debug JS activated, and also with JS Dev Mode off.

I didn't tried with iOS since I don't have a Mac.

sytabaresa commented 5 years ago

In iOS 12.4.1 with React Native 0.60.5, 3.7.5 and 3.8.1 versions work fine, also in production. I will test with React Native 0.59.4 soon, but I think that will work well.

francisleigh commented 5 years ago
"react-native-snap-carousel": "^3.7.5",
"react-native": "0.61.2",

requestAnimationFrame(() => this.carouselRef.snapToPrev()); requestAnimationFrame(() => this.carouselRef.snapToNext());

Is a work around if you're sceptical when upgrading dependancies.

nathantqn commented 5 years ago

This workaround setTimeout(() => this.carousel.snapToPrev(), 0) is still needed in "react-native-snap-carousel": "^3.8.2", :(

almeidacavalcante commented 4 years ago
setTimeout(() => this.carouselComponent.snapToNext(), 250)

adding timeout in the current version works for me :) less than 250 will not snap to next

You just saved the day. I almost forgot about this javascript context trick that is very usefull sometimes... thank you!

appli-intramuros commented 4 years ago

The only workaround that works for me is enabling momentum by setting props enableMomentum. I suspect that without this props, the _activeItem is not updated when calling snapToNext/snapToPrev.

source code

carmenchapa commented 4 years ago

This solution setTimeout(() => this._carousel.snapToNext(), 0) and this one requestAnimationFrame(() => this._carousel.snapToPrev()) work for me, however the animation disappears in both cases (only on android). Any hint for this problem?

valeriik commented 4 years ago

This solution setTimeout(() => this._carousel.snapToNext(), 0) and this one requestAnimationFrame(() => this._carousel.snapToPrev()) work for me, however the animation disappears in both cases (only on android). Any hint for this problem?

The same, the animation disappears in both cases (only on android).

nathantqn commented 4 years ago

I'm facing the exact same problem with @valeriik and @carmenchapa. I tried to use enableMomentum, but it makes the animation on Android looks a bit weird

nlokare commented 4 years ago

@9600baud this thread documents the issue we are seeing on android.

bfaulk96 commented 4 years ago

Just for a little further information (on the most recent 3.8.4 version): snapToNext() works if used outside of the carousel like this:

<Fragment>
  <Carousel
    ref={(c) => { this.carousel = c }}
    data={data}
    ...
  />
  <TouchableOpacity 
    style={{ backgroundColor: 'lime', position: 'absolute', bottom: 24, width: '90%', height: 42 }} 
    onPress={this.carousel.snapToNext} />
</Fragment>

But if you call this.carousel.snapToNext() from a carousel item it does not work (and must use the workarounds mentioned by others on this thread):

renderCarouselItem = ({ item, index }) => {
  return <SectionInput
    saveSection={() => { this.carousel.snapToNext(); this.saveSection() }}
    index={index}
    section={item}/>
}
bfaulk96 commented 4 years ago

Update: Pull Request #648 fixes this! Are there any maintainers (@bd-arc maybe?) that can take a look to get this fix out? It appears that the issue was, when triggering the snapToNext() or snapToPrev() based on a button inside the carousel, the touch event was being seen the same as a touch & drag event, which triggered the snapToItem() function. The PR mentioned above differentiates the two, which fixes our problem

bd-arc commented 4 years ago

@bfaulk96 Thanks for the info!

I plan on releasing an update very soon that should enhance the carousel from a "feeling" standpoint. I'll make sure to merge the PR you mentioned beforehand.

dmitryshelomanov commented 4 years ago

On android animation disappear v 3.8.4

bd-arc commented 4 years ago

PR merged and published in version 3.9.0. Let me know if this finally solves the issue!

bd-arc commented 4 years ago

Just realized there was a potential issue with the PR.

Given the fact that ScrollView's scrollTo() method doesn't accept a callback or a duration, this line won't be executed at the proper time if the scroll is animated...

27leaves commented 4 years ago

I think I have the same issue. I just tried the following code:

this.setState(
    {
        screens: newScreens,
    },
    () => {
        setTimeout(() => this.carousel.snapToNext(), 500);
    },
);

It updates the state by updating one or more screens to the current screens and afterwards snaps to the next. It ends up at the correct screen, unfortunately mostly without an animation. Sometimes it animates.

Version 3.9.0.

delnaz commented 4 years ago

When using snapToItem(), the first time the callback onSnapToItem() is not being called. However the _snapToItem is getting called twice, once with the old index.

This issue persist even in "react-native-snap-carousel": "^3.8.4".

The timeout solution works fine but is there a better way?

luigicr18 commented 4 years ago

I have the same issue, I have to use a setTimeout with snapToItem setTimeout(() => this._carousel.snapToItem(this.state.currentIndex, true, fireCallback),250)

27583

As you can see, the selected item is 85, but the snapToItem have the old one the 88. If I use a setTimeout it works, but does not look very elegant.

SamiChab commented 4 years ago

Did anyone found a solution for the animation which disappears on Android?

Heltonphg commented 3 years ago

É uma pena que uma lib tão boa esteja abandonada! Até agora nada de soluções :(

ahmetacikgoz commented 3 years ago
setTimeout(() => {
    this._carousel.snapToItem(-1);
    this._carousel.snapToItem(0);
}, 250);

I had issue to snap zero and it worked for me.

CyberCyclone commented 3 years ago

The only workaround that works for me is enabling momentum by setting props enableMomentum. I suspect that without this props, the _activeItem is not updated when calling snapToNext/snapToPrev.

source code

I don't know if this is really relevant to this issue, but enableMomentum fixed a bug where snapToNext/snapToPrev wasn't updating the _activeItem as was mentioned previously on here.

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

roots-ai commented 3 years ago

The issues still persist on the latest version for RN 0.66 and Router Flux4.3.1... the issue still occurs.

Any updates here?

xvonabur commented 2 years ago

Upgraded from 3.7.5 to 3.9.1 and faced the issue on Android with no animation after first snapToItem call. With enableMomentum={true} snapToItem animation works every time.

mixdev commented 2 years ago

This happened to me when the items.length>15. It is possibly because the VirtualizedList internally has set initialNumToRender=15. and any item with index>15 will fail in snapToItem() because it is not there. For a long time it looked like a random issue. Works ok when initialNumToRender increased to a larger number but FPS drops drammatically hence not feasible.

This is a bug IMO. Trying to figure it out.

aynurkarimov commented 2 years ago

Probably it is not the issue but maybe it will help somebody.

I had a problem with snapToNext (v3.9.1 of the lib) and animation of dots, it didn't work on Android (the current index never changed) therefore I couldn't implement a basic feature to move to the next slide. The problem has been solved with a simple component state which saves the current index of the slider (starts from 0) and instead of snapToNext used snapToItem(state + 1) and afterwards setState(state + 1).

Also I have these props on Carousel component:

const [slideNumber, setSlideNumber] = useState<number>(0);

  const handleButtonNext = useCallback(() => {
      ref.current?.snapToItem(slideNumber + 1);
      setSlideNumber(slideNumber + 1);
  }, [slideNumber, setSlideNumber]);

      <Carousel
        ref={ref}
        data={data}
        slideStyle={styles.slider}
        sliderWidth={windowWidth}
        itemWidth={windowWidth - PADDINGS.primary * 2}
        inactiveSlideOpacity={1}
        renderItem={handleRenderItem}
        enableMomentum
        onMomentumScrollEnd={() => setSlideNumber(ref.current.currentIndex)}