I18n reload (It's serious this time :) )#632
Conversation
As default i18n-node read all files within that directory and detect all given locales by their filenames.
mmckegg
left a comment
mmckegg
left a comment
There was a problem hiding this comment.
Looking pretty good @gmarcos87. Just a couple of requested changes (as below).
This does have me wondering (since this is the first time I've worked with internationalization), what is the plan for keeping the translations up to date when new features/strings are added?
Other question: what about languages where word order is different? Do we need to think about placeholders and formatting in the i18n strings?
| var config = ${JSON.stringify(config)} | ||
| var data = ${JSON.stringify(opts.data)} | ||
| var title = ${JSON.stringify(opts.title || 'Patchwork')} | ||
| var title = ${JSON.stringify(opts.title || "Patchwork" )} |
There was a problem hiding this comment.
Is there a specific reason for switching to use double quotes on some strings?
Patchwork code mostly conforms to JavaScript Standard Style (and I personally use this as a linter when working on it), and this only allows double quotes (") when you want to escape single ones.
| 'ev-change': (ev) => api.settings.sync.set({ | ||
| patchwork: {theme: ev.target.value} | ||
| h('h2', i18n('Theme')), | ||
| computed(currentTheme, currentTheme => { |
There was a problem hiding this comment.
When you rebase/merge with the latest master, you'll see that the theme picker has been replaced with a select dropdown box instead of the toggle buttons. I suggest you use that for the language picker too!
There was a problem hiding this comment.
Oh wait, I see that this has already merged in the change. But it has overridden it with the old code putting the original theme picker back in.
Can you update this to use the new code from df3a0af ?
|
|
||
| function time (date){ | ||
| return date | ||
| .replace(/from now/, i18n('form now')) |
There was a problem hiding this comment.
just noticed a typo on this string: should be "from now" not "form now"
There was a problem hiding this comment.
It also has matching typos in the string tables.
|
Okay, I've gone ahead and fixed the things I mentioned above and merged! Also I think I'm starting to understand how the i18n module works 😃 Looks like there is a lot we can do later to improve the quality of the translations including substitutions (word ordering) and pluralisation. |
|
I also never did an i18n implementation on such large projects. I encountered several challenges, many of them unresolved. I also noticed two more technical elements to solve, the order of the variables in the sentences and how to incorporate future translations that come from external modules. For the order of the variables it may be better to opt for another translation engine, i18nline has support for pluralization and multiple variables. I do not know if I could incorporate external translations, my knowledge of depject does not go so far. It would be interesting to find a solution, even giving the user the ability to change the translation for what seems most convenient and share it on your network. |
3 participants
This pull request tries to unify all the porpose that is around i18n and patchwork.
Basically it proposes: