Closed srl295 closed 6 years ago
Looking pretty good. Issue description updated, as well as my checklist. Just a couple of open items left.
@Bargs thanks!
@Bargs WRT "registering the core plugin translations" in delivery section of phase 1 of #6515, this could be updated as follows:
registerTranslations
API and make them available to the build packagenpm start
). Hook is added to the 'optimize' module to call the registerTranslations
API.Additionally, maybe the "Blockers" section can now be cleaned up?
@Bargs WRT Jade template verification in delivery section of phase 1 of #6515, this could be updated as follows:
translate(<key>)
function in the Jade template. A tool can then be used to find such pattern and extract the keys to file@hickeyma lgtm - description updated.
@hickeyma so I was thinking more about the core plugin registration problem and I'm starting to question whether we even need to build this intermediate json bundle per language. Due to the simple, flat translation file structure we've chosen we could easily build a single unified JSON object from multiple file streams on the fly. The i18n plugin would only need to keep track of the translation file paths per language. This would simplify a number of things:
init
hook.What do you think?
@Bargs good feedback, thanks.
Are you saying that we now just register the translation files names per plugin when the plugin is being initialized? Does this mean the plugins still have to register with the i18n plugin but not during the install phase of the plugin? That would definitely simplify the infrastructure hooks difficulty. It probably doesn't matter then if you register just the filename or the actual file?
(I'm coming off of NodeSummit, just getting to some more detailed review.) @Bargs this makes some sense.
@hickeyma yeah, basically the i18n plugin just needs a reference to each translation file and knowledge of which language it contains. I think each plugin would still be responsible for explicitly registering its translation files with the i18n plugin during init time, just like how plugins currently register their routes with Hapi at init time.
@Matt and @hickeyma , this will simplify the approach for sure. And removes the install dependency. I do like this approach
I'm not clear if the initial phases are providing a translation at the server level and all users see only that one language, and then in Phase 3 it uses the browser HTTP header “accept-language” to translate per users browser language? Can we add notes to the phase descriptions to make it clear what the functionality is?
We might also want to start a discussion about which phases might be release candidates. It can happen later, but I wouldn't want a bunch of contributors to have an expectation about getting a release with some phase functionality and Elastic deciding not to release until some other phase. For example, translations for the whole server and not on a per-user basis.
a translation at the server level and all users see only that one language
I don't think this "one language" step is part of the plan. The current PR (phase 1) already translates, using accept-language
, on a per-browser basis— it's just that only the "kibana is starting up" and one other error message are translated. Phase 4 has the "Translation Identifiers Added to Kibana UI" step which actually allows the rest of the UI to be translated. Hope this helps!
@LeeDr thanks for the feedback.
a translation at the server level and all users see only that one language
To elaborate on @srl295 description above, the idea here is to deliver in a phased approach. The phases 1-3 is to put the i18n infrastructure in place and show it working end-to-end in Kibana. The key part is the i18n plugin which provides means to supply translations in a standard format that it not dependent on a localization framework. The proof of this is to translate the welcome string in Kibana. Phase 4 is about translating the rest of the UI using the underlying i18n plugin to get the strings.
Can we add notes to the phase descriptions to make it clear what the functionality is?
Sure, can do. That is a good idea.
We might also want to start a discussion about which phases might be release candidates.
This is on the TODO list with @Bargs and @epixa.
@Bargs, @LeeDr I have updated the design doc kibana-proposal-update as per your feedback. Let me know if I have covered all the angles.
@Bargs, @srl295 could you update the main ticket description?
@hickeyma done!
Thanks @srl295
@Bargs, @srl295, could you also add this line to i18n Plugin (update) in Phase 2 of #6515:
@Bargs, @epixa, @shikhasriva, @spalger, @ycombinator, @cjcenizal, @thomasneirynck, @tylersmalley, @bevacqua, @simianhacker, @BigFunger, @jbudz, @ppisljar Thank you all for attending the red-team review yesterday and for your feedback. Let me know if I forgotten anyone.
Here are notes from the review (thanks to @Bargs for taking notes!):
Feel free to comment and give feedback especially if anything has been forgotten!
I have updated the design doc kibana-proposal-update with the feedback leaving unresolved items as open or blockers.
Maybe for the moment, we use it as an intermediate document and only update the main document when we are ready?
Thats works fine. @hickeyma , Thanks for aggregating the comments
responding to https://github.com/elastic/kibana/issues/6515#issuecomment-237287195 very late
yaml instead of JSON…
I'd recommend JSON as it is more commonly used in translation systems.
@ycombinator you had some compelling reasons about using YAML, could you share them here?
Sure, I don't feel strongly about either choice but, in a past i18n project YAML was chosen over JSON for a couple of reasons:
Those are the two reasons I recall at the moment. If I think of others I'll add them to the comment. But again, I don't personally feel strongly about one being strictly better than the other.
Also, we could go with JSON initially and start accepting YAML at a later point in time if we start to run into the above reasons in practice.
@ycombinator Thanks for the feedback. For now, we will use JSON. We can always provide a YAML to JSON converter if that is the preference for localization engineers.
What do we do when the requested language exists, but it doesn't cover all translations keys (for instance, there are French translations for a plugin but not core Kibana)? Do we fill in the missing keys with the default language?
Refer to i18n plugin pr comment for more details.
Decide the phases to be delivered in the Elastic release candidates
Discussed with @Bargs. Not necessary for the feature. More for Elastic internal QA.
@srl295, @Bargs I have updated the design doc kibana-proposal-update. Could you update the design doc when you get a chance?
Updated
@Bargs thanks
thanks ... great collaboration and awesome progress.
@srl295, @Bargs, @spalger I have updated the design doc kibana-proposal-update following some changes in Phase 1. Could you update the design doc when you get a chance pls.?
@Bargs, @spalger I wonder should we re-examine the design again following the merge of "Phase 1"? I started a Phase 2 PR (https://github.com/elastic/kibana/pull/8766) and @posijon is working on it. Maybe we should see if current design still applies for the phase?
It might be worth taking a look. There are things, like pulling the translations from a rest api, that I would push back on.
Yeah we should definitely take a look and make sure we're on the same page. The REST API bit, for instance, I don't think has been updated since our "red team" meeting. At that time I believe we said we'd try embedding the translations in the initial page load first.
Also, issue description updated.
Great stuff @Bargs and @spalger. Lets try and sync-up on IRC and work out how to progress for phase 2 and 3.
@Bargs, @epixa, @shikhasriva, @spalger, @posijon Thank you all for attending the sync-up yesterday and for your feedback.
Here are notes from the meeting (let me know if I have misrepresented anything):
xplugins
with React@Bargs, @spalger Updated the design doc after yesterday's meeting as kibana-proposal-update. Could you update the design doc when you get a chance pls.?
Updated
+1
@spalger Updated the design doc kibana-proposal-update. Could you update the design doc when you get a chance pls.?
Could you update the design doc when you get a chance pls.?
Sorry, which design doc are you referring to?
@spalger he's asking for this issue ^^ ’s text to be updated from the above gist. As issue creator I was able to do it - so Done
Thanks @srl295
@spalger unfortunately no rights on the doc to update!
FYI: 🎦 posted blog+movie showing off g11n progress so far https://srl295.github.io/2017/03/17/translating-kibana/
Great work so far! @srl295
Thanks for all the great work! When can we expect phase 3 to be available?
@elkusr There's no target yet for phase 3, and there's still some work in phase 2 to complete first now that Kibana is using react so much.
How are these efforts progressing now?
Is that correct, that the tasks presented in (https://srl295.github.io/2017/03/17/translating-kibana/) can be used to add any language to a Kibana instance (for example 5.6)? In other words, adding another language to Kibana is feasible as of this writing, it just takes a "little" effort, and that Phase 3 is about to ease this effort?
@zzvara It isn't possible to translate Kibana right now. Phases 2 and 3 need to be completed before you can translate templates across Kibana.
Kibana Globalization
Note: These requirements were originally written up over a year ago. Take any specific implementation details with a grain of salt. Phases that were already implemented should be reevaluated to make sure they still make sense, especially in light of the new kibana platform work.
Purpose
Architecture Diagram
This was our original plan, but it might need to change dramatically for the new platform
Design
Phase 1
Note this phase was completed, but it's possible the implementation might need to change for the new platform. Most of the code lives here
Purpose
Decide Language for Translation Algorithm
I18n Class
Manages the language translations for Kibana
Responsible for loading translated content per language
The translations file are JSON files with a flat keyspace, where the keys are unique. This uniqueness between translation plugins could be achieved by prefixing the keys with the plugin name. The key signifies the translation ID which would be referenced in translatable files (like JS, HTML etc.).
The key value is the translation string
Example translation JSON file
en.json
UiI18n Class
I18n
class)accept-language header
to BCP 47 tagsTool for Verifying All Translations have Translatable Strings
i18n(<key>)
function in the Jade template. A tool can then be used to find such pattern and extract the keys to fileDeliverable
I18n
unit testsPhase 2
Purpose
Translations Available Client Side
Translation Identifiers Added to Kibana UI
BEFORE (HTML)
AFTER (HTML)
Tool for Verifying All Translations have Translatable Strings (Update)
Translation Plugin Generator
Deliverable
Phase 3
Purpose
Translation Identifiers Added to Kibana UI
Deliverables
Phase 4
Purpose
Handling Language Packs
Deliverables
Blockers
Open Questions/Issues
History
This section has links to prior versions of this issue text.
Authors