Implement i18n for electron wrapper menu - Closes #768 #797

reyraa · 2017-09-29T15:06:02Z

Adds an i18n module to translate menu items
Creates Deutsch translation files
Uses osLocale to get the default locale of the browser
Uses electron-json-storage to store the last saved language
Communicates with react app to change the language consistently
Closes Setup i18n in Electron menus #768

ginacontrino · 2017-10-02T15:14:48Z

app/i18n.js

+const fs = require('fs');
+const storage = require('electron-json-storage'); // eslint-disable-line import/no-extraneous-dependencies
+const osLocale = require('os-locale'); // eslint-disable-line import/no-extraneous-dependencies
+


"os-locale" needs to be added to the package.json

slaweet · 2017-10-02T15:17:02Z

When opening npm run start for the first time with the new changes I get this error.

A JavaScript error occurred in the main process
Uncaught Exception:
TypeError: locale.substr is not a function
    at Localization.init (/Users/slaweet/git/nano-review/app/i18n.js:13:60)
    at i18n.getLng (/Users/slaweet/git/nano-review/app/main.js:38:10)
    at storage.get (/Users/slaweet/git/nano-review/app/i18n.js:28:7)
    at /Users/slaweet/git/nano-review/node_modules/async/dist/async.js:421:16
    at next (/Users/slaweet/git/nano-review/node_modules/async/dist/async.js:5302:29)
    at /Users/slaweet/git/nano-review/node_modules/async/dist/async.js:906:16
    at /Users/slaweet/git/nano-review/node_modules/electron-json-storage/lib/storage.js:136:14
    at nextTask (/Users/slaweet/git/nano-review/node_modules/async/dist/async.js:5297:14)
    at next (/Users/slaweet/git/nano-review/node_modules/async/dist/async.js:5304:9)
    at /Users/slaweet/git/nano-review/node_modules/async/dist/async.js:906:16

slaweet · 2017-10-04T08:06:22Z

app/locales/en/common.json

+  "Lisk Forum": "Lisk Forum",
+  "Report Issue...": "Report Issue...",
+  "What's New...": "What's New..."
+ }


I'd suggest to use other name then common.json (e.g. menu.json) for this file to make it easier to distinguish from src/locales/en/common.json in any localization platform we will choose.

…lient app

reyraa · 2017-10-13T09:25:21Z

For reviewing please note I've reset the history to fix merge conflicts easier.

slaweet · 2017-10-16T08:24:46Z

