Fix Native Rotation Android (#18872)

Summary:
Fixes #14161
Android crashes in some cases if an animated transform config contains a string value, like a rotation.
This PR fixes that by ensuring all values sent to the native side are doubles. It adds `__transformDataType` to AnimatedTransform.js.

Added integration test `ReactAndroid/src/androidText/js/AnimatedTransformTestModule.js` This test fails with the following error `INSTRUMENTATION_RESULT: longMsg=java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Double`, if the changes to AnimatedTransform.js are reverted.

[Android] [Fixed] - Fixes Android crash on animated style with string rotation
Pull Request resolved: https://github.com/facebook/react-native/pull/18872

Differential Revision: D13894676

Pulled By: cpojer

fbshipit-source-id: 297e8132563460802e53f3ac551c3ba9ed943736
This commit is contained in:
scisci 2019-01-31 01:33:38 -08:00 committed by Facebook Github Bot
parent 02697291ff
commit e405e84fc3
6 changed files with 160 additions and 16 deletions

View File

@ -260,6 +260,22 @@ function shouldUseNativeDriver(config: AnimationConfig | EventConfig): boolean {
return config.useNativeDriver || false;
}
function transformDataType(value: any): number {
// Change the string type to number type so we can reuse the same logic in
// iOS and Android platform
if (typeof value !== 'string') {
return value;
}
if (/deg$/.test(value)) {
const degrees = parseFloat(value) || 0;
const radians = (degrees * Math.PI) / 180.0;
return radians;
} else {
// Assume radians
return parseFloat(value) || 0;
}
}
module.exports = {
API,
addWhitelistedStyleProp,
@ -272,6 +288,7 @@ module.exports = {
generateNewAnimationId,
assertNativeAnimatedModule,
shouldUseNativeDriver,
transformDataType,
get nativeEventEmitter() {
if (!nativeEventEmitter) {
nativeEventEmitter = new NativeEventEmitter(NativeAnimatedModule);

View File

@ -347,21 +347,7 @@ class AnimatedInterpolation extends AnimatedWithChildren {
}
__transformDataType(range: Array<any>) {
// Change the string array type to number array
// So we can reuse the same logic in iOS and Android platform
return range.map(function(value) {
if (typeof value !== 'string') {
return value;
}
if (/deg$/.test(value)) {
const degrees = parseFloat(value) || 0;
const radians = (degrees * Math.PI) / 180.0;
return radians;
} else {
// Assume radians
return parseFloat(value) || 0;
}
});
return range.map(NativeAnimatedHelper.transformDataType);
}
__getNativeConfig(): any {

View File

@ -103,7 +103,7 @@ class AnimatedTransform extends AnimatedWithChildren {
transConfigs.push({
type: 'static',
property: key,
value,
value: NativeAnimatedHelper.transformDataType(value),
});
}
}

View File

@ -0,0 +1,55 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.tests;
import android.view.View;
import com.facebook.react.testing.ReactInstanceSpecForTest;
import com.facebook.react.testing.StringRecordingModule;
import com.facebook.react.bridge.JavaScriptModule;
import com.facebook.react.testing.ReactAppInstrumentationTestCase;
import com.facebook.react.testing.ReactTestHelper;
/**
* Integration test for {@code removeClippedSubviews} property that verify correct scrollview
* behavior
*/
public class AnimatedTransformTest extends ReactAppInstrumentationTestCase {
private StringRecordingModule mStringRecordingModule;
@Override
protected String getReactApplicationKeyUnderTest() {
return "AnimatedTransformTestApp";
}
@Override
protected ReactInstanceSpecForTest createReactInstanceSpecForTest() {
mStringRecordingModule = new StringRecordingModule();
return super.createReactInstanceSpecForTest()
.addNativeModule(mStringRecordingModule);
}
public void testAnimatedRotation() {
waitForBridgeAndUIIdle();
View button = ReactTestHelper.getViewWithReactTestId(
getActivity().getRootView(),
"TouchableOpacity");
// Tap the button which triggers the animated transform containing the
// rotation strings.
createGestureGenerator().startGesture(button).endGesture();
waitForBridgeAndUIIdle();
// The previous cast error will prevent it from getting here
assertEquals(2, mStringRecordingModule.getCalls().size());
}
}

View File

@ -0,0 +1,81 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @providesModule AnimatedTransformTestModule
*/
'use strict';
var BatchedBridge = require('BatchedBridge');
var React = require('React');
var StyleSheet = require('StyleSheet');
var View = require('View');
var TouchableOpacity = require('TouchableOpacity');
var Text = require('Text');
var RecordingModule = require('NativeModules').Recording;
const styles = StyleSheet.create({
base: {
width: 200,
height: 200,
backgroundColor: 'red',
transform: [{rotate: '0deg'}],
},
transformed: {
transform: [{rotate: '45deg'}],
},
});
/**
* This app presents a TouchableOpacity which was the simplest way to
* demonstrate this issue. Tapping the TouchableOpacity causes an animated
* transform to be created for the rotation property. Since the property isn't
* animated itself, it comes through as a static property, but static properties
* can't currently handle strings which causes a string->double cast exception.
*/
class AnimatedTransformTestApp extends React.Component {
constructor(props) {
super(props);
this.toggle = this.toggle.bind(this);
}
state = {
flag: false,
};
toggle() {
this.setState({
flag: !this.state.flag,
});
}
render() {
// Using this to verify if its fixed.
RecordingModule.record('render');
return (
<View style={StyleSheet.absoluteFill}>
<TouchableOpacity
onPress={this.toggle}
testID="TouchableOpacity"
style={[styles.base, this.state.flag ? styles.transformed : null]}>
<Text>TouchableOpacity</Text>
</TouchableOpacity>
</View>
);
}
}
var AnimatedTransformTestModule = {
AnimatedTransformTestApp: AnimatedTransformTestApp,
};
BatchedBridge.registerCallableModule(
'AnimatedTransformTestModule',
AnimatedTransformTestModule
);
module.exports = AnimatedTransformTestModule;

View File

@ -35,6 +35,11 @@ require('TimePickerDialogTestModule');
const AppRegistry = require('AppRegistry');
const apps = [
{
appKey: 'AnimatedTransformTestApp',
component: () =>
require('AnimatedTransformTestModule').AnimatedTransformTestApp,
},
{
appKey: 'CatalystRootViewTestApp',
component: () =>