add i18n support#23
Conversation
|
This is looking good. I made some simple comments and just curious what your thoughts are on them. |
|
@DuffMck , thoughts on my comments? |
|
Sorry, I haven't find your comments... can you give me the link to the file/s where they are? |
vincentjames501
left a comment
vincentjames501
left a comment
There was a problem hiding this comment.
Looks like I never started the review. My bad.
| 'THU': 'Thursday', | ||
| 'FRI': 'Friday', | ||
| 'SAT': 'Saturday' | ||
| 'en': { |
There was a problem hiding this comment.
I think rather than these being localized individually, these should just be the lookup keys in the provided translations map so that way there is only one place people have to go to add translations.
There was a problem hiding this comment.
I agree, next week I hope to have enough time change this one
|
|
||
| angular.module('angular-cron-gen', []).service('cronGenService', CronGenService).component('cronGenTimeSelect', { | ||
| angular.module('angular-cron-gen', ['pascalprecht.translate']).config(["$translateProvider", function ($translateProvider) { | ||
| $translateProvider.translations('en', { |
There was a problem hiding this comment.
I haven't used this library (we rolled our own i18n lib internally before something like this existed). Should we namespace these localization keys? Can this blow away someone else's localizations? Does this merge into their existing english/Italian translations (hopefully it doesn't blow it away)?
There was a problem hiding this comment.
I think that it's possible, but I must check. Probably I should find some more smart way to passing translations I will make some tries!
|
|
||
| this.parsedOptions = this.mergeDefaultOptions(this.options); | ||
|
|
||
| $translate.use(this.parsedOptions.language); |
There was a problem hiding this comment.
Since they are going to have to require angular translate in their project, would it make sense to just remove the language setting from here and just use whatever has been previously set and if someone wants to configure it they just have to call that prior to component initialization? Does it choose a reasonable default if use isn't called (again, not an expert in this library)?
There was a problem hiding this comment.
it has sense, the solution of the next point probably solve this one too.
2 participants
Hi,
I've introduced support for internationalization and Italian translation, if you like it you can merge my code!
Inside the option object now you can find the field "language" where users can only choose it|en (default) right now. More languages can maybe be added by someone else.
By,
David