-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Apply localization to all hard coded text #2718
Apply localization to all hard coded text #2718
Conversation
It looks good to me, but I would love a second opinion since the PR is massive and React Native is not my strongest suit. @roryabraham or @Julesssss could any of you take a look? |
Sorry @pecanoro, but I'm unable to take on additional reviews just yet -- if this is still open once my N5 blockers are cleared I'll happily take a look though. |
I can take a look! |
I think @iwiznia also wanted to take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Just a few comments
Updated |
🚀 Deployed to staging in version: 1.0.41-3🚀
|
@eVoloshchak @iwiznia Anything that needs QA here? Or can we check it off the list? |
No QA for this, but please be on the look for any text that looks wrong (ie: instead of saying something like |
🚀 Deployed to production in version: 1.0.44-0🚀
|
style: 'cancel', | ||
}, | ||
{ | ||
text: this.props.translate('common.settings'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No such key common.settings
in languages/en: https://github.com/Expensify/Expensify.cash/pull/2718/files#diff-22037c1e4dbdea8803b450b3fce06e6c475762a31ecf84b34dc264ba0e443637R3
If it ever becomes the case that the user revoked camera permissions the app will seem to do nothing when they press on "Add Attachment" -> "Take Photo".
While the app actually throws an error here
Revoked.Camera.Permissions.mp4
The problem will be worse if the user selects "Deny & don't ask" as nothing would seem to happen after you press on "Take Photo"
- Expected: When the app lacks permissions to take photos for some reason it should show alert message explaining why the "Take Photo" button doesn't work
- Actual: Pressing "Take Photo" does nothing
@kidroca great catches! @eVoloshchak can you send a PR fixing them please? |
Submitted a PR |
Details
Applied localization to the Expensify.cash app, all hard coded text is replaced with locale-aware components/ methods.
Fixed Issues
Fixes #2408
Tested On
Screenshots
Leaving out the screenshots because there are no visual changes