Skip to content

translated warnings/dont-call-proptypes #121

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

Merged
merged 8 commits into from
Jul 2, 2019

Conversation

mohshbool
Copy link
Member

I haven't translated the title because I wasn't sure whether to do so or not since some of the translations did and some others didn't!

@netlify
Copy link

netlify bot commented Jun 28, 2019

Deploy preview for ar-reactjs ready!

Built with commit 46bc192

https://deploy-preview-121--ar-reactjs.netlify.com

Copy link
Collaborator

@Fcmam5 Fcmam5 left a comment

Choose a reason for hiding this comment

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

ما شاء الله أخي محمد! عمل جميل
I had some suggestions (mainly some missing `ّ` شدّة). Also, I suggest translating the title, it's very important for the SEO and for readers and please, can you translate the strings in the examples like:
const message = `"${propName}" property of "${componentName}" has been deprecated.\n${explanation}`;

Co-Authored-By: Fortas Abdeldjalil <[email protected]>
mohshbool and others added 2 commits June 29, 2019 03:11
Co-Authored-By: Fortas Abdeldjalil <[email protected]>
Co-Authored-By: Fortas Abdeldjalil <[email protected]>
@mohshbool
Copy link
Member Author

ما شاء الله أخي محمد! عمل جميل
I had some suggestions (mainly some missing ّ شدّة). Also, I suggest translating the title, it's very important for the SEO and for readers and please, can you translate the strings in the examples like:

const message = `"${propName}" property of "${componentName}" has been deprecated.\n${explanation}`;

شكرًا!
regarding the title issue, it's written in #1 not to translate the error message itself since users search for it so (the error/warning is thrown in English)
as for the messages, I was also planning to translate the comments but it turns out that the code part is LTR so its a little messed up. preview: https://imgur.com/a/OwDiyhj

@Fcmam5
Copy link
Collaborator

Fcmam5 commented Jun 29, 2019

regarding the title issue, it's written in #1 not to translate the error message itself since users search for it so (the error/warning is thrown in English)

That makes sense, sorry I was mistaken in that point.

as for the messages, I was also planning to translate the comments but it turns out that the code part is LTR so its a little messed up. preview: https://imgur.com/a/OwDiyhj

That's true and it will be super weird if we reorder words so it will be understandable to the reader. We should find a solution for such cases

@mohshbool
Copy link
Member Author

mohshbool commented Jun 29, 2019

as for the messages, I was also planning to translate the comments but it turns out that the code part is LTR so its a little messed up. preview: https://imgur.com/a/OwDiyhj

That's true and it will be super weird if we reorder words so it will be understandable to the reader. We should find a solution for such cases

I don't think it's that necessary to translate them, I saw the RTL setup, I think the code would look a bit odd if it's written RTL

@iRayan7
Copy link
Member

iRayan7 commented Jul 2, 2019

Hi @Fcmam5, please review this PR and merge it after approving it.
make sure you checked out this note from #1

Warnings
These are the pages that you get when you click the links in the console (e.g. https://reactjs.org/warnings/dont-call-proptypes.html). People tend to search these, so please don't translate the error message itself.

Thanks.

@iRayan7 iRayan7 requested a review from Fcmam5 July 2, 2019 00:20
Copy link
Collaborator

@Fcmam5 Fcmam5 left a comment

Choose a reason for hiding this comment

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

@mohshbool, I'm sorry I missed some "micro-mistakes", would you mind reviewing them?

mohshbool and others added 3 commits July 2, 2019 13:39
Co-Authored-By: Fortas Abdeldjalil <[email protected]>
Co-Authored-By: Fortas Abdeldjalil <[email protected]>
@mohshbool
Copy link
Member Author

@mohshbool, I'm sorry I missed some "micro-mistakes", would you mind reviewing them?

@Fcmam5 I have committed your suggestions, idk how I managed to miss them 😅

@Fcmam5 Fcmam5 added approved and removed in-review labels Jul 2, 2019
@Fcmam5 Fcmam5 merged commit e37b37d into reactjs:master Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants