feat: Relates to #402. Internationalisation. Base Support #452

ltfschoen · 2019-02-26T01:16:48Z

Update to latest React.js to support react-i18n library
Added support for multiple locales. Able to change between different languages (separate PRs shall provide other languages). Other languages may be added by following the instructions in the Readme
Translate Fether menus and Fether window contents
Integrate into menu (i.e. preferences/settings) or try to automatically detect the language
Check that it works consistently across Windows and Linux, as well as macOS
Move some "Internationalisation" sections from README.md into the Fether FAQ of the Parity Wiki. See 88c9b18#diff-04c6e90faac2675aa89e2176d2eec7d8R187
- Refer to Fether FAQ PR https://github.com/paritytech/wiki/pull/327
Convert all text in fether-electron i18n config file into i18n. See 88c9b18#diff-e71da345e8fcc8df02de6ee26e679513R43 Not required as discussed with Tbaut
Convert all text in fether-react i18n config file into i18n. See 88c9b18#diff-473a1d2ae1b77ea2a159e91cf293fe6cR50 Not required as discussed with Tbaut
Convert all text in scripts/fetch-latest-parity.js into i18n. Not required as discussed with Tbaut

Notes:

PRs for specific languages other than English (default) may be provided by contributors using the following template, which shows what changes are required to add support for another language (i.e. German) feat: Relates to #402. Internationalisation. German Language Support #464
T&Cs will remain in English. This is explained to users in the Readme.

ltfschoen · 2019-02-26T20:36:34Z

I've pushed commits so we can toggle between the English and German languages. However I

Could someone fluent in German please review packages/fether-react/src/i18n/locales/de.json to check that the German translation is correct (since I've just used Google Translate)?

Where in the UI will we allow users to toggle between different supported languages?
If we incorporate it into the Fether context menu would it go under:

Preferences
Languages
> English
> German

How will we approach translating the legal Terms of Use in /Users/scon/code/src/paritytech/fether/packages/fether-react/src/Onboarding/termsAndConditions.md into different supported languages?

Tbaut · 2019-02-27T13:40:02Z

Although it's a feature that definitely brings a lot of value, I think it's a little early to invest too much time on it. Bounties or challenges (win a parity t-shirt and goodies for each language added) can help us bring a lot of languages. But we need mainnet and more adoption for that. The T&Cs is a tricky part, we might need to keep them in english for now.

… menu

…ctions

Tbaut

Impressive!
We forgot small things like transaction speed High and Low in /fether-react/src/Send/TxForm/TxForm.js
The tooltip when hovering the "amount" field on the TxForm is still in english (it reads "Please fill out this field"). This comes from the chrome engine, we should disable it by adding novalidate="novalidate" to the <form>.

I've tested a lot but probably not everything yet.

amaury1093

Nice! Works well, I have some small comments, mainly questions, as I'm not super familiar with this lib.

amaury1093 · 2019-03-18T14:32:07Z

packages/fether-electron/src/main/app/menu/i18n/index.js

+  .use(Backend)
+  .init({
+    debug: true,
+    defaultNS: 'ns1',


I suppose ns1 stands for namespace 1. Should we rename it to fether-electron here and fether-react in the react folder? Or something else shorter, but more relevant

Yes, agreed

I've renamed it to the package name from package.json. And if a fether-react component uses a child component from fether-ui then it uses the fether-react namespace

amaury1093 · 2019-03-18T14:33:56Z

packages/fether-electron/src/main/app/menu/i18n/index.js

+});
+
+i18next.on('initialized', options => {
+  pino.info('i18n backend: Detected initialisation of i18n');


I think some of these could be pino.debug.

amaury1093 · 2019-03-18T14:35:08Z

packages/fether-react/src/utils/withTokens.js

@@ -1,3 +1,8 @@
+// Copyright 2015-2018 Parity Technologies (UK) Ltd.


2019 (multiple instances)

amaury1093 · 2019-03-18T17:10:20Z

packages/fether-react/src/i18n/index.js

+// https://react.i18next.com/getting-started
+import i18next from 'i18next';
+// // https://github.com/i18next/i18next-browser-languageDetector
+import LanguageDetector from 'i18next-browser-languagedetector';


Does this work in Electron?

Also, why do we need it? Imo: language should be set/detected by electron, then electron emits the IPC message set-language, and fether-react just follows what electron says.

Yes, you're correct. I just removed it and it works, so we don't need it

amaury1093 · 2019-03-18T17:12:05Z

packages/fether-react/src/i18n/index.js

+    },
+    saveMissing: true
+  })
+  .then(() => console.log('i18n frontend: success'))


