Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[i18n] Translations for Monitoring: Cluster and Alerts #24736

Merged
merged 20 commits into from
Nov 16, 2018
Merged

[i18n] Translations for Monitoring: Cluster and Alerts #24736

merged 20 commits into from
Nov 16, 2018

Conversation

maryia-lapata
Copy link
Contributor

Translations for Monitoring: Cluster and Alerts pages

Fixes: #24713

@maryia-lapata maryia-lapata requested a review from Nox911 October 29, 2018 13:40
Nox911
Nox911 reviewed Oct 30, 2018
View reviewed changes
Copy link
Contributor

@Nox911 Nox911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix comments

Nox911
Nox911 suggested changes Oct 30, 2018
View reviewed changes
Copy link
Contributor

@Nox911 Nox911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Please fix some minor comments, and let's discuss some names of id.

@Nox911
Copy link
Contributor

Nox911 commented Oct 30, 2018

LGTM

@chrisronline
Copy link
Contributor

Do I need anything setup to test this? Or maybe more broadly, how do I test this? :)

@maryia-lapata
Copy link
Contributor Author

@chrisronline you can set up a pseudo-locale to see what labels are translated. Pseudo-locale adds diacritical mark to each [A-Za-z] character and duplicates every third character to increase messages length by approx. 33%. It's also used to check whether ui breaks with increased message length or not.

How to use:

Set i18n.locale to en-xa in ./config/kibana.yml
Restart yarn start.

There is one more way to test translation. It's possible to check custom locale. It's more complex because this approach hasn't automated yet. Here are the steps:

  1. run command node scripts/i18n_check --output ./. It will generate en.json file with default translations and it place the file in the root folder.
  2. create for example fr.json file (fr is locale name here), copy translations from en.json, update messages in fr.json according to locale (you can just update one or several messages or for example add special symbols at the begging and at the end of messages ("$$some message$$")).
  3. point plugin to the translation file. For example, in src/core_plugins/kibana/index.js add path to fr.json file in translations array:
translations: [
	        require('path').resolve(__dirname, './../../../fr.json')
	      ],
  1. set up locale in config/kibana.yml by changing i18n.locale param. (i18n.locale: "fr")
    Restart yarn start.

To make PRs easy to review we are splitting all Monitoring translations into several PRs. So please take into account that one screen (for example Cluster) may be translated not fully in one PR, because we translate folder by folder and some labels on the screen can be translated in other PR.

chrisronline
chrisronline reviewed Oct 30, 2018
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I added a couple comments/questions but this is looking great!

x-pack/plugins/monitoring/public/components/cluster/overview/elasticsearch_panel.js
<FormattedMessage
id="xpack.monitoring.cluster.overview.esPanel.jvmHeapLabel"
defaultMessage="{javaVirtualMachine} Heap"
values={{ javaVirtualMachine: 'JVM' }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So JVM is not something translated, but I see Elasticsearch not used like this, does that mean Elasticsearch will be translated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. There is also Kibana translation glossary (https://docs.google.com/document/d/1e-lHAPAtxodP8U4gbwA3lw8FyhhW7M303--77nMpe8w/edit) that contains terms that will remain untranslated.
Mostly we put in values such technical labels like property names url.body.query.

chrisronline
chrisronline reviewed Oct 31, 2018
View reviewed changes
x-pack/plugins/monitoring/public/components/cluster/overview/elasticsearch_panel.js Outdated
statusIndicator={statusIndicator}
url="elasticsearch"
title={props.intl.formatMessage({
id: 'xpack.monitoring.cluster.overview.esPanel.elasticsearchTitle', defaultMessage: 'Elasticsearch' })}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't Elasticsearch be not translated, per this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it shouldn't.
I reverted this change.

@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
@elastic elastic deleted a comment from elasticmachine Nov 1, 2018
chrisronline
chrisronline approved these changes Nov 1, 2018
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (once tests are passing)

@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elastic elastic deleted a comment from elasticmachine Nov 8, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@maryia-lapata
Copy link
Contributor Author

Blocked by #25465.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@maryia-lapata maryia-lapata merged commit 11cda77 into elastic:master Nov 16, 2018
@maryia-lapata maryia-lapata deleted the feature/translations/monitoring-cluster branch November 16, 2018 12:03
maryia-lapata added a commit to maryia-lapata/kibana that referenced this pull request Nov 20, 2018
@maryia-lapata
[i18n] Translations for Monitoring: Cluster and Alerts (elastic#24736)
d9d056a 
* Translations for Cluster and Alerts

* Translations for cluster and alerts

* Translations for cluster and alerts

* Fix typos

* Update id

* Update Notification snapshot

* Translate lastEvent label

* Revert changes for untranslated label.
@maryia-lapata maryia-lapata mentioned this pull request Nov 20, 2018
Merged
maryia-lapata added a commit that referenced this pull request Nov 20, 2018
@maryia-lapata
[i18n] Translations for Monitoring: Cluster and Alerts (#24736) (#25924)
5441e8c 
* Translations for Cluster and Alerts

* Translations for cluster and alerts

* Translations for cluster and alerts

* Fix typos

* Update id

* Update Notification snapshot

* Translate lastEvent label

* Revert changes for untranslated label.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisronline chrisronline chrisronline approved these changes

@Nox911 Nox911 Nox911 approved these changes

Assignees
No one assigned
Labels
backported Project:i18n Team:Monitoring Stack Monitoring team v6.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maryia-lapata @Nox911 @chrisronline @elasticmachine