Add support for springs for NativeAnimated on iOS #9048

ryangomba · 2016-07-27T16:43:43Z

This diff adds support for native spring animations on iOS. This overlaps some spring work done by @kmagiera on the Android side of things.

Test plan (required)

Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the spring examples.

ghost · 2016-07-27T16:44:47Z

By analyzing the blame information on this pull request, we identified @buba447 and @spicyj to be potential reviewers.

ghost · 2016-07-27T16:44:50Z

Thank you for your pull request. As you may know, we require contributors to sign our Contributor License Agreement, and we don't seem to have you on file and listed as active anymore. In order for us to review and merge your code, please email cla@fb.com with your details so we can update your status.

ghost · 2016-07-27T19:14:14Z

@ryangomba updated the pull request.

ghost · 2016-07-27T19:23:00Z

@ryangomba updated the pull request.

oblador · 2016-07-31T16:38:27Z

Related PR: #8860

ghost · 2016-08-02T18:51:27Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

javache · 2016-08-03T11:02:37Z

cc @ritzau

ryangomba · 2016-08-03T22:16:41Z

cc @janicduplessis. Do you know if anyone else is actively contributing to iOS native animations? (doesn't look like it, from the commit log). You're my go-to right now :)

janicduplessis · 2016-08-03T22:46:15Z

@javache Do you want someone from FB to have a look at the NativeAnimated PRs before shipping it? If not I can take care of them.

javache · 2016-08-04T09:35:55Z

@janicduplessis @ritzau is ramping up on a lot of the Animated stuff, he's out this week, so I'd prefer if he could take a look at this as well.

janicduplessis · 2016-08-12T17:36:03Z

@ryangomba Could you separate this into 4 different PRs? It will make the review process easier.

janicduplessis · 2016-08-12T17:43:51Z

Libraries/NativeAnimation/Nodes/RCTInterpolationAnimatedNode.m

@@ -15,6 +15,8 @@ @implementation RCTInterpolationAnimatedNode
  __weak RCTValueAnimatedNode *_parentNode;
  NSArray<NSNumber *> *_inputRange;
  NSArray<NSNumber *> *_outputRange;
+  BOOL _clampLeft;


Can you change this to work with #9366, you can revert the JS changes and assume that you will always get passed extrapolateLeft and extrapolateRight. Let's also add support for the identity value (returns the input) to have full parity with JS.

ghost · 2016-08-27T17:01:35Z

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @buba447 as a potential reviewer. Could you take a look please or cc someone with more context?

ghost · 2016-08-28T15:34:41Z

@ryangomba updated the pull request - view changes

facebook-github-bot · 2016-10-20T17:29:07Z

@ryangomba updated the pull request - view changes

ryangomba · 2016-10-20T19:12:01Z

@janicduplessis I've addressed the issues caught above; this should be good to go!

janicduplessis · 2016-10-20T20:22:20Z

@ryangomba Looks like the build is failing for TVOS, it was added pretty recently and I don't know much about it. https://travis-ci.org/facebook/react-native/jobs/169298841#L6268. Maybe @dlowder-salesforce can help here.

facebook-github-bot · 2016-10-20T20:41:38Z

@ryangomba updated the pull request - view changes

ryangomba · 2016-10-20T20:42:14Z

@janicduplessis should pass this time

Summary: This diff adds support for clamping on iOS. It separates out code originally submitted in #9048. Test plan (required) Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the new clamped spring example. Closes #9625 Differential Revision: D4053231 fbshipit-source-id: 29048de444ff5f6d7fe7dce7897399b483ee6d2d

janicduplessis · 2016-10-20T22:43:44Z

@ryangomba One test is still failing but it looks unrelated probably just flacky. Can you rebase on master there is a conflict now because I merged your clamp PR.

facebook-github-bot · 2016-10-20T22:49:02Z

@ryangomba updated the pull request - view changes

douglowder · 2016-10-20T22:58:35Z

@janicduplessis @ryangomba the Apple TV issue is happening because RCTFrameAnimation.m and RCTSpringAnimation.m are included in the iOS target, but not in the tvOS target. So UIExplorer fails to link due to missing object files for those classes.

ryangomba · 2016-10-20T23:01:04Z

@dlowder-salesforce right, it's fixed now

janicduplessis · 2016-10-20T23:02:08Z

Great! Thanks for your work on this @ryangomba, going to wait for CI then shipit.

janicduplessis · 2016-10-21T01:50:15Z

@facebook-github-bot shipit

Summary: This diff adds support for native spring animations on iOS. This overlaps some spring work done by kmagiera on the Android side of things. **Test plan (required)** Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the spring examples. Closes facebook#9048 Differential Revision: D4056088 Pulled By: foghina fbshipit-source-id: a593408cb61cb850572bab4a0884f7157cece656

Summary: This diff adds support for clamping on iOS. It separates out code originally submitted in facebook#9048. Test plan (required) Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the new clamped spring example. Closes facebook#9625 Differential Revision: D4053231 fbshipit-source-id: 29048de444ff5f6d7fe7dce7897399b483ee6d2d

