Refactor extension for translation

[thomaschampagne]

[praveentiru]

@thomaschampagne. After a lot of pain and suffering, I have finally basic code in place to run translation. In the screenshot below, you are seeing ####### in the menu because they are translation strings for en-GB. Hopefully, things would be easier now.

@thomaschampagne I will push changes to my fork. You can look at them there if you are interested. Would love feedback.

[praveentiru]

Step 1: Enable translation for messages added to view through Javascript

• [x] Implement PoC
• [x] Investigate how to effectively load translation libraries.
• [x] Implement String substituion - Description
• [x] Implement Message Inheritance - Description
• [x] Develop Helper method to run translations
• [x] Enable translation across all js pages

Step 2: Implement Number formatting (We live different worlds: 1,00,000 or 100,000 or 100.000)

• [ ] Develop Helper method for number formatting
• [ ] Implement changes to js to ensure proper formatting of numbers

Step 3: Implement Date formatting (10/12/2015 can be 12^th October, 2015 or, 10^th December, 2015)

• [ ] Develop Helper method for date formatting
• [ ] Implement changes to js to ensure proper formatting of numbers

Step 4: Migrate to globalize-webpack-plugin

• [x] If it simplifies the set-up process of Globalize
• [x] If yes, migrate creation of Globalize instance in js/StravistiX.js

Step 5: What to do about angular pages?

This clearly explains why basic angular i18n is not good enough. Need to see if using something like angular-translate is better approach rather than re-using jQuery Globalize.

[praveentiru]

Translation enabled for JS views under common section of Stravistix.js. Two minor changes pending due to absence of app resources in modifiers

• SegmentRankPercentageModifier
• DisplayFlyByFeedModifier

[praveentiru]

@thomaschampagne documenting challenge here so that all have access

I ran into trouble when I saw worker thread for activity analysis was failing to execute. I have finally isolated the source of issue. It is very strange.

The following code triggers the problem - Code here

$.getJSON(this.appResources_.cldrBase, function (data) {
   // #10 - Loading CLDR data needed for globalize to work
   Globalize.load(data);
   // #10 - Loading translation libraries from locales folder
   var loadCount = 0;
   for (var i = 0; i < locArray.length; i++) {
        $.getJSON(locArray[i], function (data) {
        Globalize.loadMessages(data);
        loadCount++;
        if (loadCount === locArray.length) {
               var currentLocale = window.navigator.language || window.navigator.userLanguage;
               // We should create a global instance for translation only after loading all messages
               appRes.globalizeInstance = Globalize(currentLocale);
                // #10 - Moving all the UI generation into function to ensure globalize instance
                // is properly built as we need it to translate
                strav.handleApp_();
            }
        });
    }
});

strav.handleApp_ builds the UI of the extension. I have structured the code so that, it is triggered only after all translation libraries are loaded properly which would enable me to translate the UI as it is getting built. This method of building UI is triggering failure of worker in ActivityProcessor.js@Line 110. Web worker fails due to inability to clone the object.

This problem does not happen handleApp_ call is moved out of callback method. Advice on how to resolve this would be great.

[praveentiru]

Found the culprit. It is appRes.globalizeInstance.

One of the input variables for Web Worker is appResources_. I have added globalizeInstance to appResources_ to enable access to translator across the app. Web Worker is unable to clone this object and it fails. The reason it works when we move out of scope of callback is because web worker starts before instance of globalize could be instantiated.

Now, I will look for solution :)

[praveentiru]

Fix to solve this issue.

Change 1@AcitivityProcessor.js#L113 Change appResources: this.appResources, to extnId: this.appResources.extensionId,

Change 2@ComputeAnalysisWorker.js#L31 Change self.importRequiredLibraries(self.required, mainThreadEvent.data.appResources.extensionId); to self.importRequiredLibraries(self.required, mainThreadEvent.data.extnId);

Web worker does not need everything from appresources only extension id to load javascripts. :)

[praveentiru]

Just uploaded a large commit - Here.