Use debug, no console.logs.

amaury1093 · 2019-03-18T17:16:10Z

packages/fether-react/src/App/App.js

+      .getCurrentWindow()
+      .webContents.addListener('set-language', newLanguage => {
+        i18n.changeLanguage(newLanguage);
+        store.set(LANG_LS_KEY, newLanguage);


If I understood correctly, this setting is already saved in electron-settings. Isn't saving it in localStorage redundant?

When I was deciding where to save the user's language setting I found I only had access to electron-settings in the fether-electron package, but not in fether-react (and I don't think we'd want the front-end to have access to the file system anyway). I need to store the language in localStorage too since I'm using i18n in the front-end for the Fether contents, and when it initialises it sets the language to lng in /Users/scon/code/src/paritytech/fether/packages/fether-electron/src/main/app/menu/i18n/index.js based on the language last saved in localStorage or English as a fallback, and when the App.js component finishes mounting it checks if the language saved in localStorage differs from the currently set i18n language, and if so it retrieves the latest localStorage language value and sets that to the i18n language (see /Users/scon/code/src/paritytech/fether/packages/fether-react/src/App/App.js).

fether-electron has the Fether menu language config.

i18n config initialises to the language stored in electron-settings under key fether-language, or uses fallback default language of English (see fether-electron/src/main/app/menu/i18n/index.js)

When it's setting up the Fether menu it determines which language checkbox to tick based on the value of the key fether-language stored in electron-settings. See /Users/scon/code/src/paritytech/fether/packages/fether-electron/src/main/app/menu/template/index.js

If the user clicks one of the languages in the Fether menu, then it triggers the click handler, which changes the fether-electron i18n language, sets the fether-language key in electron-settings to the language that was clicked, updates the Fether menu, and emits set-language', which the fether-react event listener handles by changing the fether-react i18n language, sets the fether-language` key in localStorage to the language that was clicked, and reloads the Fether window. See /Users/scon/code/src/paritytech/fether/packages/fether-react/src/App/App.js

fether-react has the i18n Fether contents language config

i18n config initialises to the language stored in localStorage under key fether-language, or uses fallback default language of English (see /Users/scon/code/src/paritytech/fether/packages/fether-react/src/i18n/index.js)

i18n config initialises to the language stored in localStorage

My idea is:

fether-electron stores user's language in electron-settings - we agree on that

On startup (i.e. componendDidMount inside App.js), react reads from IPC from electron (i.e. using electron.remote) and sets the language accordingly

Everytime react receives a set-language IPC message, it changes the language accordingly

There's really no need for localStorage

But the problem is that after the user changes the language in the UI, first the click () handler for the language they chose changes the language in fether-electron's i18n config and updates the Fether menu, which successfully changes the language shown in the Fether menu (but not the Fether window contents), but then it emits the 'set-language' event which triggers the addListener('set-language' block in componendDidMount of App.js where it changes the i18n language in the front-end to match the new language stored in electron-settings. But then in order for the Fether window contents to change to the new language that was emitted, we need to reload the Fether window, which is why we end the block with electron.remote.getCurrentWindow().webContents.reload();, but when it reloads, it also reloads the front-end i18n configuration back to its defaults.

I've created a separate branch where I've added two additional commits, one that merges the German language branch into this branch (so you can see that it works switching between languages, noting that I've actually added the German language support as a separate PR), and a second commit where i've tried get your idea to work unsuccessfully https://github.com/paritytech/fether/compare/luke-402-i18n-temp?expand=1

… Low tx speed

…debug

ltfschoen · 2019-03-19T03:05:53Z

Impressive!
We forgot small things like transaction speed High and Low in /fether-react/src/Send/TxForm/TxForm.js
The tooltip when hovering the "amount" field on the TxForm is still in english (it reads "Please fill out this field"). This comes from the chrome engine, we should disable it by adding novalidate="novalidate" to the <form>.

I've tested a lot but probably not everything yet.

Thanks, yes I found that I had to add noValidate to prevent that tooltip in a few other places.

ltfschoen · 2019-03-19T03:06:17Z

@Tbaut @amaurymartiny I've pushed commits that I believe address your review comments

Tbaut

👍 LGTM

…eload via bridge

ltfschoen · 2019-04-11T15:46:01Z

@amaurymartiny @Tbaut i've updated this PR ready for review incorporating all the latest changes in master and resolving merge conflicts

Merge latest from master
Update package.json pre-commit hook to not include .ts files
Update window.bridge to expose add and remove listener for current window web contents, and reload
(without exposing remote)
Convert "New version available" "... was released!" text to i18n
Removed the "Internationalisation" section from the README.md, as this will go in the Fether FAQ

ltfschoen · 2019-04-11T17:43:00Z

i've created a Fether FAQ PR https://github.com/paritytech/wiki/pull/327 that explains:

How to switch between languages that Fether supports?
How to add support for a new language to Fether?

amaury1093

Looks good!

@ltfschoen I looked at your commit, but I'm still convinced we don't need to store language in localStorage. Anyways, this PR is working, my proposal is just optimization after-all, so let's merge it.

…)" This reverts commit 649d6bf.

ltfschoen added 2 commits February 26, 2019 00:06

chore: Update to latest React 16.8.3 to requirements of react-i18next

e7a81dc

feat: Scaffold basic translation with English and German

27c6e06

ltfschoen added the A3-inprogress label Feb 26, 2019

feat: Relates to #402. German translation fully working

12142e2

ltfschoen added the Z1-question label Feb 26, 2019

fix: Switch back to English by default

fe1efc3

ltfschoen added A0-pleasereview and removed A3-inprogress labels Feb 27, 2019

ltfschoen added 3 commits February 27, 2019 17:32

Merge branch 'master' into luke-402-i18n

33c66f3

fix: Allow user to switch between languages in preferences of context…

55b4270

… menu

feat: Translate the context menus

9574abc

ltfschoen added A3-inprogress and removed A0-pleasereview labels Feb 28, 2019

ltfschoen added 4 commits March 17, 2019 21:59

refactor: Remove German language. Add as separate PR

f3b6124

refactor: Remove blank line

060ac5f

docs: Update Readme with Internationalisation Add New language instru…

013470b

…ctions

docs: Update Readme with Known Issues and Usage instructions

674219c

ltfschoen changed the title ~~feat: Relates to #402. Internationalisation~~ feat: Relates to #402. Internationalisation. Base Support Mar 17, 2019

ltfschoen added A0-pleasereview and removed A3-inprogress Z1-question labels Mar 17, 2019

Merge branch 'master' into luke-402-i18n

766a842

Tbaut reviewed Mar 18, 2019

View reviewed changes

amaury1093 reviewed Mar 18, 2019

View reviewed changes

ltfschoen added 4 commits March 19, 2019 00:28

review-fix: Disabled tooltip Please fill out this field. Add High and…

accff7d

… Low tx speed

review-fix: Update license headers to be 2019 instead of 2018

824cff5

review-fix: Change ns1 to fether-electron and fether-react. Use pino.…

7a38c24

…debug

fix: Add missing i18n conversion for macOS Edit menu

cabd0b5

ltfschoen added 2 commits March 19, 2019 03:21

review-fix: Remove unused i18next browser languagedetector dependency

781eed4

Merge branch 'master' into luke-402-i18n

7904485

Tbaut added A5-grumble and removed A0-pleasereview labels Mar 19, 2019

Tbaut approved these changes Mar 19, 2019

View reviewed changes

merge latest from master and fix conflicts

88c9b18

ltfschoen mentioned this pull request Mar 26, 2019

feat: Relates to #402. Internationalisation. German Language Support #464

Merged

13 tasks

ltfschoen added the A3-inprogress label Mar 26, 2019

ltfschoen and others added 4 commits April 11, 2019 16:47

merge latest master and fix conflicts. TODO do not expose remote

f8907fc

fix: Do not expose remote. Only expose add and remove listener, and r…

3a3baee

…eload via bridge

feat: Convert new release available text to i18n

5aacf6a

Merge branch 'master' into luke-402-i18n

cfbb472

ltfschoen added A0-pleasereview and removed A3-inprogress labels Apr 11, 2019

Tbaut requested a review from amaury1093 April 12, 2019 09:57

amaury1093 approved these changes Apr 12, 2019

View reviewed changes

amaury1093 added A8-looksgood and removed A0-pleasereview A5-grumble labels Apr 12, 2019

amaury1093 mentioned this pull request Apr 12, 2019

Don't store language preferences both in electron-settings and localStorage #509

Closed

Tbaut merged commit 649d6bf into master Apr 12, 2019

Tbaut added a commit that referenced this pull request Apr 12, 2019

Revert "feat: Relates to #402. Internationalisation. Base Support (#452…

037d166

…)" This reverts commit 649d6bf.

amaury1093 mentioned this pull request May 6, 2019

fix: Use preload script as proxy for main-renderer communication #518

Merged

ltfschoen mentioned this pull request May 7, 2019

Internationalisation support #402

Closed

Tbaut deleted the luke-402-i18n branch May 7, 2019 19:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Relates to #402. Internationalisation. Base Support #452

feat: Relates to #402. Internationalisation. Base Support #452

ltfschoen commented Feb 26, 2019 •

edited

Loading

ltfschoen commented Feb 26, 2019

Tbaut commented Feb 27, 2019 •

edited

Loading

Tbaut left a comment

amaury1093 left a comment

amaury1093 Mar 18, 2019

ltfschoen Mar 19, 2019

ltfschoen Mar 19, 2019

amaury1093 Mar 18, 2019

amaury1093 Mar 18, 2019

amaury1093 Mar 18, 2019 •

edited

Loading

ltfschoen Mar 19, 2019

amaury1093 Mar 18, 2019

amaury1093 Mar 18, 2019 •

edited by ltfschoen

Loading

ltfschoen Mar 19, 2019 •

edited

Loading

amaury1093 Mar 19, 2019

ltfschoen Mar 20, 2019

ltfschoen Mar 20, 2019 •

edited

Loading

ltfschoen commented Mar 19, 2019

ltfschoen commented Mar 19, 2019

Tbaut left a comment

ltfschoen commented Apr 11, 2019 •

edited

Loading

ltfschoen commented Apr 11, 2019

amaury1093 left a comment

		@@ -1,3 +1,8 @@
		// Copyright 2015-2018 Parity Technologies (UK) Ltd.

feat: Relates to #402. Internationalisation. Base Support #452

feat: Relates to #402. Internationalisation. Base Support #452

Conversation

ltfschoen commented Feb 26, 2019 • edited Loading

ltfschoen commented Feb 26, 2019

Tbaut commented Feb 27, 2019 • edited Loading

Tbaut left a comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaury1093 Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaury1093 Mar 18, 2019 • edited by ltfschoen Loading

Choose a reason for hiding this comment

ltfschoen Mar 19, 2019 • edited Loading

Choose a reason for hiding this comment

fether-electron has the Fether menu language config.

fether-react has the i18n Fether contents language config

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltfschoen Mar 20, 2019 • edited Loading

Choose a reason for hiding this comment

ltfschoen commented Mar 19, 2019

ltfschoen commented Mar 19, 2019

Tbaut left a comment

Choose a reason for hiding this comment

ltfschoen commented Apr 11, 2019 • edited Loading

ltfschoen commented Apr 11, 2019

amaury1093 left a comment

Choose a reason for hiding this comment

ltfschoen commented Feb 26, 2019 •

edited

Loading

Tbaut commented Feb 27, 2019 •

edited

Loading

amaury1093 Mar 18, 2019 •

edited

Loading

amaury1093 Mar 18, 2019 •

edited by ltfschoen

Loading

ltfschoen Mar 19, 2019 •

edited

Loading

ltfschoen Mar 20, 2019 •

edited

Loading

ltfschoen commented Apr 11, 2019 •

edited

Loading