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

Using the Effect Hook #66

Merged
merged 24 commits into from
Feb 9, 2019
Merged

Using the Effect Hook #66

merged 24 commits into from
Feb 9, 2019

Conversation

escorponox
Copy link
Contributor

No description provided.

@alejandronanez
Copy link
Member

👋 @escorponox welcome to this project.

Is this your PR still in progress? I see a lot of translated content but the title says [WIP], if it is ready for review, please update the title.

Thanks!

@escorponox
Copy link
Contributor Author

There is still a lot of content to be translated. I will remove the [WIP] once it is finished.

Thanks!!!

@escorponox escorponox changed the title [WIP] Using the Effect Hook Using the Effect Hook Feb 6, 2019
@escorponox
Copy link
Contributor Author

escorponox commented Feb 6, 2019

Ready for review. @alejandronanez @carburo @dmoralesm

@carburo carburo self-requested a review February 7, 2019 16:24
Copy link
Member

@carburo carburo left a comment

Choose a reason for hiding this comment

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

That was a big one. Really good job!
I pointed out some small issues that should be addressed before merging.

content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
content/docs/hooks-effect.md Show resolved Hide resolved
content/docs/hooks-effect.md Outdated Show resolved Hide resolved
carburo and others added 16 commits February 7, 2019 22:31
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
Co-Authored-By: escorponox <[email protected]>
@escorponox
Copy link
Contributor Author

@carburo I think all issues are fixed.

content/docs/hooks-effect.md Show resolved Hide resolved
>
>Eagle-eyed readers may notice that this example also needs a `componentDidUpdate` method to be fully correct. We'll ignore this for now but will come back to it in a [later section](#explanation-why-effects-run-on-each-update) of this page.
>Los lectores avispados podrán percatarse de que este ejemplo necesita también un método `componentDidUpdate` para ser completamente correcto. De momento vamos a ignorar este hecho, pero volveremos a él en una [sección posterior](#explanation-why-effects-run-on-each-update) de esta página.
Copy link
Member

Choose a reason for hiding this comment

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

No biggie, but I think that detallistas sounds a bit better than avispados.

What do you think?

Suggested change
>Los lectores avispados podrán percatarse de que este ejemplo necesita también un método `componentDidUpdate` para ser completamente correcto. De momento vamos a ignorar este hecho, pero volveremos a él en una [sección posterior](#explanation-why-effects-run-on-each-update) de esta página.
>Los lectores detallistas podrán percatarse de que este ejemplo necesita también un método `componentDidUpdate` para ser completamente correcto. De momento vamos a ignorar este hecho, pero volveremos a él en una [sección posterior](#explanation-why-effects-run-on-each-update) de esta página.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perspicaces may fit better imho. Do you agree? detallista is used for personal relationships in Spain

https://dle.rae.es/?id=SkJr6XL

@carburo carburo merged commit 4a7d069 into reactjs:master Feb 9, 2019
@escorponox escorponox deleted the hooks-effect branch February 11, 2019 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants