Skip to content

Commit

Permalink
Fix Native Rotation Android (facebook#18872)
Browse files Browse the repository at this point in the history
Summary:
Fixes facebook#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: facebook#18872

Differential Revision: D13894676

Pulled By: cpojer

fbshipit-source-id: 297e8132563460802e53f3ac551c3ba9ed943736
  • Loading branch information
scisci authored and facebook-github-bot committed Jan 31, 2019
1 parent a33d678 commit be0a159
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 16 deletions.
17 changes: 17 additions & 0 deletions Libraries/Animated/src/NativeAnimatedHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -272,6 +288,7 @@ module.exports = {
generateNewAnimationId,
assertNativeAnimatedModule,
shouldUseNativeDriver,
transformDataType,
get nativeEventEmitter() {
if (!nativeEventEmitter) {
nativeEventEmitter = new NativeEventEmitter(NativeAnimatedModule);
Expand Down
16 changes: 1 addition & 15 deletions Libraries/Animated/src/nodes/AnimatedInterpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/nodes/AnimatedTransform.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class AnimatedTransform extends AnimatedWithChildren {
transConfigs.push({
type: 'static',
property: key,
value,
value: NativeAnimatedHelper.transformDataType(value),
});
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}

}
81 changes: 81 additions & 0 deletions ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js
Original file line number Diff line number Diff line change
@@ -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;
5 changes: 5 additions & 0 deletions ReactAndroid/src/androidTest/js/TestBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ require('TimePickerDialogTestModule');
const AppRegistry = require('AppRegistry');

const apps = [
{
appKey: 'AnimatedTransformTestApp',
component: () =>
require('AnimatedTransformTestModule').AnimatedTransformTestApp,
},
{
appKey: 'CatalystRootViewTestApp',
component: () =>
Expand Down

0 comments on commit be0a159

Please sign in to comment.