Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for delete animation in LayoutAnimation on Android #7171

Closed

Conversation

janicduplessis
Copy link
Contributor

@janicduplessis janicduplessis commented Apr 23, 2016

Android follow up to #6779

Test plan
Tested add/removing views in the UIExample explorer with and without setting a LayoutAnimation. Tested that user interation during the animation is properly disabled.

layout-anim-2

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Apr 23, 2016
@janicduplessis janicduplessis force-pushed the layout-animation-android branch 2 times, most recently from 5bfb4cf to b5cb260 Compare April 24, 2016 06:06
@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

2 similar comments
@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

@ghost
Copy link

ghost commented Apr 29, 2016

@janicduplessis updated the pull request.

@mkonicek
Copy link
Contributor

#socool

Let me send a link to this PR to Olivier who implemented the LayoutAnimation on Android.

Also cc @astreet and @kmagiera

@ghost
Copy link

ghost commented May 16, 2016

@ericvicenti would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@mkonicek
Copy link
Contributor

@janicduplessis I just pinged @astreet and Olivier.

@@ -38,6 +39,8 @@ const UIExplorerNavigationReducer = require('./UIExplorerNavigationReducer');
const UIExplorerStateTitleMap = require('./UIExplorerStateTitleMap');
const URIActionMap = require('./URIActionMap');

UIManager.setLayoutAnimationEnabledExperimental(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just enable this for the specific UIExplorerApp that showcases layout animations?

Copy link
Contributor Author

@janicduplessis janicduplessis May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, I added a LayoutAnimation example in UIExplorer directly and I think the animation example app is still part of UIExplorer anyway.

I could also just enable/disable it when showing the LayoutAnimation example page but I don't know if it is really necessary.

@astreet
Copy link
Contributor

astreet commented May 17, 2016

Thanks for taking this on, looks like the right direction! I'd also like to get Oli's thoughts

@ghost
Copy link

ghost commented May 20, 2016

@janicduplessis updated the pull request.

@ghost
Copy link

ghost commented May 20, 2016

@janicduplessis updated the pull request.

@ghost
Copy link

ghost commented May 20, 2016

@janicduplessis updated the pull request.

@ghost
Copy link

ghost commented May 20, 2016

@janicduplessis updated the pull request.

@janicduplessis
Copy link
Contributor Author

@astreet Thanks for reviewing this! Addressed most of the things you pointed out.

@astreet
Copy link
Contributor

astreet commented May 26, 2016

Sorry about the late review, looks great, thanks for updating! :)

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels May 26, 2016
@ghost
Copy link

ghost commented May 26, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 0fb5ccf May 26, 2016
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jun 6, 2016
Summary:
Android follow up to #6779

**Test plan**
Tested add/removing views in the UIExample explorer with and without setting a LayoutAnimation. Tested that user interation during the animation is properly disabled.

![layout-anim-2](https://cloud.githubusercontent.com/assets/2677334/14760549/d60ebe2a-0914-11e6-8f17-ea04d8bf813b.gif)
Closes facebook/react-native#7171

Differential Revision: D3352450

Pulled By: astreet

fbshipit-source-id: 233efa041626eb26d99511d12a924e54a10f96cc
samerce pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
Android follow up to facebook#6779

**Test plan**
Tested add/removing views in the UIExample explorer with and without setting a LayoutAnimation. Tested that user interation during the animation is properly disabled.

![layout-anim-2](https://cloud.githubusercontent.com/assets/2677334/14760549/d60ebe2a-0914-11e6-8f17-ea04d8bf813b.gif)
Closes facebook#7171

Differential Revision: D3352450

Pulled By: astreet

fbshipit-source-id: 233efa041626eb26d99511d12a924e54a10f96cc
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
Android follow up to facebook#6779

**Test plan**
Tested add/removing views in the UIExample explorer with and without setting a LayoutAnimation. Tested that user interation during the animation is properly disabled.

![layout-anim-2](https://cloud.githubusercontent.com/assets/2677334/14760549/d60ebe2a-0914-11e6-8f17-ea04d8bf813b.gif)
Closes facebook#7171

Differential Revision: D3352450

Pulled By: astreet

fbshipit-source-id: 233efa041626eb26d99511d12a924e54a10f96cc
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants