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.

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

Translate heatmap and heatmap_options #23812

Merged
merged 9 commits into from
Oct 18, 2018

Conversation

tibmt
Copy link
Contributor

@tibmt tibmt commented Oct 4, 2018

issue #23257
Translation of Heatmap visualization component

@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@pavel06081991 pavel06081991 self-requested a review October 4, 2018 15:01
Copy link
Contributor

@pavel06081991 pavel06081991 left a comment

Choose a reason for hiding this comment

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

Great work! Please fix some comments

<strong>Required:</strong> You must specify at least one range.
<span
i18n-id="kbnVislibVisTypes.controls.heatmapOptions.requiredTitle"
i18n-default-message="<strong>Required:</strong> You must specify at least one range."
Copy link
Contributor

Choose a reason for hiding this comment

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

use i18n-values for texts containing nested tags

>
Add Range
</button>
i18n-id="kbnVislibVisTypes.controls.heatmapOptions.addRangeButton"
Copy link
Contributor

Choose a reason for hiding this comment

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

addRangeButton => addRangeButtonLabel

<div class="text text-center text-info">Note: colors can be changed in the legend</div>
<div
class="text text-center text-info"
i18n-id="kbnVislibVisTypes.controls.heatmapOptions.noteTitle"
Copy link
Contributor

Choose a reason for hiding this comment

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

noteTitle => howToChangeColorsDescription

<label
class="kuiSideBarFormRow__label"
for="enableHover"
i18n-id="kbnVislibVisTypes.editors.heatmap.HighlightLabel"
Copy link
Contributor

Choose a reason for hiding this comment

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

HighlightLabel => highlightLabel

@@ -86,7 +86,7 @@ export default function HeatmapVisType(Private) {
{
group: 'metrics',
name: 'metric',
title: 'Value',
title: i18n('kbnVislibVisTypes.heatmap.valueTitle', { defaultMessage: 'Value' }),
Copy link
Contributor

Choose a reason for hiding this comment

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

valueTitle => metricTitle

@@ -97,23 +97,23 @@ export default function HeatmapVisType(Private) {
{
group: 'buckets',
name: 'segment',
title: 'X-Axis',
title: i18n('kbnVislibVisTypes.heatmap.xAxisTitle', { defaultMessage: 'X-Axis' }),
Copy link
Contributor

Choose a reason for hiding this comment

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

xAxisTitle => segmentTitle

min: 0,
max: 1,
aggFilter: ['!geohash_grid', '!filter']
},
{
group: 'buckets',
name: 'group',
title: 'Y-Axis',
title: i18n('kbnVislibVisTypes.heatmap.yAxisTitle', { defaultMessage: 'Y-Axis' }),
Copy link
Contributor

Choose a reason for hiding this comment

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

yAxisTitle => groupTitle

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pavel06081991
Copy link
Contributor

@tibmt add 'kbnVislibVisTypes' to .i18nrc.json for CI to pass CI validation

<strong>Required:</strong> You must specify at least one range.
</p>
<p
i18n-id="kbnVislibVisTypes.controls.heatmapOptions.requiredTitle"
Copy link
Contributor

Choose a reason for hiding this comment

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

requiredTitle => specifiedRangeNumberWarningMessage

i18n-default-message="{icon} {required} You must specify at least one range."
i18n-values="{
icon: '<span class=&quot;kuiIcon fa-danger text-danger&quot;></span>',
required: '<strong>' + editorState.requiredDescription + '</strong>'
Copy link
Contributor

Choose a reason for hiding this comment

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

requiredDescription => requiredText

@@ -88,6 +88,10 @@ module.directive('heatmapOptions', function () {
$scope.uiState.on('colorChanged', () => {
$scope.customColors = true;
});

$scope.editorState.requiredDescription = i18n('kbnVislibVisTypes.params.ranges.warning.requiredDescription', {
defaultMessage: 'Required:'
Copy link
Contributor

Choose a reason for hiding this comment

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

requiredDescription => requiredText

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@pavel06081991 pavel06081991 left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

i18n-id="kbnVislibVisTypes.controls.heatmapOptions.specifiedRangeNumberWarningMessage"
i18n-default-message="{icon} {required} You must specify at least one range."
i18n-values="{
icon: '<span class=\'kuiIcon fa-danger text-danger\'></span>',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use backtick strings to prevent escaping single quotes.

Copy link
Contributor Author

@tibmt tibmt Oct 15, 2018

Choose a reason for hiding this comment

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

@timroes There is error if we will use backtick.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Angular does not allow use backtick strings in template

@@ -88,6 +88,10 @@ module.directive('heatmapOptions', function () {
$scope.uiState.on('colorChanged', () => {
$scope.customColors = true;
});

$scope.editorState.requiredText = i18n('kbnVislibVisTypes.params.ranges.warning.requiredText', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't push this to the editorState, but use the $scope directly.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM

@tibmt tibmt merged commit 9a3414c into elastic:master Oct 18, 2018
@tibmt tibmt deleted the feature/translations/heatmap branch October 18, 2018 11:17
tibmt added a commit to tibmt/kibana that referenced this pull request Oct 18, 2018
Translation of Heatmap visualization component
tibmt added a commit that referenced this pull request Oct 18, 2018
Translation of Heatmap visualization component
@maryia-lapata maryia-lapata added v6.5.0 Feature:Heatmap Heatmap visualization labels Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants