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

Translations with update script #2156

Merged
merged 11 commits into from
Mar 22, 2021
Merged

Conversation

andrepimenta
Copy link
Member

@andrepimenta andrepimenta commented Jan 21, 2021

Description
This PR adds new translations and an update script to update them faster.

The update-script.js updates all the languages inside the languages folder according to en.json. This ensures that, when we get new or updated languages, they will get checked against the default locale: All keys will get organized like en.json, existing keys will get updated, missing keys will be added and any extra keys will get removed.

Example of a response from the script:

hi-in.json updated!
id-id.json updated!
ja-jp.json updated!
ko-kr.json updated!
pt-br.json updated!
ru-ru.json updated!
tl.json updated!
vi-vn.json updated!
zh-cn.json updated!
All done!

Copy this to i18n.js if it's not there

import en from './languages/en.json'
import esXl from './languages/es-xl.json'
import es from './languages/es.json'
import hiIn from './languages/hi-in.json'
import idId from './languages/id-id.json'
import jaJp from './languages/ja-jp.json'
import koKr from './languages/ko-kr.json'
import ptBr from './languages/pt-br.json'
import ruRu from './languages/ru-ru.json'
import tl from './languages/tl.json'
import viVn from './languages/vi-vn.json'
import zhCn from './languages/zh-cn.json'

