feature: allow serialized animated event as onScroll

[Jberivera]

Platforms affected

All

What does this PR do?

This PR is for allowing to pass a serializable Animated.event as an onScroll property, I was trying to spread the array of maps that use Animated.event as the first argument but react-native was just using the first item in that array of maps, so I made a fix for using or sharing the animated value passed as prop instead of the one that it's defined in the library.

What testing has been done on this change?

using a regular callback as a prop for onScroll and using an Animated.event with useNativeDriver set on

I'm going to be testing the feature listed below, to be sure this change doesn't break anything.

Tested features checklist

• Default setup (example)
• Carousels with and without momentum enabled (prop enableMomentum)
• Vertical carousels (prop vertical)
• Slide alignment (prop activeSlideAlignment)
• Autoplay (prop autoplay)
• Loop mode (prop loop)
• ScrollView/FlatList carousels (prop useScrollView)
• Callback methods
• ParallaxImage component
• Pagination component
• Layouts and custom interpolations

The next gif link is some evidence that despite that this ticket was just about using Animated.event, it is not breaking custom interpolations, so in order to show you that, I'm going to comment this block of code. Take into account that even without this fix this is not going to affect old Carousel configurations, only new ones with onScroll={Animated.event([{ nativeEvent: { contentOffset: { x: this.translateX } } }], { useNativeDriver: true })} as a prop, so the fix is only for those who want to use useNativeDriver and custom interpolations at same time.

The next gif-bug is not the final result. see the comment above.

https://user-images.githubusercontent.com/4753246/49823369-7f02d000-fd4d-11e8-98ef-e806a5fa2916.gif

After adding commented lines back and using the same example configuration as shown below.

constructor (props) {
        ...
        this.translateX = new Animated.Value(0);
}

customExample (number, title, refNumber, renderItemFunc) {
        const isEven = refNumber % 2 === 0;

        // Do not render examples on Android; because of the zIndex bug, they won't work as is
        return !IS_ANDROID ? (
            <View style={[styles.exampleContainer, isEven ? styles.exampleContainerDark : styles.exampleContainerLight]}>
                <Text style={[styles.title, isEven ? {} : styles.titleDark]}>{`Example ${number}`}</Text>
                <Text style={[styles.subtitle, isEven ? {} : styles.titleDark]}>{title}</Text>
                <Carousel
                  data={isEven ? ENTRIES2 : ENTRIES1}
                  renderItem={renderItemFunc}
                  sliderWidth={sliderWidth}
                  itemWidth={itemWidth}
                  containerCustomStyle={styles.slider}
                  contentContainerCustomStyle={styles.sliderContentContainer}
                  scrollInterpolator={scrollInterpolators[`scrollInterpolator${refNumber}`]}
                  slideInterpolatedStyle={animatedStyles[`animatedStyles${refNumber}`]}
                  useScrollView={true}
                  onScroll={Animated.event([{ nativeEvent: { contentOffset: { x: this.translateX } } }], { useNativeDriver: true })}
                />
            </View>
        ) : false;
    }

   render() {
        ....
        const example5 = this.customExample(5, 'Custom animation 1', 1, this._renderItem);
        const example6 = this.customExample(6, 'Custom animation 2', 2, this._renderLightItem);
        const example7 = this.customExample(7, 'Custom animation 3', 3, this._renderDarkItem);
        const example8 = this.customExample(8, 'Custom animation 4', 4, this._renderLightItem);
        ....
   }

final result.

note that since they all are sharing the same Animated.Value reference they are going to be in sync in some way, but this is just because of the way code example above works.

[bd-arc]

Hey @Jberivera,

Thanks for the carefully crafted PR!

In the RN thread you're referring to, it is mentioned that the issue was solved in RN 0.44. Do you have more info about that?

[Jberivera]

Hey @Jberivera,

Thanks for the carefully crafted PR!

In the RN thread you're referring to, it is mentioned that the issue was solved in RN 0.44. Do you have more info about that?

Hi @bd-arc, I didn't find anything more about the issue, just that it was happening to me in react-native version 0.57.5 and if this bug is something that appears and disappears between version, this fix guarantees us that we don't need to be worried about.

I squashed this last change I was wondering if it is important to try to re-set this handler references or there is no need for doing that. Thanks for your attention.

[bd-arc]

Hey @Jberivera,

Regarding your last change, I'm not a 100 % positive that forcing a component update is required. To be honest, I've never had to do that — which doesn't mean that we should avoid it entirely ;-)

If you're unsure about this, I propose to keep your change in the code and to comment it. This way, if we end up discovering side effects, we could try enabling it. What do you think?

[Jberivera]

Hey @Jberivera,

Regarding your last change, I'm not a 100 % positive that forcing a component update is required. To be honest, I've never had to do that — which doesn't mean that we should avoid it entirely ;-)

If you're unsure about this, I propose to keep your change in the code and to comment it. This way, if we end up discovering side effects, we could try enabling it. What do you think?

@bd-arc Updated.

[agarant]

@Jberivera great PR!

I just tested with a vertical carousel and it works no problem.

[agarant]

@bd-arc Is there a blocker on this PR, can I help with something to get this merged ?

[bd-arc]

@agarant That's nice of you to ask!

The only issue has been the lack of time to work on the plugin lately. I'll update the component and merge a few PR (including this one) real soon ;-)

[bd-arc]

Available in version 3.8.0.