Fixes: #35 support i18n#74
Conversation
There was a problem hiding this comment.
If this is an optional param, then it should be:
@param {object=} lang translated i18n strings
|
Is this just a PR to get the coding pattern approved, and then it will be applied to the other charts? |
|
Other than those issues, LGTM |
|
Yes I wanted to show the concept and if we think it makes sense I will apply this to other directives. |
|
I personally think we should use the strings themselves as keys and pass them into a translation service (or a directive that uses the service in the case of html). We could use angular-gettext for this. The user would just have to provide the I don't think we want to force the user to set every single string they want translated via a javascript object in the code that uses the angular-patternfly component. This quickly becomes cumbersome: $scope.lang = {
available: 'Disponible',
notAvailable: 'No disponible',
yes: 'si',
no: 'no'
};Instead we should encourage users to use .po files (pretty much the standard for i18n) and generate a javascript dictionary from that. This approach also has the added benefit of falling back to English in the case that a translation is not found instead of resolving to undefined or resulting in an error. Also, how would the implementation here support plurals? What about languages with more than one plural form? Using an existing translation library handles these issues for you. |
|
I am not completely against using existing translation but the parent application will need to tell us what language to use. We also need to make sure we can bootstrap multiple translation libraries (I have seen this be an issue in angular applications in the past where we actually had to merge translation files since config was done as a singleton). As far as .po files vs. json when it comes to client side libraries JSON seems to be the new norm: |
|
Also the beautiful part of not doing the actual translation is products can add languages based on their needs so we don't need to provide translations for every language out there and if a product wants to easily change a string value they can. If they want to use "Free" vs. "Available" vs. "Not Used" vs. "Whatever" they are free to customize things from a terminology perspective as they see fit. |
|
I too believe the approach here is too cumbersome. Could we use something like I18Next and require the application to set the locale? We could use I18Next for all of our strings and provide the appropriate translation files. If the application wanted to use some other library they could . Strings passed to our directives could be pre-translated or they could be keys. Granted it would be easier to not have to provide translations. I just think requiring applications to pass in lang objects for every string we want to present is a bit overbearing and if we change how we present strings or which strings we present then the applications would need to change. |
That's a good point. I'm not opposed to writing something ourselves but I do think that the translations should be done via a service/directive instead of with plain JS objects.
i18next supports gettext and I'd argue that angular-translate is a bad implementation. In addition to not supporting .po files it also requires made up IDs for the translation dictionary, and it uses filters instead of directives which is not as performant. We could just write a simple service that reads in a JSON file (to be provided by the consuming project via whatever way they want) and then looks up values in that file based on {
"es": {
"yes": "sí",
"no": "no"
},
"fr": {
"yes": "oui",
"no": "non"
}
}<p i18n>Yes</p>var yes = i18n('yes');Although we would still have to consider how to handle plurals... |
I agree, and I was not suggesting that we should do our own translations, just that we should create an easier way for the consumer to provide the translations. |
|
FYI this handles plurals https://docs.angularjs.org/api/ng/directive/ngPluralize |
Sort of, you still have to mark the string for translation and then override the localization rules, check out the first paragraph on that page:
|
|
Please let me know if anyone has an objection to using this. I started to write my own provider etc to wrap i18next and then I realized that was silly. I think i18next is our best bet so I was thinking of adding this as well: https://github.com/i18next/ng-i18next. I have this plugged in and working now but I wanted to see if anyone had any major push back before I went further. Pending everyone thinks that makes sense I will close this PR and open up a new one once I am ready. |
|
I agree with that approach. |
I agree since it allows for both JSON and gettext (looks like there is also grunt task for converting from gettext to i18next's JSON format). Also, it seems that ng-i18next provides a filter which we can inject for translations within our JS code. This approach looks good to me from a quick glance, thanks for researching. |
|
LGTM,
On Wed, Sep 9, 2015 at 12:58 PM, Eleni Rundle notifications@github.com
Dave Taylor |
4 participants
Handles basic string replacement. Assumes products translate strings and pass in via lang object.