I18n.translations = {
        en,
        esXl,
        es,
        hiIn,
        idId,
        jaJp,
        koKr,
        ptBr,
        ruRu,
        tl,
        viVn,
        zhCn,
}```

@andrepimenta andrepimenta requested a review from a team as a code owner January 21, 2021 17:50
@andrepimenta andrepimenta added needs-qa Any New Features that needs a full manual QA prior to being added to a release. needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jan 21, 2021
locales/i18n.js Outdated
};
// If language selected get locale
getUserPreferableLocale();

const currentLocale = I18n.currentLocale();

// Is it a RTL language?
export const isRTL = currentLocale.indexOf('he') === 0 || currentLocale.indexOf('ar') === 0;
export const isRTL = currentLocale.indexOf('jaJp') === 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

is 'jaJp' the only RTL lang currently? maybe we could set this up to be ready for more languages now? eg:

const RTLLangs = [ 'jaJp' ];

isRTL = () => RTLLangs.some(lang => currentLocale.indexOf(lang) === 0)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is actually not doing anything, but will change it

@omnat
Copy link
Contributor

omnat commented Feb 4, 2021

@andrepimenta talking to Aaron realized that Japanese isn't a Right to Left language. So we don't need to treat Japanese strings differently. Treat all strings the same.

Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

This run through is for Hindi, I still have to go through the other languages.

Issue 1:

Not sure if we're worried about this or not, but when switching languages in iOS, I see this warning; = https://recordit.co/V0uV3RsN79

Screen Shot 2021-02-05 at 4 30 26 PM

Issue 2: This one is for Filipino Language

The backup your wallet reminder on the bottom overlaps with text on smaller devices (iPhone 5s)

Screen Shot 2021-02-05 at 4 36 52 PM

Steps to reproduce:

  • create a new wallet
  • skip back up
  • go to settings > general > and change language to Hindi
  • go back to wallet home, and look at the reminder alert at the bottom

Issue 3:

The onboarding tutorial for Explore the browser is shown above when it's usually shown below; seen here = https://recordit.co/O2suPwBhkJ

Screen Shot 2021-02-05 at 5 11 38 PM

Steps to reproduce:

  • Smaller device needed (I reproduced on iPhone 5s)
  • after changing language to Hindi, go to browser, and scroll to the bottom and tap on the take a tour button

Issue 4:

On ETH Sign on smaller devices, I am unable to scroll to view account banner, and sign message; seen here = https://recordit.co/opftC4uxjg

Screen Shot 2021-02-05 at 8 42 49 PM

steps to reproduce:

  • Smaller device (I reproduced on iPhone 5s)
  • go to metamask.github.io/test-dapp
  • connect, and scroll down to ETH sign and tap it

Issue 5:

On smaller devices the not enough ETH banner at the bottom of the send flow gets cut off

image

steps:

  • iPhone 5s
  • go to send flow with an account that has 0 ETH

Issue 6:

On smaller devices the protect your wallet modal is stretched

image

steps:

  • iPhone 5s
  • create a wallet but skip back up
  • on wallet view copy address to clipboard to bring up the protect your wallet modal

Issue 7: all untranslated places

  • When a wallet isn't backed up the "Unprotected" isn't translated

Screen Shot 2021-02-05 at 5 03 49 PM

  • Password is not translated in any of the text input fields that show password (login view, trying to back up wallet, change password, or view seed phrase/private key)

Screen Shot 2021-02-05 at 7 07 35 PM

  • When entering an RPC network, the warning for Chain ID is not translated

Screen Shot 2021-02-05 at 8 00 12 PM

  • On Android, some txn types are not translated (Approve / Contract Deployment / Smart Contract Interaction) in comparison to iOS

Screen Shot 2021-02-05 at 8 39 31 PM

  • The not enough ETH message when confirming a txn

Screen Shot 2021-02-05 at 8 29 29 PM

  • When going through the onboarding flow, the steps on the top are not translated on Android in comparison to iOS

image

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Feb 6, 2021
@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented Feb 8, 2021

Bahasa Indonesian Language Issues:

Issue 8:

On the "Secure your wallet" screen on smaller screens (iPhone 5s) I can't tap the X to close the "seed phrase" > "What is a 'Seed phrase'" modal. The only option I have is to tap out of the modal since the X is not shown.

image

Similar instance is the next view on the onboarding secure your wallet flow, upon tapping the "Why is it important?" link the X is so close to the text it's hard to distinguish.

image

Issue 9:

Similar to Issue 6 above for Hindi, for Bahasa Indonesian Language it's also stretched on smaller devices (iPhone 5s) as well as the "X" to close the modal overlapping with the text

image

Issue 10:

The "Add funds" button icon alignment is off for Bahasa Indonesian Language on Android (pixel emulator)

image

Issue 11:

The view hint button on smaller devices (iPhone 5s) has overlapping text

image

Also over laps in Japanese:
Screen Shot 2021-02-09 at 2 03 54 PM

Issue 12:

Similar to Issue 4 I can scroll now in Indonesian however, I cannot read anything on smaller device (iPhone 5s); seen here = http://recordit.co/UMb1pMgPMO

Issue 13:

Add to favorites in browser menu is getting cut off on various screen sizes (iPhone 11 Pro, iPhone 5s, Pixel 2)

Screen Shot 2021-02-08 at 6 06 09 PM

Issue 14:

The approve icon on Android doesn't display in txn history

image

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented Feb 9, 2021

Issue 15:

When I am on the Korean translation and I am on approve modal, and try and go to advance gas I get this error = http://recordit.co/unZ9qatNzi

Screen Shot 2021-02-09 at 2 25 43 PM

Screen Shot 2021-02-09 at 2 25 54 PM

Go to an approve modal in browser, or tap on this link to have it immediately pop up as a deep link

https://metamask.app.link/approve/0x1d7a9cd8c67e3c74a84bb76cbc1b606e2e460db5@4/approve?address=0x1FDb169Ef12954F20A15852980e1F0C122BfC1D6&uint256=1

Issue 16:

In Korean translation, on android app I'm not seeing approve icon nor received icon in transaction history compared to iOS

image

Issue 17:

When language is set to Portuguese - Brazil, the collectibles buttons/icons and text is misaligned on smaller device (iPhone 5s)

Screen Shot 2021-02-09 at 5 29 19 PM

Russian as well:

Screen Shot 2021-02-09 at 5 51 44 PM

@omnat
Copy link
Contributor

omnat commented Feb 9, 2021

Per the discussion: Issues 1, 7, 10, 14 won't be fixed with this merge. Reason: either they'd be fixed by other work, or considering them minor/ non-blockers. The missing translations in Issue 7 are likely due to lack of good enough translations - and we could iterate on it with community & user feedback over time.

For the rest of the issues, these are the fixes we came up with:

Issue 2, 13:

Text flows out of the error message:
image

Fix:

Make the text field such that the long texts display in 2 lines. Rough visual note:
image

Fix for 13 similar:

Make the text of 'Add to Favorites' go to 2 lines:
image

Issue 3:

image

Fix:

Show the blue tutorial box below the browser, instead of above it.

Issue 4, 12:

Issue of scrollable area on Sign screen:
image

Fix:

  • Include error message in the scrollable area
  • Maximize the Sign message action sheet height to 90% of the screen (so we utilize the space available above on small screens)

Issue 5:

The error message box cuts of text/copy in small screen
image

Fix:

Extend the yellow error message height, and let it go below the fold - user can scroll to see the full message.

Issue 6, 8, 9:

Problem of the 'x' (close) button overlapping with the copy, and not clickable.
image
image
image
image

Fix:

Add padding around 'x' button and ensure its clickable. The overlapping text can be broken to 2 lines like in the fix of Issues 2&13

Issue 11:

Overlapping text and clickable buttons (of text style)
image
image

Fix:

Make the text go to 2 lines. Add padding to the blue text buttons, so they never overlap and are readable + clickable.

@andrepimenta @cjeria Please add to it if I missed something.
cc @ibrahimtaveras00

@andrepimenta
Copy link
Member Author

andrepimenta commented Feb 10, 2021

@ibrahimtaveras00 Issues mentioned by @omnat should be solved. Many of these issues also happen when english is selected when you have a smaller device. So we should check the entire app on smaller devices and list all the problems. But yes those problems can be even more problematic on other languages.

@andrepimenta andrepimenta added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Feb 10, 2021
@Gudahtt
Copy link
Member

Gudahtt commented Feb 17, 2021

We received some feedback about the vi-vn translations here: MetaMask/metamask-extension#10270 (comment)

Line 250: Missing "tiền" - Nhận tiền
Line 339: Missing "thông minh" (smart contract = hợp đồng thông minh)
Line 608: Replace "Phí gas hủy" with "Phí hủy gas"
Line 613: Replace "Phí mạng" with "Phí giao dịch". (mạng = network, transaction = giao dịch)
Line 806: Name is different from nickname. Nickname = Biệt danh
Line 810: Same above. Prefer to keep the word MEMO in English.
Line 1346: I guess 50$4 is a typo, because $ and 4 are in the same place on the keyboard.

@omnat omnat linked an issue Mar 4, 2021 that may be closed by this pull request
2 tasks
@omnat omnat removed a link to an issue Mar 4, 2021
2 tasks
@omnat omnat removed the next release label Mar 8, 2021
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

@andrepimenta I am unable to test anything on the browser as the browser keeps refreshing = https://recordit.co/ksOZgOGALC Thus I couldn't look at signing nor approve

Issue 1: Deemed won't fix
Issue 2: Fixed 👍🏽
Issue 3: Fixed in the sense that it's better than what it was but it's not below

Screen Shot 2021-03-17 at 5 18 58 PM

Issue 4: Can't test this fix due to issue mentioned above

Issue 5: I can't test this because when I input the tether token address nothing happens; seen here https://recordit.co/30qAafrc1C (even if I have that address saved, no message appears

Token address test (Tether) = 0xdac17f958d2ee523a2206206994597c13d831ec7

Issue 6: Fixed 👍🏽
Issue 7: Deemed won't fix
Issue 8: Fixed 👍🏽
Issue 9: Fixed 👍🏽
Issue 10: Deemed won't fix
Issue 11: Fixed 👍🏽

Issue 12: Can't test this fix due to issue mentioned above

Issue 13: Fixed 👍🏽 Even though the browser keeps refreshing I was able to see this for a split second
Issue 14: Deemed won't fix

Issue 15: Can't test this fix due to issue mentioned above

Issue 16: Deemed won't fix
Issue 17: non blocker

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 17, 2021
@andrepimenta
Copy link
Member Author

We received some feedback about the vi-vn translations here: MetaMask/metamask-extension#10270 (comment)

Line 250: Missing "tiền" - Nhận tiền
Line 339: Missing "thông minh" (smart contract = hợp đồng thông minh)
Line 608: Replace "Phí gas hủy" with "Phí hủy gas"
Line 613: Replace "Phí mạng" with "Phí giao dịch". (mạng = network, transaction = giao dịch)
Line 806: Name is different from nickname. Nickname = Biệt danh
Line 810: Same above. Prefer to keep the word MEMO in English.
Line 1346: I guess 50$4 is a typo, because $ and 4 are in the same place on the keyboard.

Updated! Thanks!

@andrepimenta
Copy link
Member Author

@andrepimenta I am unable to test anything on the browser as the browser keeps refreshing = https://recordit.co/ksOZgOGALC Thus I couldn't look at signing nor approve

Issue 1: Deemed won't fix
Issue 2: Fixed 👍🏽
Issue 3: Fixed in the sense that it's better than what it was but it's not below

Screen Shot 2021-03-17 at 5 18 58 PM

Issue 4: Can't test this fix due to issue mentioned above

Issue 5: I can't test this because when I input the tether token address nothing happens; seen here https://recordit.co/30qAafrc1C (even if I have that address saved, no message appears

Token address test (Tether) = 0xdac17f958d2ee523a2206206994597c13d831ec7

Issue 6: Fixed 👍🏽
Issue 7: Deemed won't fix
Issue 8: Fixed 👍🏽
Issue 9: Fixed 👍🏽
Issue 10: Deemed won't fix
Issue 11: Fixed 👍🏽

Issue 12: Can't test this fix due to issue mentioned above

Issue 13: Fixed 👍🏽 Even though the browser keeps refreshing I was able to see this for a split second
Issue 14: Deemed won't fix

Issue 15: Can't test this fix due to issue mentioned above

Issue 16: Deemed won't fix
Issue 17: non blocker

Hey @ibrahimtaveras00 this was because the branch was out of date with develop. I have updated it and now should be fine. By the way this also brings the Spanish translations for swaps if you want to take a look.

Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Fixes look good now, and Spanish on swaps is fine 👍🏽
QA Passed

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Mar 20, 2021
@andrepimenta andrepimenta merged commit 88ee21f into develop Mar 22, 2021
@andrepimenta andrepimenta deleted the improvement/translations branch March 22, 2021 15:37
estebanmino pushed a commit that referenced this pull request Mar 22, 2021
* translations and update script

* Using new translation for es

* Update gitignore

* save old

* Remove RTL

* Fix qa issues

* Update es.json

* Update tests

* Update vi-vn.json
estebanmino pushed a commit that referenced this pull request Mar 24, 2021
* translations and update script

* Using new translation for es

* Update gitignore

* save old

* Remove RTL

* Fix qa issues

* Update es.json

* Update tests

* Update vi-vn.json
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* translations and update script

* Using new translation for es

* Update gitignore

* save old

* Remove RTL

* Fix qa issues

* Update es.json

* Update tests

* Update vi-vn.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) next release QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants