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

Translate: Fragments #82

Merged
merged 4 commits into from
Feb 7, 2019

Conversation

RamirezAlex
Copy link
Contributor

This is related to #80
Please review.

Copy link
Contributor

@elyalvarado elyalvarado left a comment

Choose a reason for hiding this comment

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

Hey @RamirezAlex some suggestions

content/docs/fragments.md Outdated Show resolved Hide resolved
content/docs/fragments.md Outdated Show resolved Hide resolved
content/docs/fragments.md Outdated Show resolved Hide resolved
@RamirezAlex
Copy link
Contributor Author

Thanks @elyalvarado. I did the proper fixes you suggested.

Copy link
Member

@alejandronanez alejandronanez left a comment

Choose a reason for hiding this comment

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

Hey @RamirezAlex, thanks for wrapping this up!

The only big change I think we should do here, is to change the code back to English.

Besides that, everything looks 👍


```jsx
class Columns extends React.Component {
render() {
return (
<div>
<td>Hello</td>
<td>World</td>
<td>Hola</td>
Copy link
Member

Choose a reason for hiding this comment

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

Hey @RamirezAlex, we're leaving the code as is, we're only translating comments. So please, leave this as <td>Hello</td>

We use lots of codepen examples, and if we translate the code contents we could confuse people!


```jsx
function Glossary(props) {
return (
<dl>
{props.items.map(item => (
// Without the `key`, React will fire a key warning
// Sin la 'key', React disparará una advertencia de key
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could sound better as

Suggested change
// Sin la 'key', React disparará una advertencia de key
// Sin el prop 'key', React disparará una advertencia de key

Copy link
Member

@alejandronanez alejandronanez left a comment

Choose a reason for hiding this comment

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

Fragments should be renamed to Fragmentos :)

content/docs/fragments.md Outdated Show resolved Hide resolved
@RamirezAlex
Copy link
Contributor Author

Thank you @alejandronanez I updated the PR.

Copy link
Member

@alejandronanez alejandronanez left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your FIRST contribution 🎉 ! 🚢

@alejandronanez alejandronanez merged commit 8072f7d into reactjs:master Feb 7, 2019
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