Let users override "useNativeDriver" for Shimmer / ActivityIndicator #1148
Conversation
| const nativeVersion = Platform.constants.reactNativeVersion; | ||
| if (Platform.OS === 'macos' && nativeVersion.minor >= 62) { | ||
| return true; | ||
| } else if (nativeVersion.minor >= 66) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This isn't a perfect check of whether we are running with react-native-macos or not: An iOS or Android app could be running with react-native-macos 0.63, and this would return false where it should return true. I couldn't think of a better way to do it though.
| /** | ||
| * overrides the useNativeDriver prop on the component's animation | ||
| */ | ||
| useNativeDriver?: boolean; |
There was a problem hiding this comment.
can we not expose it as a prop if we do the RN version check inside the component?
There was a problem hiding this comment.
I added the prop in case, say, a user is using some other fork of react-native or managed to patch react-native to include that fix, so they could override our default value.
There was a problem hiding this comment.
I feel like we are trying to cover all use cases rather than take it one step at a time. I personally think exposing this as a prop can be risky as it could affect performance of the control.
|
Talked offline, and I'm closing this in favor of a different solution |
2 participants
Platforms Impacted
Description of changes
A client team would like to use Shimmer & ActivityIndicator. However, we would want to make sure we set
useNativeDriverfor those applications properly to ensure the best performance based on their react-native version. Due to a bug with animations (fixed with facebook/react-native#29585 and microsoft/react-native-macos#763), we can't set the useNativeDriver prop to true unless we're on RN 0.66+ or RN-macOS 0.62+.Let's add a check that sets
useNativeDriverto it's proper value based on our RN version, and also adduseNativeDriveras a prop to Shimmer and ActivityIndicator, should the client need to override it for whatever reason (lex: they patch the bug in their own repo, or update to a newer version of react-native).Verification
Ran the iOS and macOS test app with console.logs of the what value
useNativeDriveris set to, to make sure it was set properly.Pull request checklist
This PR has considered (when applicable):