-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kibana Globalization - Phase 2 #8766
Conversation
Can one of the admins verify this patch? |
Dependent on #7545 |
21255b2
to
8fe6204
Compare
Can one of the admins verify this patch? |
3a33bff
to
a6cd076
Compare
c6830ec
to
cc72df8
Compare
@spalger PR ready for review. |
jenkins, test this |
webpackShims/ui-bootstrap.js
Outdated
return deferred.promise; | ||
}; | ||
}) | ||
.config(['$translateProvider', function ($translateProvider) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the ['$translateProvider', ...]
wrapper around this provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, updated.
webpackShims/ui-bootstrap.js
Outdated
}; | ||
}) | ||
.config(['$translateProvider', function ($translateProvider) { | ||
$translateProvider.preferredLanguage('en'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC the language is not variable at this point, so perhaps we could simplify this (and remove the translationLoader
factory) by just using $translateProvider.translations('default', chrome.getTranslations())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks, Means I can now remove the custom loader.
@@ -66,6 +66,7 @@ export default async (kbnServer, server, config) => { | |||
|
|||
async function getKibanaPayload({ app, request, includeUserProvidedConfig }) { | |||
const uiSettings = server.uiSettings(); | |||
const translations = await uiI18n.getTranslationsForRequest(request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may not be necessary, but if angular-translate
requires the language code for the translations it's providing, perhaps we could update getTranslationsForRequest
to return the resolved language code as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point but unnecessary now.
@@ -1,11 +1,27 @@ | |||
define(function (require) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is specifically for the ui-bootstrap
dependency. I think it would be better if we moved the new logic into src/ui/public/chrome/api/translations.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to loading the translations directly with angular-translate then the custom loader factory function is now obsolete.
webpackShims/angular.js
Outdated
@@ -1,7 +1,8 @@ | |||
require('jquery'); | |||
require('node_modules/angular/angular'); | |||
require('node_modules/angular-translate/dist/angular-translate.min'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to override the default import. We minify the distributable and prefer non-minified code for development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sure. Removed.
TRANSLATION.md
Outdated
@@ -0,0 +1,140 @@ | |||
Translating Kibana |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this into docs/development/plugin/development-internationalization.asciidoc
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll hold back my review of the docs until we move them there, since the transition will wipe out all comments anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, moved and converted to an asciidoc.
@@ -0,0 +1,23 @@ | |||
A Kibana translation plugin structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the intention here, but I'm not a fan of having an unused template as a part of the source. Fixtures are for tests, so if we wanted to make use of them that way I would feel better about it, but this would be best as a pull-request to generator-kibana-plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aim to amuse! :) I have made changes in the other repo as elastic/generator-kibana-plugin#45
cc72df8
to
4926fab
Compare
This commit is part og Kibana globalization and PR elastic/kibana#8766
Updates after review comments: elastic#8766 (review)
@spalger Thanks for #8766 (review) . Comments updated and ready for review again. |
- use translate directive in HTML attribute instead of body for variable replacement
Updates after review comments: elastic#8766 (review)
* Otherwise, the '$http GET' to fetch translation resources shows up as a deferred task here. * Referenced from: srl295@5d9959c
Updates following review comments: elastic#8766 (review)
Update the following review comments: elastic#8766 (review) elastic#8766 (review)
lol @Bargs. Added strings to the translation file and then got distracted! Committed the changes now. |
jenkins, test this |
Backports PR #8766 **Commit 1:** Add boilerplate plugin * Original sha: 9ac0878 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-12-15T18:39:17Z **Commit 2:** Load translations on the client side Translations need to be loaded on the client side so that they can be used the globalization frameworks like 'angular-translate'. This translations are made available by embedding the translations in the initial HTML payload. * Original sha: 1f84e57 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-01-13T15:30:26Z **Commit 3:** Update translation plugin template for the change in how translations are published * Original sha: d2fc0c4 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-01-16T15:05:04Z **Commit 4:** Add angular-translate as the Angular globalization framework The angular-translate framework integrates with the Kibana i18n engine whereby it consumes translations from the i18n engine and not the translations directly. This is achieved by using a custom loader which calls the client side getTranslations() method. This method return the translations loaded by the i18n engine during instantation of Kibana server instance. * Original sha: 0ab48f0 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-01T22:03:17Z **Commit 5:** Extract translation strings from the index pattern create page into translation file This commit is where the strings are now localized for this page and therefore will be loaded by the Angular globalization framework. It also associates a translation ID to each string block and therefore enables translation of the strings into different languages. It also provides the pattern for localizing other pages in the UI. * Original sha: 959c7ed * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-02T11:33:08Z **Commit 6:** Add Kibana translation documentation * Original sha: 5317e6e * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-02T15:18:26Z **Commit 7:** Update "translate" tag syntax - use translate directive in HTML attribute instead of body for variable replacement * Original sha: 68202d9 * Authored by Jonathan Lo <jlo@us.ibm.com> on 2017-02-07T20:24:30Z * Committed by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-06T15:56:26Z **Commit 8:** Add new translation key pattern to verify task * Original sha: 13cfd15 * Authored by Jonathan Lo <jlo@us.ibm.com> on 2017-02-07T22:14:55Z * Committed by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-06T15:56:26Z **Commit 9:** Fix syntax error in create view * Original sha: 165d4cd * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-08T10:43:18Z **Commit 10:** Update the translation verification to handle separate translation patterns * Original sha: 48e1ecd * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-08T17:01:45Z **Commit 11:** Update after review Updates after review comments: #8766 (review) * Original sha: afee6d9 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-10T15:01:46Z **Commit 12:** Update internationalization documentation * Original sha: 177408e * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-16T16:33:25Z **Commit 13:** [test] call $timeout.flush() before debounce tests * Otherwise, the '$http GET' to fetch translation resources shows up as a deferred task here. * Referenced from: srl295@5d9959c * Original sha: 4dbf3a0 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-21T11:12:18Z **Commit 14:** Update after review Updates following review comments: #8766 (review) * Original sha: c2438de * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-02-28T16:35:27Z **Commit 15:** Merge extract and verify translation build tasks into one task * Original sha: 622802c * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-01T16:53:02Z **Commit 16:** Update document after new changes for js and info string translations * Original sha: 2f269f8 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-01T17:15:04Z **Commit 17:** Update after review Update the following review comments: #8766 (review) #8766 (review) * Original sha: 5e94886 * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-03T14:27:28Z **Commit 18:** Replace the translation string with the translation IDs in index create pattern view * Original sha: fb8dffb * Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2017-03-06T16:20:05Z
* Integrate angular-translate globalization framework with i18n engine * Provide template for enabling translations in an AngularJS view by translating a view * Verification tool for translation keys used in angular-translate * Documentation
…38daaefc30e21bdd18d1ce2263923cfe12a * Integrate angular-translate globalization framework with i18n engine * Provide template for enabling translations in an AngularJS view by translating a view * Verification tool for translation keys used in angular-translate * Documentation # Conflicts: # src/core_plugins/kibana/public/management/sections/indices/_create.html # src/ui/public/chrome/chrome.js # tasks/build/verify_translations.js # tasks/utils/i18n_verify_keys.js
Phase 2
Purpose
Translations Available Client Side
hose that want to enable translation in Kibana
those that want to contribute translations to Kibana
those that want to create a Kibana Plugin
Translation Identifiers Added to Kibana UI
BEFORE (HTML)
AFTER (HTML)
Tool for Verifying All Translations have Translatable Strings (Update)
Translation Plugin Generator
Deliverable