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

[Docs] Add a sample to clarify animation callback #16770

Closed
wants to merge 2 commits into from

Conversation

jsdario
Copy link
Contributor

@jsdario jsdario commented Nov 9, 2017

Motivation

Even having worked long with React Native, I am pretty new to animations, so it took me a while to figure out how to trigger some code after an animation had finished. After some investigation, it
was evident I did not ready as in depth as I am supposed to, but this can be a problem for many
who, like me, try to search quick through docs and other resources.

Here more cases:
https://stackoverflow.com/questions/38053071/react-native-animated-complete-event
#3212

Test Plan

Read the proposed sample, and see if it is adequate.

Release Notes

[DOCS] [ENHANCEMENT] [AnimatedImplementation.js] - Adds a sample for animation callbacks

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot 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 Nov 9, 2017
@facebook-github-bot
Copy link
Contributor

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

@jsdario jsdario changed the title Add a sample to clarify animation callback [Docs] Add a sample to clarify animation callback Nov 9, 2017
@hramos
Copy link
Contributor

hramos commented Nov 9, 2017

Can you rebase? We just landed a commit that shifts documentation to plain old markdown files in the docs/ directory. These jsdoc comments will be stripped out soon and I'd hate to lose your change.

@jsdario
Copy link
Contributor Author

jsdario commented Nov 9, 2017

Sure thing @hramos! Just did so :)

@pull-bot
Copy link

pull-bot commented Nov 9, 2017

Messages
📖

📄 Docs - Thanks for your contribution to the docs!

@facebook-github-bot label Documentation

Attention: @hramos

Generated by 🚫 dangerJS

Copy link
Contributor

@janicduplessis janicduplessis left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Left a few comments inline.

docs/animated.md Outdated
@@ -66,6 +66,16 @@ invoked with `{finished: true}`. If the animation is done because `stop()`
was called on it before it could finish (e.g. because it was interrupted by a
gesture or another animation), then it will receive `{finished: false}`.

```javascript
this.animateValue.spring({}).start(function onComplete({finished}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use arrow function here

docs/animated.md Outdated
@@ -66,6 +66,16 @@ invoked with `{finished: true}`. If the animation is done because `stop()`
was called on it before it could finish (e.g. because it was interrupted by a
gesture or another animation), then it will receive `{finished: false}`.

```javascript
this.animateValue.spring({}).start(function onComplete({finished}) {
if (finished === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (finished) {

docs/animated.md Outdated
```javascript
this.animateValue.spring({}).start(function onComplete({finished}) {
if (finished === true) {
console.log('Animation was stopped')
Copy link
Contributor

Choose a reason for hiding this comment

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

"Animation was completed" maybe?

@jsdario
Copy link
Contributor Author

jsdario commented Nov 10, 2017

Hi @janicduplessis, thanks for the comments. Just updated based on feedback 😄 I can squash commits if necessary.

@janicduplessis
Copy link
Contributor

@jsagorin Thanks, you don't have to squash commits it will be done automatically when your PR lands.

@janicduplessis
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 11, 2017
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@janicduplessis is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Nov 11, 2017
@facebook-github-bot
Copy link
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@janicduplessis
Copy link
Contributor

@hramos Can you check why this failed to import?

@jsdario
Copy link
Contributor Author

jsdario commented Nov 19, 2017

Can I do something to help with this? @hramos @janicduplessis

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 21, 2017
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hramos
Copy link
Contributor

hramos commented Nov 21, 2017

Trying to land again. I couldn't find any particular reason this failed to import.

bowerman0 pushed a commit to bowerman0/react-native that referenced this pull request Dec 5, 2017
Summary:
Even having worked long with React Native, I am pretty new to animations, so it took me a while to figure out how to trigger some code after an animation had finished. After some investigation, it
was evident I did not ready as in depth as I am supposed to, but this can be a problem for many
who, like me, try to search quick through docs and other resources.

Here more cases:
https://stackoverflow.com/questions/38053071/react-native-animated-complete-event
facebook#3212

Read the proposed sample, and see if it is adequate.

 [DOCS] [ENHANCEMENT] [AnimatedImplementation.js] - Adds a sample for animation callbacks
Closes facebook#16770

Differential Revision: D6304441

Pulled By: hramos

fbshipit-source-id: c22e391aece6a62684a78847fc74df203c2438d7
bowerman0 pushed a commit to bowerman0/react-native that referenced this pull request Dec 5, 2017
Summary:
Even having worked long with React Native, I am pretty new to animations, so it took me a while to figure out how to trigger some code after an animation had finished. After some investigation, it
was evident I did not ready as in depth as I am supposed to, but this can be a problem for many
who, like me, try to search quick through docs and other resources.

Here more cases:
https://stackoverflow.com/questions/38053071/react-native-animated-complete-event
facebook#3212

Read the proposed sample, and see if it is adequate.

 [DOCS] [ENHANCEMENT] [AnimatedImplementation.js] - Adds a sample for animation callbacks
Closes facebook#16770

Differential Revision: D6304441

Pulled By: hramos

fbshipit-source-id: c22e391aece6a62684a78847fc74df203c2438d7
@hramos hramos added the Merged This PR has been merged. label Mar 8, 2019
@react-native-bot react-native-bot removed Import Started This pull request has been imported. This does not imply the PR has been approved. Import Failed labels Mar 8, 2019
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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants