Modified VirtualListCell component to enable 'useNativeDriver' as Animated parameter

[alariej]

The VirtualListCell component of the VirtualListView extension does not use useNativeDriver: true for Animated styles, resulting in no animations on Android.

This PR modifies the component to use 'useNativeDriver' for animating the 'top' value (moving the cell vertically in the list). The 'width' and 'left' values needed to be moved to a static style, as these values are not supported by 'useNativeDriver' (and it seems to me they were not part of the 'timing' animation anyway).

Tested on a physical device (LG G6 running Android 9).

[ghost]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ alariej sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[erictraut]

Thanks for taking the time to make a contribution.

My understanding is that the native driver supports both "top" and "left". It doesn't make sense that it would support the former but not the latter. Do you have evidence that "left" is not supported?

It makes sense that "width" is not supported by the native animation driver, but I don't think there's any need to animate the width of a cell, since the width is normally unchanged in typical use cases.

@berickson1 wrote much of this code originally, so perhaps he has some thoughts here.

[alariej]

You are right that 'left' could actually be included in the animation. However, it doesn't seem to be part of the animation in the current VirtualListCell code:

this._topAnimation = RX.Animated.timing(this._topValue, {
   toValue: top,
   duration: 200,
   delay: animationDelay,
   easing: RX.Animated.Easing.InOut(),
});

There is no variable called '_leftAnimation' nor a 'timing' definition for anything but the 'top' value. It could be that 'left' animation was planned in the original concept, but never implemented.

In any case, I wonder if it really makes sense to animate 'left' at all, since a VirtualListView is by definition / inheritance a vertically organized component.

Please let me know what you think. If required, I could either modify this PR, or submit a new PR with the 'left' animation.

Ref: https://github.com/microsoft/reactxp/blob/master/extensions/virtuallistview/src/VirtualListCell.tsx

[alariej]

Also, this code in VirtualListCell:

// On native platforms, we'll stick with translate[X|Y] because it has a performance advantage.
this._animatedStylePosition = RX.Styles.createAnimatedViewStyle({
   transform: [{
      translateY: this._topValue,
   }],
   left: this._leftValue,
});

'left' is not part of 'transform', and there is no 'translateX' anywhere.

[erictraut]

OK, thanks. The PR looks good to me. Let's give @berickson1 another day to respond. If he doesn't have time to look at it, I'll merge it as is.

[berickson1]

Apologies on the delay, I wrote feedback but never submitted my review. In it's current state, this looks like a breaking change to me since the width and left prop won't be respected beyond the initial size that is provided.

[berickson1]

This change will make the width (and left position) of items static after it's first loaded. VLV cells currently support changing widths via a prop. I'm not sure this is a desirable change, at the very least it's a breaking change. Typically width doesn't change on an item in common cases, but I can't speak for all the uses of VLV.

[alariej]

OK thanks for the feedback @erictraut and @berickson1. Let me take another look at this PR and see if I can come up with a better solution to make it non-breaking.