search

genielabs / HomeGenie

HomeGenie, the programmable automation intelligence
https://homegenie.it
GNU General Public License v3.0
393 stars 155 forks source link

Z-Wave.Me Floor Thermostat widget, widget localization improvements. #62

Closed Bounz closed 9 years ago

Bounz commented 9 years ago

Hi Gene. I've made a custom widget for Z-Wave.Me floor thermostat. It allows to control this thermostat more effectively, using Economy Heat and Furnace modes. I also localized this widget, and during localization process added some functions to widget localization system:

Finally I added missed 'home_menu_events' localization key.

genemars commented 9 years ago

Hi Bounz, some things to fix before merging:

Btw... thanks for all =) you seem to know HG code very well

Bounz commented 9 years ago

Well, when I started to localize the widget I created 2 json files for en and ru and added there some keys found in html. I was surprised, that the widget itself became localized, but popup window with options - not. After investigation I found out that because of asynchronous widget's localization it happens after initializing the widget (first RenderView execution) where popups are initialized. Thats why when the program execution enters the ajax callback of HG.WebApp.Locales.LocalizeWidget function there is no html code with popup's description in module's div, as it is moved to the end of page to special overlay div for popup. To address this problem I've added additional block of code that localizes the popup https://github.com/genielabs/HomeGenie/pull/62/files#diff-3d545eb43aeaa2597c97efdaf98ec2a8R737.

After that I found a problem that in original thermostat widget when we switch thermostat modes, the text, representing the mode, is inserted in html directly, without getting localized (https://github.com/genielabs/HomeGenie/blob/master/BaseFiles/Common/html/pages/control/widgets/homegenie/generic/thermostat.json#L230). I decided to create HG.WebApp.Locales.GetWidgetLocaleString because we can't use HG.WebApp.Locales.GetLocaleString as the main localization engine uses different localization string source (or we can try to add widget's localization strings into the main locale store HG.WebApp.Data._CurrentLocale, but in that case we have to fill HG.WebApp.Data._DefaultLocale too and check for duplicate keys). As we don't have analog of HG.WebApp.Data._CurrentLocale for widget I added Locale data attribute, it is used in HG.WebApp.Locales.GetWidgetLocaleString (https://github.com/Bounz/HomeGenie/blob/master/BaseFiles/Common/html/js/api/homegenie.webapp.js#L773). Using this function gives the ability to write the code like this:

var localizedDisplayMode = HG.WebApp.Locales.GetWidgetLocaleString(widget, displayMode);
if(localizedDisplayMode != undefined)
    widget.find('[data-ui-field=mode_value]').html(localizedDisplayMode);
else
    widget.find('[data-ui-field=mode_value]').html(displayMode);

After that almost everything worked fine except one little thing: that little string with current thermostat mode was not localized upon widget initialization, because widget's locale haven't been loaded yet. There were 2 possible solutions:

  1. Add four <span data-locale-id=''> for each thermostat mode and then display or hide them depending of current thermostat mode;
  2. Add the ability to localize some text in HG.WebApp.Locales.LocalizeWidget function.

I decided to use the second solution and created data-localizable attribute that shows HG.WebApp.Locales.LocalizeWidget function that content of the element with this attribute should be localized:

$(container).find('[data-localizable]').each(function(index){
    var stringid = $(this).text();
    var text = findLocaleString(locale, stringid);
    if (text != null) {                    
        $(this).text(text);                    
    }
});

One last note about findLocaleString function naming. I named it so because it's used only inside the module to fix code duplication and is not exposed to the external scope.

Bounz commented 9 years ago

Gene, I merged your changes, so now pull request can be automatically merged. Aslo I renamed findLocaleString to HG.WebApp.Locales.FindLocaleString

genemars commented 9 years ago

good solution about the data-localizable trick. A couple of more remarks and we are done:

Bounz commented 9 years ago

Fixed. What about findLocaleString - I've left it full named HG.WebApp.Locales.FindLocaleString because now it's used in two different functions and I can't place it inside the calling function.