diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js index f153251a5..e1b604044 100644 --- a/Libraries/Animated/src/AnimatedImplementation.js +++ b/Libraries/Animated/src/AnimatedImplementation.js @@ -82,7 +82,7 @@ type AnimationConfig = { class Animation { __active: bool; __isInteraction: bool; - __nativeTag: number; + __nativeId: number; __onEnd: ?EndCallback; start( fromValue: number, @@ -91,7 +91,11 @@ class Animation { previousAnimation: ?Animation, animatedValue: AnimatedValue ): void {} - stop(): void {} + stop(): void { + if (this.__nativeId) { + NativeAnimatedAPI.stopAnimation(this.__nativeId); + } + } _getNativeAnimationConfig(): any { // Subclasses that have corresponding animation implementation done in native // should override this method @@ -105,9 +109,9 @@ class Animation { } __startNativeAnimation(animatedValue: AnimatedValue): void { animatedValue.__makeNative(); - this.__nativeTag = NativeAnimatedHelper.generateNewAnimationTag(); + this.__nativeId = NativeAnimatedHelper.generateNewAnimationId(); NativeAnimatedAPI.startAnimatingNode( - this.__nativeTag, + this.__nativeId, animatedValue.__getNativeTag(), this._getNativeAnimationConfig(), this.__debouncedOnEnd.bind(this) @@ -311,6 +315,7 @@ class TimingAnimation extends Animation { } stop(): void { + super.stop(); this.__active = false; clearTimeout(this._timeout); window.cancelAnimationFrame(this._animationFrame); @@ -381,6 +386,7 @@ class DecayAnimation extends Animation { } stop(): void { + super.stop(); this.__active = false; window.cancelAnimationFrame(this._animationFrame); this.__debouncedOnEnd({finished: false}); @@ -595,6 +601,7 @@ class SpringAnimation extends Animation { } stop(): void { + super.stop(); this.__active = false; window.cancelAnimationFrame(this._animationFrame); this.__debouncedOnEnd({finished: false}); diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index 6085a0f54..e70749802 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -16,7 +16,7 @@ var NativeAnimatedModule = require('NativeModules').NativeAnimatedModule; var invariant = require('fbjs/lib/invariant'); var __nativeAnimatedNodeTagCount = 1; /* used for animated nodes */ -var __nativeAnimationTagCount = 1; /* used for started animations */ +var __nativeAnimationIdCount = 1; /* used for started animations */ type EndResult = {finished: bool}; type EndCallback = (result: EndResult) => void; @@ -38,9 +38,13 @@ var API = { assertNativeAnimatedModule(); NativeAnimatedModule.disconnectAnimatedNodes(parentTag, childTag); }, - startAnimatingNode: function(animationTag: number, nodeTag: number, config: Object, endCallback: EndCallback): void { + startAnimatingNode: function(animationId: number, nodeTag: number, config: Object, endCallback: EndCallback): void { assertNativeAnimatedModule(); - NativeAnimatedModule.startAnimatingNode(nodeTag, config, endCallback); + NativeAnimatedModule.startAnimatingNode(animationId, nodeTag, config, endCallback); + }, + stopAnimation: function(animationId: number) { + assertNativeAnimatedModule(); + NativeAnimatedModule.stopAnimation(animationId); }, setAnimatedNodeValue: function(nodeTag: number, value: number): void { assertNativeAnimatedModule(); @@ -101,8 +105,8 @@ function generateNewNodeTag(): number { return __nativeAnimatedNodeTagCount++; } -function generateNewAnimationTag(): number { - return __nativeAnimationTagCount++; +function generateNewAnimationId(): number { + return __nativeAnimationIdCount++; } function assertNativeAnimatedModule(): void { @@ -114,6 +118,6 @@ module.exports = { validateProps, validateStyles, generateNewNodeTag, - generateNewAnimationTag, + generateNewAnimationId, assertNativeAnimatedModule, }; diff --git a/Libraries/Animated/src/__tests__/AnimatedNative-test.js b/Libraries/Animated/src/__tests__/AnimatedNative-test.js index 4e17ef207..d6b254e4a 100644 --- a/Libraries/Animated/src/__tests__/AnimatedNative-test.js +++ b/Libraries/Animated/src/__tests__/AnimatedNative-test.js @@ -28,6 +28,7 @@ describe('Animated', () => { nativeAnimatedModule.connectAnimatedNodes = jest.genMockFunction(); nativeAnimatedModule.disconnectAnimatedNodes = jest.genMockFunction(); nativeAnimatedModule.startAnimatingNode = jest.genMockFunction(); + nativeAnimatedModule.stopAnimation = jest.genMockFunction(); nativeAnimatedModule.setAnimatedNodeValue = jest.genMockFunction(); nativeAnimatedModule.connectAnimatedNodeToView = jest.genMockFunction(); nativeAnimatedModule.disconnectAnimatedNodeFromView = jest.genMockFunction(); @@ -59,6 +60,7 @@ describe('Animated', () => { expect(nativeAnimatedModule.connectAnimatedNodes.mock.calls.length).toBe(2); expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith( + jasmine.any(Number), jasmine.any(Number), {type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)}, jasmine.any(Function) @@ -164,6 +166,7 @@ describe('Animated', () => { var nativeAnimatedModule = require('NativeModules').NativeAnimatedModule; expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith( + jasmine.any(Number), jasmine.any(Number), {type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)}, jasmine.any(Function) @@ -269,4 +272,22 @@ describe('Animated', () => { .toBeCalledWith(jasmine.any(Number), { type: 'props', props: { style: jasmine.any(Number) }}); }); + it('send stopAnimation command to native', () => { + var value = new Animated.Value(0); + var animation = Animated.timing(value, {toValue: 10, duration: 50, useNativeDriver: true}); + var nativeAnimatedModule = require('NativeModules').NativeAnimatedModule; + + animation.start(); + expect(nativeAnimatedModule.startAnimatingNode).toBeCalledWith( + jasmine.any(Number), + jasmine.any(Number), + {type: 'frames', frames: jasmine.any(Array), toValue: jasmine.any(Number)}, + jasmine.any(Function) + ); + var animationId = nativeAnimatedModule.startAnimatingNode.mock.calls[0][0]; + + animation.stop(); + expect(nativeAnimatedModule.stopAnimation).toBeCalledWith(animationId); + }); + }); diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/AnimationDriver.java b/ReactAndroid/src/main/java/com/facebook/react/animated/AnimationDriver.java index f2426c79c..ad715d45c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/AnimationDriver.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/AnimationDriver.java @@ -17,9 +17,10 @@ import com.facebook.react.bridge.Callback; */ /*package*/ abstract class AnimationDriver { - boolean mHasFinished = false; - ValueAnimatedNode mAnimatedValue; - Callback mEndCallback; + /*package*/ boolean mHasFinished = false; + /*package*/ ValueAnimatedNode mAnimatedValue; + /*package*/ Callback mEndCallback; + /*package*/ int mId; /** * This method gets called in the main animation loop with a frame time passed down from the diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index 6778d739d..ab4e19fc5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -212,6 +212,7 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements @ReactMethod public void startAnimatingNode( + final int animationId, final int animatedNodeTag, final ReadableMap animationConfig, final Callback endCallback) { @@ -219,6 +220,7 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements @Override public void execute(NativeAnimatedNodesManager animatedNodesManager) { animatedNodesManager.startAnimatingNode( + animationId, animatedNodeTag, animationConfig, endCallback); @@ -226,6 +228,16 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements }); } + @ReactMethod + public void stopAnimation(final int animationId) { + mOperations.add(new UIThreadOperation() { + @Override + public void execute(NativeAnimatedNodesManager animatedNodesManager) { + animatedNodesManager.stopAnimation(animationId); + } + }); + } + @ReactMethod public void connectAnimatedNodes(final int parentNodeTag, final int childNodeTag) { mOperations.add(new UIThreadOperation() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index c833a75d3..057546802 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -98,6 +98,7 @@ import javax.annotation.Nullable; } public void startAnimatingNode( + int animationId, int animatedNodeTag, ReadableMap animationConfig, Callback endCallback) { @@ -117,11 +118,34 @@ import javax.annotation.Nullable; } else { throw new JSApplicationIllegalArgumentException("Unsupported animation type: " + type); } + animation.mId = animationId; animation.mEndCallback = endCallback; animation.mAnimatedValue = (ValueAnimatedNode) node; mActiveAnimations.add(animation); } + public void stopAnimation(int animationId) { + // in most of the cases there should never be more than a few active animations running at the + // same time. Therefore it does not make much sense to create an animationId -> animation + // object map that would require additional memory just to support the use-case of stopping + // an animation + for (int i = 0; i < mActiveAnimations.size(); i++) { + AnimationDriver animation = mActiveAnimations.get(i); + if (animation.mId == animationId) { + // Invoke animation end callback with {finished: false} + WritableMap endCallbackResponse = Arguments.createMap(); + endCallbackResponse.putBoolean("finished", false); + animation.mEndCallback.invoke(endCallbackResponse); + mActiveAnimations.remove(i); + return; + } + } + // Do not throw an error in the case animation could not be found. We only keep "active" + // animations in the registry and there is a chance that Animated.js will enqueue a + // stopAnimation call after the animation has ended or the call will reach native thread only + // when the animation is already over. + } + public void connectAnimatedNodes(int parentNodeTag, int childNodeTag) { AnimatedNode parentNode = mAnimatedNodes.get(parentNodeTag); if (parentNode == null) { diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java index 9e713ee0d..5465f0746 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java @@ -32,9 +32,12 @@ import org.powermock.modules.junit4.rule.PowerMockRule; import org.robolectric.RobolectricTestRunner; import static org.fest.assertions.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -110,6 +113,7 @@ public class NativeAnimatedNodeTraversalTest { JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1d); Callback animationCallback = mock(Callback.class); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d), animationCallback); @@ -143,6 +147,7 @@ public class NativeAnimatedNodeTraversalTest { JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d); Callback animationCallback = mock(Callback.class); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d), animationCallback); @@ -209,11 +214,13 @@ public class NativeAnimatedNodeTraversalTest { Callback animationCallback = mock(Callback.class); JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 101d), animationCallback); mNativeAnimatedNodesManager.startAnimatingNode( + 2, 2, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1010d), animationCallback); @@ -258,6 +265,7 @@ public class NativeAnimatedNodeTraversalTest { Callback animationCallback = mock(Callback.class); JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 101d), animationCallback); @@ -304,6 +312,7 @@ public class NativeAnimatedNodeTraversalTest { // Start animating for the first addition input node, will have 2 frames only JavaOnlyArray firstFrames = JavaOnlyArray.of(0d, 1d); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", firstFrames, "toValue", 200d), animationCallback); @@ -311,6 +320,7 @@ public class NativeAnimatedNodeTraversalTest { // Start animating for the first addition input node, will have 6 frames JavaOnlyArray secondFrames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1d); mNativeAnimatedNodesManager.startAnimatingNode( + 2, 2, JavaOnlyMap.of("type", "frames", "frames", secondFrames, "toValue", 1010d), animationCallback); @@ -370,11 +380,13 @@ public class NativeAnimatedNodeTraversalTest { Callback animationCallback = mock(Callback.class); JavaOnlyArray frames = JavaOnlyArray.of(0d, 1d); mNativeAnimatedNodesManager.startAnimatingNode( + 1, 1, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 2d), animationCallback); mNativeAnimatedNodesManager.startAnimatingNode( + 2, 2, JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 10d), animationCallback); @@ -401,4 +413,53 @@ public class NativeAnimatedNodeTraversalTest { mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); verifyNoMoreInteractions(mUIImplementationMock); } + + /** + * This test verifies that when {@link NativeAnimatedModule#stopAnimation} is called the animation + * will no longer be updating the nodes it has been previously attached to and that the animation + * callback will be triggered with {@code {finished: false}} + */ + @Test + public void testHandleStoppingAnimation() { + createSimpleAnimatedViewWithOpacity(1000, 0d); + + JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.2d, 0.4d, 0.6d, 0.8d, 1.0d); + Callback animationCallback = mock(Callback.class); + mNativeAnimatedNodesManager.startAnimatingNode( + 404, + 1, + JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 1d), + animationCallback); + + ArgumentCaptor callbackResponseCaptor = ArgumentCaptor.forClass(ReadableMap.class); + + reset(animationCallback); + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + verify(mUIImplementationMock, times(2)) + .synchronouslyUpdateViewOnUIThread(anyInt(), any(ReactStylesDiffMap.class)); + verifyNoMoreInteractions(animationCallback); + + reset(animationCallback); + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.stopAnimation(404); + verify(animationCallback).invoke(callbackResponseCaptor.capture()); + verifyNoMoreInteractions(animationCallback); + verifyNoMoreInteractions(mUIImplementationMock); + + assertThat(callbackResponseCaptor.getValue().hasKey("finished")).isTrue(); + assertThat(callbackResponseCaptor.getValue().getBoolean("finished")).isFalse(); + + reset(animationCallback); + reset(mUIImplementationMock); + // Run "update" loop a few more times -> we expect no further updates nor callback calls to be + // triggered + for (int i = 0; i < 5; i++) { + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + } + + verifyNoMoreInteractions(mUIImplementationMock); + verifyNoMoreInteractions(animationCallback); + } }