Skip to content

Conversation

@ChrisSki
Copy link
Contributor

First PR and have completed the CLA request

The reason I chose to submit this is because it was initially unclear from the docs that the transition group needed to already be mounted in order for it to work properly.

The reason I chose to submit this is because it was initially unclear from the docs that the transition group needed to already be mounted in order for it to work properly.
Copy link
Member

Choose a reason for hiding this comment

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

I'll let @chenglou handle comments about content, but just a nit that this link will be broken in the website. It should be /react/docs/animation.html#getting-started or even more simply #getting-started since it's on the same page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha. I was thinking that but wasn't sure.

@chenglou
Copy link
Contributor

Just some nits, otherwise it's good! Also, I didn't even know that. Shouldn't this be a bug?

@ChrisSki
Copy link
Contributor Author

I'm not sure if it is a bug or not. I just thought this was the way it was meant to work. Did you make the changes that you commented about above, or are you waiting for me to do this? Sorry, this is my first PR for an open source project :)

@chenglou
Copy link
Contributor

I guess you made this change through Github's edit button? I think you still can't amend the same PR through Github. You'd have to clone the repo locally, set up the branches, make the changes and yada yada (you can try that). Let's just finish correcting this if no one else has comment (@spicyj), and then you can close this PR and submit a new one.

@ChrisSki
Copy link
Contributor Author

Ok, @chenglou, I think I corrected them. Regarding }, this); I had copied the original code from the docs.

@chenglou
Copy link
Contributor

That'll be good for now. Thanks!

chenglou added a commit that referenced this pull request Aug 20, 2014
@chenglou chenglou merged commit 25c9d6d into facebook:master Aug 20, 2014
@ChrisSki ChrisSki deleted the patch-1 branch August 20, 2014 01:12
zpao added a commit that referenced this pull request Sep 3, 2014
chenglou added a commit that referenced this pull request Sep 3, 2014
zpao added a commit that referenced this pull request Sep 3, 2014
@zpao zpao removed the addons label Dec 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants