From 1c1ef6372d624e4dc63751347d64bd826e35561e Mon Sep 17 00:00:00 2001 From: scisci Date: Mon, 16 Apr 2018 01:12:35 -0400 Subject: [PATCH 1/3] adds __transformDataType to AnimatedTransform --- .../Animated/src/nodes/AnimatedTransform.js | 18 +++- .../react/tests/AnimatedTransformTest.java | 55 +++++++++++ .../js/AnimatedTransformTestModule.js | 92 +++++++++++++++++++ ReactAndroid/src/androidTest/js/TestBundle.js | 5 + 4 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 ReactAndroid/src/androidTest/java/com/facebook/react/tests/AnimatedTransformTest.java create mode 100644 ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js diff --git a/Libraries/Animated/src/nodes/AnimatedTransform.js b/Libraries/Animated/src/nodes/AnimatedTransform.js index 703fa51ceb4e1b..a39a40e9747d8e 100644 --- a/Libraries/Animated/src/nodes/AnimatedTransform.js +++ b/Libraries/Animated/src/nodes/AnimatedTransform.js @@ -87,6 +87,22 @@ class AnimatedTransform extends AnimatedWithChildren { super.__detach(); } + __transformDataType(value: any): number { + // Change the string array type to number array + // 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; + } + } + __getNativeConfig(): any { const transConfigs = []; @@ -103,7 +119,7 @@ class AnimatedTransform extends AnimatedWithChildren { transConfigs.push({ type: 'static', property: key, - value, + value: this.__transformDataType(value), }); } } diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/tests/AnimatedTransformTest.java b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/AnimatedTransformTest.java new file mode 100644 index 00000000000000..f2ddc0d2e9ed37 --- /dev/null +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/AnimatedTransformTest.java @@ -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()); + } + +} diff --git a/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js b/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js new file mode 100644 index 00000000000000..f529eb3e254455 --- /dev/null +++ b/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js @@ -0,0 +1,92 @@ +/** + * 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 ReactNativeViewAttributes = require('ReactNativeViewAttributes'); +var StyleSheet = require('StyleSheet'); +var View = require('View'); +var Animated = require('Animated'); +var TouchableOpacity = require('TouchableOpacity'); +var Text = require('Text'); +var RecordingModule = require('NativeModules').Recording; + + +var requireNativeComponent = require('requireNativeComponent'); + +var appInstance = null; + +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 + }; + + UNSAFE_componentWillMount() { + appInstance = this; + } + + toggle() { + this.setState({ + flag: !this.state.flag + }); + } + + render() { + // Using this to verify if its fixed. + RecordingModule.record('render'); + + return ( + + + TouchableOpacity + + + ); + } +} + +var AnimatedTransformTestModule = { + AnimatedTransformTestApp: AnimatedTransformTestApp, +}; + +BatchedBridge.registerCallableModule( + 'AnimatedTransformTestModule', + AnimatedTransformTestModule +); + +module.exports = AnimatedTransformTestModule; diff --git a/ReactAndroid/src/androidTest/js/TestBundle.js b/ReactAndroid/src/androidTest/js/TestBundle.js index 6fb617821b6754..6e910054742650 100644 --- a/ReactAndroid/src/androidTest/js/TestBundle.js +++ b/ReactAndroid/src/androidTest/js/TestBundle.js @@ -35,6 +35,11 @@ require('TimePickerDialogTestModule'); const AppRegistry = require('AppRegistry'); const apps = [ + { + appKey: 'AnimatedTransformTestApp', + component: () => + require('AnimatedTransformTestModule').AnimatedTransformTestApp, + }, { appKey: 'CatalystRootViewTestApp', component: () => From 243ca5079abdade10e16ff5f1766a8bc1bf614d3 Mon Sep 17 00:00:00 2001 From: scisci Date: Tue, 29 Jan 2019 22:48:19 -0500 Subject: [PATCH 2/3] extracts __transformDataType to NativeAnimatedHelper for AnimatedInterpolation and AnimatedTransform --- Libraries/Animated/src/NativeAnimatedHelper.js | 17 +++++++++++++++++ .../src/nodes/AnimatedInterpolation.js | 16 +--------------- .../Animated/src/nodes/AnimatedTransform.js | 18 +----------------- 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index 79e76db4cf3d5c..bf8243c4b58e78 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -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); diff --git a/Libraries/Animated/src/nodes/AnimatedInterpolation.js b/Libraries/Animated/src/nodes/AnimatedInterpolation.js index d426a2931defe2..1ba304d331ad5f 100644 --- a/Libraries/Animated/src/nodes/AnimatedInterpolation.js +++ b/Libraries/Animated/src/nodes/AnimatedInterpolation.js @@ -347,21 +347,7 @@ class AnimatedInterpolation extends AnimatedWithChildren { } __transformDataType(range: Array) { - // 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 { diff --git a/Libraries/Animated/src/nodes/AnimatedTransform.js b/Libraries/Animated/src/nodes/AnimatedTransform.js index a39a40e9747d8e..e3b3034675055c 100644 --- a/Libraries/Animated/src/nodes/AnimatedTransform.js +++ b/Libraries/Animated/src/nodes/AnimatedTransform.js @@ -87,22 +87,6 @@ class AnimatedTransform extends AnimatedWithChildren { super.__detach(); } - __transformDataType(value: any): number { - // Change the string array type to number array - // 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; - } - } - __getNativeConfig(): any { const transConfigs = []; @@ -119,7 +103,7 @@ class AnimatedTransform extends AnimatedWithChildren { transConfigs.push({ type: 'static', property: key, - value: this.__transformDataType(value), + value: NativeAnimatedHelper.transformDataType(value), }); } } From 7b32b8336f128c78ca0f46876b14957bddcbaacb Mon Sep 17 00:00:00 2001 From: scisci Date: Wed, 30 Jan 2019 20:01:25 -0500 Subject: [PATCH 3/3] fixes eslint errors for rotation tests --- Libraries/Animated/src/NativeAnimatedHelper.js | 2 +- .../androidTest/js/AnimatedTransformTestModule.js | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index bf8243c4b58e78..2ac39f024ab63a 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -268,7 +268,7 @@ function transformDataType(value: any): number { } if (/deg$/.test(value)) { const degrees = parseFloat(value) || 0; - const radians = degrees * Math.PI / 180.0; + const radians = (degrees * Math.PI) / 180.0; return radians; } else { // Assume radians diff --git a/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js b/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js index f529eb3e254455..f5c9e863d31336 100644 --- a/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js +++ b/ReactAndroid/src/androidTest/js/AnimatedTransformTestModule.js @@ -11,19 +11,12 @@ var BatchedBridge = require('BatchedBridge'); var React = require('React'); -var ReactNativeViewAttributes = require('ReactNativeViewAttributes'); var StyleSheet = require('StyleSheet'); var View = require('View'); -var Animated = require('Animated'); var TouchableOpacity = require('TouchableOpacity'); var Text = require('Text'); var RecordingModule = require('NativeModules').Recording; - -var requireNativeComponent = require('requireNativeComponent'); - -var appInstance = null; - const styles = StyleSheet.create({ base: { width: 200, @@ -50,16 +43,12 @@ class AnimatedTransformTestApp extends React.Component { } state = { - flag: false + flag: false, }; - UNSAFE_componentWillMount() { - appInstance = this; - } - toggle() { this.setState({ - flag: !this.state.flag + flag: !this.state.flag, }); }