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

RN 0.63 Button compatibility - useNativeDriver was not specified #1227

Open
Frexuz opened this issue Aug 2, 2020 · 7 comments
Open

RN 0.63 Button compatibility - useNativeDriver was not specified #1227

Frexuz opened this issue Aug 2, 2020 · 7 comments
Labels
enhancement plat: react-native Primarily affects react-native (iOS / Android) rn-breaking-change

Comments

@Frexuz
Copy link

Frexuz commented Aug 2, 2020

React Native now complains if useNativeDriver wasn't specified when animating.

Screenshot 2020-08-02 at 22 34 30

setOpacityTo(value: number, duration: number) {
Animated.timing(
this._opacityAnimatedValue!,
{
toValue: value,
duration: duration,
easing: Animated.Easing.InOut(),
},
).start();
}

const timingConfig: RN.Animated.TimingAnimationConfig = {
toValue: config.toValue,
easing: config.easing ? config.easing.function : undefined,
duration: config.duration,
delay: config.delay,
isInteraction: config.isInteraction,
useNativeDriver: config.useNativeDriver,
};

I can PR, but not sure if it's safe to just add useNativeDriver: true?

@deregtd
Copy link
Collaborator

deregtd commented Aug 2, 2020

Not all animations can use native driver, so maybe we just need to force config.useNativeDriver to non-optional in typescript now.

@fbartho
Copy link
Contributor

fbartho commented Aug 2, 2020

@deregtd that sounds like the right plan. -- I wish there was a cleaner API from react-native that would tell us if a particular Animation supports useNativeDriver. Then we could detect those cases and conditionally set the flag.

This feels like an API Wart from RN's side.

@deregtd
Copy link
Collaborator

deregtd commented Aug 2, 2020

API warts from react native are a significant portion of the reason ReactXP exists. :)

@Frexuz
Copy link
Author

Frexuz commented Aug 3, 2020

But the Button is only animating opacity in setOpacityTo. Why wouldn't this be ok?

 // reactxp/src/native-common/Button.tsx

 setOpacityTo(value: number, duration: number) { 
     Animated.timing( 
         this._opacityAnimatedValue!, 
         { 
             toValue: value, 
             duration: duration, 
             easing: Animated.Easing.InOut(), 
+            useNativeDriver: true, 
         }, 
     ).start(); 
 } 

@deregtd
Copy link
Collaborator

deregtd commented Aug 3, 2020

Ah, I misunderstood the suggestion. That change is okay, since the code appears to only animate opacity specifically in the button. But also we need to change the config's type to be nonoptional now to find any other broken spots.

@fbartho
Copy link
Contributor

fbartho commented Aug 9, 2020

@Frexuz -- I think we should do this. Do you want to submit a PR for it?

@fbartho fbartho added enhancement plat: react-native Primarily affects react-native (iOS / Android) rn-breaking-change labels Aug 9, 2020
@fbartho fbartho added this to the 2.1.0 or 3.0.0 milestone Aug 9, 2020
@Frexuz
Copy link
Author

Frexuz commented Aug 10, 2020

@fbartho yep :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement plat: react-native Primarily affects react-native (iOS / Android) rn-breaking-change
Projects
None yet
Development

No branches or pull requests

3 participants