tidepool-org / blip

Blip is the internal name for Tidepool for Web, a tool for seeing diabetes data in one place.
https://tidepool.org/products/tidepool/
BSD 2-Clause "Simplified" License
106 stars 54 forks source link

Localize the interface #449

Open mgu opened 6 years ago

mgu commented 6 years ago

It would be great if the strings were translatable.

coyotte508 commented 6 years ago

I've worked towards that in my own fork.

The localization proposal is here. My fork also acts as a proof of concept where the navbar is translated to French, using the process described in the proposal. In order for the translations to show, you need to select French in Account Settings.

I welcome any feedback, to see if the way it is described is acceptable, and if I should push this further, towards adding language settings in the user interface.

franck-fourel commented 6 years ago

@pazaan @cbwebdevelopment have you had a chance to have a look at what @coyotte508 (aka Eliott) proposed for multi-language?

pazaan commented 6 years ago

This looks good to me. Did you investigate something like react-i18next, considering that it also supports date localisation and plurals? It looks like it might be easy to do this with marker: 'react-i18next.t' if we want to add it later... @krystophv - do you have any comments on this approach?

coyotte508 commented 6 years ago

@pazaan The current proposal was designed with minimal changes in order to be more readily accepted by the tidepool team, using react-i18next would introduce more changes:

Dates can be handled easily in my opinion, as blip already uses moment for date display. The only thing to do would be to set the language of moment at the same time as setting the language of the application.

Regarding plurals, I don't see it becoming an issue. I didn't see any pluralization dependent on a variable number in the code. If there is a specific example in the code, I can look at it. Depending on how it's done, it can be gracefully handled by the current proposal.

react-i18next does offer more utilities, so if you are okay with the additional changes entailed, I can switch to using it.

By the way, I updated code in my fork to introduce a language setting in the user's account settings. I was pleasantly surprised to see that no change to server code was necessary to preserve the language setting across logins. Now the language is set to English by default, and can be changed to French in the Account settings.

krystophv commented 6 years ago

So I think we were looking toward using the likes of react-i18next as it supports the "cloud" based localization services like Locize. We would like to (at least eventually) avoid storing the translation data in the codebase directly so we could accept and alter translation files without involving commit, Q/A and release cycles that currently accompany any release. This allows us to have non-engineer persons contributing to the translation/localization of the application relatively easily. In general, I think we'd prefer going with a more robust system up front as opposed to trying to migrate to it when we find it necessary. I don't think that wrapping things in the HOC represents a great challenge from the perspective of being able to merge in the changes relatively easily and I think that the recommendation to use key names is good in the event that the English string changes, the reference to that key doesn't necessarily need to.

We also tend to be relatively stringent about adding new dependencies to the codebase and i18next/react-i18next seems to be much more actively developed and depended upon than i18n-extract (though admittedly I haven't researched either in great detail).

@coyotte508 Would you be amenable to using react-i18next?

coyotte508 commented 6 years ago

@krystophv Yes, no problem.

Regarding the possibility of the English string changing, when not using key names, a possibility is adding an English translation for the string instead of changing the reference string in the code.

I can begin working on this, first adding the HOC and surrounding all translatable strings by t (and resolving any difficulties with react-i18next) as a first step, and then replace all translatable strings by key names if that is the way to go.