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

Added Localizable Strings for wallet translation from english to french #41

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

Ytemiloluwa
Copy link
Contributor

Description

Structured the wallet for Localization.

Notes to the reviewers

Translated the wallet from english to french.

Changelog notice

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@Ytemiloluwa Ytemiloluwa requested a review from reez August 2, 2023 15:17
Copy link
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

This is looking nice so far!

To-Do items:

  1. Per our call earlier double check each item in the UI visually to make sure they look right in French (for example "Create a new Wallet" button text was cut off in French).

  2. Per my message in Discord check out this WWDC23 video and migrate to string catalog, which gives us a lot of nice upgrades, two specifically:

  • anytime we add a new localized string in the UI (just a plain old Text("Hello)) it will automatically pick it up and put it in the localized catalog so we know what new words we need to translate for
  • the string catalog keeps a running percentage of how much of each language is localized, like right now it tells me that french is ~75% complete

@reez
Copy link
Collaborator

reez commented Aug 9, 2023

Adding screenshots in French below...

Simulator Screenshot - iPhone 14 Pro - 2023-08-09 at 14 25 45
Simulator Screenshot - iPhone 14 Pro - 2023-08-09 at 14 25 49
Simulator Screenshot - iPhone 14 Pro - 2023-08-09 at 14 26 03
Simulator Screenshot - iPhone 14 Pro - 2023-08-09 at 14 26 11
Simulator Screenshot - iPhone 14 Pro - 2023-08-09 at 14 26 47

Copy link
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

This is almost there! A few small changes:

  • change the xcscheme as mentioned in the code comment I added during this PR review
  • if you open up Xcode 15 with the new changes in this PR (that include my changes you rebased on) you should get this added...
Screenshot 2023-08-09 at 3 11 35 PM

...because it knows there are new words to translate. I think it should do that whether or not you have a simulator but let me know if it is strictly because you don't have a simulator and thus Xcode 15 will not pick up on those new changes (in which case I can follow up with a PR that adds those changes)

After that then we are good to merge!

Also if you want (not something that has to be done before merging, just a nice to have) you can update the French for "Restore Wallet from Keychain" which isn't there yet so we are 96% complete on the French translation right now:

Screenshot 2023-08-09 at 3 18 31 PM

Copy link
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

Awesome!

@reez reez merged commit d1562b1 into main Aug 12, 2023
1 check passed
@reez reez deleted the localization branch August 12, 2023 13:26
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.

2 participants