This completes creation of default message library for summary of extended statistics (Not segment only activity) and detailed running stats. Sample screenshot with locale set to en-GB. (Using ###### as translation strings for testing).

[thomaschampagne]

@praveentiru Very nice ! Great job !

I merged your code via pull request into a feature/10 on my repo. I try to support french at the moment. I did't fell into fallback language (eg. en-US) at this time i getting this:

globalize.js:105 Uncaught Error: E_MISSING_MESSAGE_BUNDLE: Missing message bundle for locale `fr`.
createError @   globalize.js:105
validate    @   globalize.js:182
validateMessageBundle   @   message.js:1873
Globalize.messageFormatter.Globalize.messageFormatter   @   message.js:2031
Globalize.formatMessage.Globalize.formatMessage @   message.js:2068
Helper.translateDOMNode @   Helper.js:206
modify  @   MenuModifier.js:55
handleMenu_ @   StravistiX.js:296
handleApp_  @   StravistiX.js:90
(anonymous function)    @   StravistiX.js:77
c   @   strava-head-15cde5e….js:24
fireWith    @   strava-head-15cde5e….js:24
n   @   strava-head-15cde5e….js:25
(anonymous function)    @   strava-head-15cde5e….js:25

var currentLocale = window.navigator.language || window.navigator.userLanguage;

This return 'fr' on my browser

Currently discovering and learning from your job ;)

[praveentiru]

Message inheritance is properly implemented. So, you need not copy all the message keys to fr.json file. Just keep the one you have translated and remove others. Translation will fall back to root.json

I am pushing a fix where extension fails when the file is not supported. It should not fail like it id.

[thomaschampagne]

@praveentiru Indeed, i didn't need to override all keys in french if no translations root file should do the work :) Just copy past file :) Hihi ^^^

FYI Here is locales list from language dropdown list of strava:

en-GB
de-DE
en-US
es-ES
es-41
fr-FR
it-IT
nl-NL
pt-PT
pt-BR
ru-RU
ko-KR
zh-CN
ja-JP
zh-TW

I mean that list:

And chrome locale list is: https://developer.chrome.com/webstore/i18n#localeTable

Locales value you got if you type this in a chrome console:

window.navigator.language

Thanks !!!!!!!

[praveentiru]

Actually, you do not have to copy paste the root.json. In your fr.json remove everything except for code below:

{
    "fr": {
        "menu": {
            "common_settings": "Paramètres principaux",
            "health_settings": "Paramètres de santé"
        }
    }
}

You will see that rest of UI will reflect the strings in root.json. This way it is much more efficient and translator can work on adding one section at a time. :)

Now, you need to search for volunteers to support all those languages after release. :)

[thomaschampagne]

@praveentiru

You will see that rest of UI will reflect the strings in root.json. This way it is much more efficient and translator can work on adding one section at a time. :)

Sure !

Now, you need to search for volunteers to support all those languages after release. :)

Should not be a problem ;)

[thomaschampagne]

@praveentiru

• I'm getting this error with last PR. I'm searching for understand and fixing now

globalize.js:105 Uncaught Error: E_MISSING_MESSAGE_BUNDLE: Missing message bundle for locale `en-US`.

• We should cover only locales supported by strava:

  en-GB, de-DE, en-US, es-ES, es-41, fr-FR, it-IT, nl-NL, pt-PT, pt-BR, ru-RU, ko-KR, zh-CN, ja-JP, zh-TW

Don't need to translate a language like Greek not support by strava in their web interface

Then have:

supportedLocales: [ 'root', 'en-GB', 'de-DE', 'en-US', 'es-ES', 'es-41', 'fr-FR', 'it-IT', 'nl-NL', 'pt-PT', 'pt-BR', 'ru-RU', 'ko-KR', 'zh-CN', 'ja-JP', 'zh-TW' ];

Do you confirm?

Note: strava store the locale used by user into a cookie with key named ui_language

• About date formatting you listed i introduced moment.js in feature/218(-poc)|213(-poc) Lot of new things mixed once :) Maybe delay this in that branch.. i can cover it easily localized date formats later with moment.js (already managed in a pro project thought 10 languages ;)).

[praveentiru]

@thomaschampagne I just tested with my feature/pty-#10 branch code which was my last Pull request and it is working fine. Not able to re-produce. That error should not occur because en-US.json is present. Can you try getting callstack when you are getting the error? Also, what was the chrome locale when it happened.

I agree that we should support only the locales supported by Strava and pull the locale used by strava. If strava supports dynamically changing language then, I need to see how my mechanism works.

I will focus on textual translation for now. Will look into date translation if needed at a later date.

[praveentiru]

Some screenshots after the latest commit. As you can see, labels of chartjs need to enabled for translation. Otherwise all text elements have been addressed.

[praveentiru]

@thomaschampagne PoC for enabling translation in angular app used for settings up and running. YAAY!!! Look at the beautiful hashes. :) :)

Re-using globalize itself. Created a new service by name TranslationService to support translation in angular app.

[praveentiru]

@thomaschampagne We need to discuss if the current progress is enough to add it to release. It would give a good baseline to look for volunteers to start translating. In the next release, we can add the missing elements.

Your thoughts.