app/src/menu.js

      click(item, focusedWindow) {
        if (focusedWindow) {
          const options = {
            buttons: ['OK'],
            icon: `${__dirname}/assets/lisk.png`,
-            message: `Lisk Nano\nVersion ${app.getVersion()}\n${copyright}`,
+            message: `${i18n.t('Lisk Nano\nVersion')} ${app.getVersion()}\n${copyright}`,


I would prefer the \nnot to be part of the translation string, it might be confusing to the translators. On the other hand, the version should be part of the translations string.

${i18n.t('Lisk Nano')}\n${i18n.t('Version {{version}}', { version: app.getVersion() })}

slaweet · 2017-10-16T08:27:51Z

src/locales/en/common.json

  "{{count}} of entered delegate names could not be resolved:": "{{count}} of entered delegate names could not be resolved:",
-  "{{count}} of entered delegate names could not be resolved:_plural": "{{count}} of entered delegate names could not be resolved:"
+  "{{count}} of entered delegate names could not be resolved:_plural": ""


Please revert changes on lines 149-197. There was a problem in npm run i18n-scanner that caused those changes, but now it should keep the strings intact.

slaweet · 2017-10-16T08:32:36Z

app/src/main.js

+ * Sends an event to client application
+ * @param {String} locale - the 2 letter name of the local
+ */
+const sendDetectedLang = (locale) => {


sendDetectedLang looks very similar tosendUrlToRouter. The common pattern could be abstracted into a function, e.g.: sendEvent(eventName, data)

we don't need event in sendDetectedLang but we don't need it in sendUrlToRouter

slaweet · 2017-10-17T13:11:29Z

app/src/menu.js

-        click() {
-          electron.shell.openExternal('https://explorer.lisk.io');
+        {
+          type: i18n.t('separator'),


This should be just type: 'separator',. 'separator' is not a string in the UI.

slaweet · 2017-10-17T13:13:23Z

src/locales/en/common.json

-  "Last {{count}} days": "Last {{count}} day",
-  "Last {{count}} days_plural": "Last {{count}} days",
+  "Last {{count}} days": "Last {{count}} days",
+  "Last {{count}} days_plural": "",


These two lines should not be changed. It's my bad. I've sent you wrong common.json.

slaweet

Now it's all good. Thank you.

reyraa self-assigned this Sep 29, 2017

reyraa added the 🏗️ in progress label Sep 29, 2017

reyraa requested review from slaweet, alepop and yasharAyari September 29, 2017 15:06

reyraa added 👁️ pending review and removed 🏗️ in progress labels Oct 2, 2017

reyraa requested a review from ginacontrino October 2, 2017 13:21

ginacontrino reviewed Oct 2, 2017

View reviewed changes

slaweet reviewed Oct 4, 2017

View reviewed changes

slaweet changed the base branch from development to 1.2.0 October 9, 2017 09:17

yasharAyari added 🏗️ in progress and removed 👁️ pending review labels Oct 10, 2017

slaweet assigned yasharAyari and unassigned reyraa Oct 12, 2017

reyraa added 5 commits October 13, 2017 11:19

Add json storage to store selected language

7d32a9f

Create a utility to receive the default/stored language from electron

7515df4

Initiate the apiLocale listener

35ea643

Use a generic t function to translate menu labels

da90a94

Add listeners amd event emitters to comminicate the locale with the c…

ef091e2

…lient app

reyraa force-pushed the 768-electron-menu-i18n branch from b055619 to ef091e2 Compare October 13, 2017 09:22

slaweet reviewed Oct 16, 2017

View reviewed changes

yasharAyari added 2 commits October 16, 2017 11:59

Merge branch 'development' into '768-electron-menu-i18n'

ebf226e

Fix a bug in languages.js

95275d2

slaweet suggested changes Oct 16, 2017

View reviewed changes

yasharAyari added 5 commits October 17, 2017 13:28

Store user selected language in a json file

0f907ec

Update electron-json-file in package.json

e94a298

Merge branch 'development' into 768-electron-menu-i18n

8dcada9

Fix a bug in common.json

42e6e12

Fix a bug in menu.js

c3175f8

yasharAyari added 👁️ pending review and removed 🏗️ in progress labels Oct 17, 2017

yasharAyari added 2 commits October 17, 2017 16:24

Fix a bug in common.json and electron menu strings to it

e690f71

Run npm clean and copy-files before npm run eslint in genkinsfile

743539a

slaweet suggested changes Oct 17, 2017

View reviewed changes

yasharAyari added 2 commits October 17, 2017 17:15

Fix a bug in menu.json

925b1bd

Change some translation strings in common.json

e8d0f05

slaweet removed the request for review from yasharAyari October 17, 2017 13:49

slaweet changed the base branch from 1.2.0 to development October 17, 2017 13:53

slaweet changed the base branch from development to 1.2.0 October 17, 2017 13:53

Fix a bug in ipcLocale test file

c44b476

slaweet approved these changes Oct 18, 2017

View reviewed changes

yasharAyari merged commit 915920d into 1.2.0 Oct 18, 2017

yasharAyari deleted the 768-electron-menu-i18n branch October 18, 2017 06:48

slaweet added ✅ ready and removed 👁️ pending review labels Nov 3, 2017

karmacoma unassigned yasharAyari Nov 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement i18n for electron wrapper menu - Closes #768 #797

Implement i18n for electron wrapper menu - Closes #768 #797

reyraa commented Sep 29, 2017 •

edited

Loading

ginacontrino Oct 2, 2017

slaweet commented Oct 2, 2017

slaweet Oct 4, 2017

reyraa commented Oct 13, 2017

slaweet Oct 16, 2017

slaweet Oct 16, 2017

slaweet Oct 16, 2017

yasharAyari Oct 17, 2017

slaweet Oct 17, 2017

slaweet Oct 17, 2017

slaweet left a comment

Implement i18n for electron wrapper menu - Closes #768 #797

Implement i18n for electron wrapper menu - Closes #768 #797

Conversation

reyraa commented Sep 29, 2017 • edited Loading

Choose a reason for hiding this comment

slaweet commented Oct 2, 2017

Choose a reason for hiding this comment

reyraa commented Oct 13, 2017

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

slaweet left a comment

Choose a reason for hiding this comment

reyraa commented Sep 29, 2017 •

edited

Loading