Summary: This diff adds support for native spring animations on iOS. This overlaps some spring work done by kmagiera on the Android side of things. **Test plan (required)** Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the spring examples. Closes facebook#9048 Differential Revision: D4056088 Pulled By: foghina fbshipit-source-id: a593408cb61cb850572bab4a0884f7157cece656

Summary: This diff adds support for clamping on iOS. It separates out code originally submitted in facebook#9048. Test plan (required) Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the new clamped spring example. Closes facebook#9625 Differential Revision: D4053231 fbshipit-source-id: 29048de444ff5f6d7fe7dce7897399b483ee6d2d

Summary: This diff adds support for native spring animations on iOS. This overlaps some spring work done by kmagiera on the Android side of things. **Test plan (required)** Run UIExplorer NativeAnimated examples before and after - compare the results. Pay special attention to the spring examples. Closes facebook#9048 Differential Revision: D4056088 Pulled By: foghina fbshipit-source-id: a593408cb61cb850572bab4a0884f7157cece656

Summary: This diff adds ModuloAnimatedNode on iOS. It separates out code originally submitted in facebook#9048. Test plan (required) Set up an animation with a modulo node, and `useNativeModule: true`. Compare results with `useNativeModule: false`. Closes facebook#9626 Differential Revision: D3799636 fbshipit-source-id: 594499f11be41bf3ee709249056a3feedeace9eb

ghost added the GH Review: review-needed label Jul 27, 2016

ryangomba changed the title ~~NativeAnimation additions for iOS~~ [NativeAnimation] Additions for iOS Jul 27, 2016

ryangomba changed the title ~~[NativeAnimation] Additions for iOS~~ [NativeAnimation] Springs, Modulo, extrapolation, offsets for iOS Jul 27, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 2, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 3, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 4, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 12, 2016

janicduplessis reviewed Aug 12, 2016
View reviewed changes

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 27, 2016

ryangomba force-pushed the native-animation-additions branch from 987a771 to d80d701 Compare August 28, 2016 15:34

ryangomba changed the title ~~[NativeAnimation] Springs, Modulo, extrapolation, offsets for iOS~~ [NativeAnimation][iOS] Springs Aug 28, 2016

ryangomba mentioned this pull request Aug 28, 2016

Add support for clamping for NativeAnimated on iOS #9625

Closed

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 28, 2016

This was referenced Aug 28, 2016

[NativeAnimated][iOS] Modulo node #9626

Closed

[NativeAnimated][iOS] Value offsets #9627

Closed

facebook-github-bot added the GH Review: review-needed label Oct 20, 2016

janicduplessis approved these changes Oct 20, 2016

View reviewed changes

ryangomba changed the title ~~[NativeAnimation][iOS] Springs~~ Add support for springs for NativeAnimated on iOS Oct 20, 2016

[NativeAnimated][iOS] Springs

de8ffda

ryangomba force-pushed the native-animation-additions branch from 713884d to de8ffda Compare October 20, 2016 22:48

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 21, 2016

facebook-github-bot closed this in d950db4 Oct 21, 2016

ryangomba deleted the native-animation-additions branch October 21, 2016 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for springs for NativeAnimated on iOS #9048

Add support for springs for NativeAnimated on iOS #9048

ryangomba commented Jul 27, 2016 •

edited

Loading

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

oblador commented Jul 31, 2016

ghost commented Aug 2, 2016

javache commented Aug 3, 2016

ryangomba commented Aug 3, 2016

janicduplessis commented Aug 3, 2016

javache commented Aug 4, 2016

janicduplessis commented Aug 12, 2016

janicduplessis Aug 12, 2016

ghost commented Aug 27, 2016

ghost commented Aug 28, 2016

facebook-github-bot commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

facebook-github-bot commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

facebook-github-bot commented Oct 20, 2016

douglowder commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

janicduplessis commented Oct 21, 2016

Add support for springs for NativeAnimated on iOS #9048

Add support for springs for NativeAnimated on iOS #9048

Conversation

ryangomba commented Jul 27, 2016 • edited Loading

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

ghost commented Jul 27, 2016

oblador commented Jul 31, 2016

ghost commented Aug 2, 2016

javache commented Aug 3, 2016

ryangomba commented Aug 3, 2016

janicduplessis commented Aug 3, 2016

javache commented Aug 4, 2016

janicduplessis commented Aug 12, 2016

janicduplessis Aug 12, 2016

Choose a reason for hiding this comment

ghost commented Aug 27, 2016

ghost commented Aug 28, 2016

facebook-github-bot commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

facebook-github-bot commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

facebook-github-bot commented Oct 20, 2016

douglowder commented Oct 20, 2016

ryangomba commented Oct 20, 2016

janicduplessis commented Oct 20, 2016

janicduplessis commented Oct 21, 2016

ryangomba commented Jul 27, 2016 •

edited

Loading