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

Make sure that onSnapToItem is triggered AFTER scroll animation's completion #34

Closed
bd-arc opened this issue Feb 6, 2017 · 9 comments
Closed

Comments

@bd-arc
Copy link
Contributor

bd-arc commented Feb 6, 2017

Currently, React Native doesn't provide a callback for the scrollTo() method, which is a bummer.

The ScrollView component has a prop onScrollAnimationEnd, but unfortunately this works on iOS only (even though it is referenced in the Java file). Moreover, a bug currently prevents the callback from being fired with horizontal scrollers.

Another lead would be to link the duration of the scroll animation to a timer before firing the callback. Unfortunately, scrollTo() doesn't accept a custom duration.

An idea might be to activate momentum events on Android and see if onMomentumScrollEnd can provide a good enough feedback without impairing performance too much. But this event might not be fired when snapping programatically... Moreover, it is automatically enabled if a momentum callback has been set.

ScrollResponder is another lead worth pursuing...

@piuccio
Copy link

piuccio commented May 9, 2017

marginally related to this, inside onSnapToItem callback the value of this.carousel.currentIndex is still the previous index not the one we're snapping too.

I noticed this because I'm using

        <Carousel
          ref={(carousel) => { this.carousel = carousel; }}
          onSnapToItem={(index) => {
            console.log('snap', index, this.carousel.currentIndex);
            this.forceUpdate()
          }}
        >
          {screens}
        </Carousel>
        <CarouselControls
          onPress={(page) => this.carousel.snapToItem(page)}
          selected={this.carousel ? this.carousel.currentIndex : 0}
          length={screens.length}
        />

CarouselControls is my custom component that uses this.carousel.currentIndex so I'm using forceUpdate instead of duplicating the state in my component, but the currentIndex is not uptodate

@bd-arc
Copy link
Contributor Author

bd-arc commented May 9, 2017

@piuccio Thanks for your feedback! This is pretty strange; I'll take a look at it as soon as possible.

@joseygordev
Copy link

Some answer about this problem ?

@bd-arc
Copy link
Contributor Author

bd-arc commented May 21, 2017

Hi @joseygordev, are you referring to @piuccio's issue or to the original one?

Regarding the former, I should be able to take a look at it in the next few days; regarding the latter, we are still awaiting an evolution of React Native's source code. The issue is still pending 6 months later, without any feedback from RN's team...

@bd-arc
Copy link
Contributor Author

bd-arc commented Jun 2, 2017

Hi guys,

I finally had time to dig into this issue. I can confirm that, sadly, it is deeply related to React Native's implementation.

For all the reasons explained in my first post, we can't rely on anything but the onScrollEndDrag callback when enableMomentum is disabled. Unfortunately, this means that activeItem computation will be wrong if you haven't scrolled enough so that the next slide became active. Here is a short screencast of what I mean: http://i.imgur.com/VseNsKh.gif

Since FlatList doesn't resolve any of the issues, we're kind of stuck right now :-(

@bd-arc
Copy link
Contributor Author

bd-arc commented Jun 2, 2017

I just had an idea worth trying, which is to rely on slide animation's completion and to combine it with a time threshold to "determine" if scroll has ended.

This is clearly far from ideal, but it might still lead to better results than onScrollEndDrag. I'll keep you posted!

@bd-arc
Copy link
Contributor Author

bd-arc commented Jun 7, 2017

@piuccio @joseygordev Version 2.2.1 should resolve your issue with the faulty index.

I've completely refactored the way callbacks are handled when momentum is disabled. Let's face it: this is hacky since ScrollView neither provides callbacks to the scrollTo method nor gives access to a onScrollEnd event.

Make sure to play with the new props scrollEndDragThrottleValue and snapCallbackDebounceValue. Let me know how this works for you.

@bd-arc bd-arc closed this as completed Jun 7, 2017
@joseygordev
Copy link

@bd-arc thank you very much for you attention, component very nice.

@bd-arc
Copy link
Contributor Author

bd-arc commented Jun 16, 2017

@joseygordev Note that version 2.3.0 handles no-momentum callbacks way better ;-)

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

No branches or pull requests

3 participants