Localization "Index Patterns" tab #20525
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,7 @@ import uiRoutes from 'ui/routes'; | |
| import { uiModules } from 'ui/modules'; | ||
| import appTemplate from './app.html'; | ||
| import landingTemplate from './landing.html'; | ||
| import { management, translateSectionName } from 'ui/management'; | ||
| import { FeatureCatalogueRegistryProvider, FeatureCatalogueCategory } from 'ui/registry/feature_catalogue'; | ||
| import { timefilter } from 'ui/timefilter'; | ||
| import 'ui/kbn_top_nav'; | ||
|
|
@@ -46,7 +46,7 @@ require('ui/index_patterns/route_setup/load_default')({ | |
|
|
||
| uiModules | ||
| .get('apps/management') | ||
| .directive('kbnManagementApp', function (Private, $location, i18n) { | ||
| return { | ||
| restrict: 'E', | ||
| template: appTemplate, | ||
|
|
@@ -66,6 +66,7 @@ uiModules | |
| if ($scope.section) { | ||
| $scope.section.items.forEach(item => { | ||
| item.active = `#${$location.path()}`.indexOf(item.url) > -1; | ||
| item.display = translateSectionName(i18n, item.display); | ||
|
There was a problem hiding this comment. I think it makes more sense for the code registering the section to be responsible for localizing this text, e.g.:
|
||
| }); | ||
| } | ||
| } | ||
|
|
@@ -74,12 +75,20 @@ uiModules | |
|
|
||
| uiModules | ||
| .get('apps/management') | ||
| .directive('kbnManagementLanding', function (kbnVersion, i18n) { | ||
| return { | ||
| restrict: 'E', | ||
| link: function ($scope) { | ||
| $scope.sections = management.items.inOrder; | ||
| $scope.kbnVersion = kbnVersion; | ||
|
|
||
| if ($scope.sections && $scope.sections.length) { | ||
| $scope.sections.forEach(section => { | ||
| section.items.forEach(item => { | ||
| item.display = translateSectionName(i18n, item.display); | ||
|
|
||
| }); | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,8 @@ import { | |
| EuiButton, | ||
| } from '@elastic/eui'; | ||
|
|
||
| import { FormattedMessage } from '@kbn/i18n/react'; | ||
|
|
||
| export const EmptyState = ({ | ||
| onRefresh, | ||
| }) => ( | ||
|
|
@@ -40,27 +42,47 @@ export const EmptyState = ({ | |
| <EuiFlexItem grow={false}> | ||
| <EuiTitle> | ||
| <EuiTextColor color="subdued"> | ||
| <h2 style={{ textAlign: 'center' }}> | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.header" | ||
| defaultMessage="Couldn't find any Elasticsearch data" | ||
| /> | ||
| </h2> | ||
| </EuiTextColor> | ||
| </EuiTitle> | ||
| <EuiSpacer size="s"/> | ||
| <EuiText> | ||
| <p> | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.label.detail" | ||
| defaultMessage="{needToIndex} {learnHowLink} or {getStartedLink}" | ||
| values={{ | ||
| needToIndex: ( | ||
|
There was a problem hiding this comment. I think I would prefer the values object here to be declared as a constant above and then referenced here. This would make things easier to read, and would also prevent a new object being created with each rerender. See this for an explanation of why that might be bad: https://medium.com/@esamatti/react-js-pure-render-performance-anti-pattern-fb88c101332f There was a problem hiding this comment. @bmcconaghy Basically, this is common pattern for avoiding creation of new object, but I would not say, that in this case, because:
There was a problem hiding this comment. Fair enough on the performance point. I actually find the FormattedMessage nested in FormattedMessage harder to read personally, as I get lost in the multiple levels of nesting, so would still like to see the values defined outside of the FormattedMessage and then referenced there. There was a problem hiding this comment. It feels like the right solution for this readability problem would be integrating i18n into EUI components at some point, so that this: <EuiTextColor color="subdued">
<FormattedMessage
id="kbn.management.createIndexPattern.emptyStateLabel.needToIndexLabel"
defaultMessage="You'll need to index ....."
/>
</EuiTextColor>can be changed to: <EuiTextColor color="subdued" i18n-id="kbn.management.createIndexPattern.emptyStateLabel.needToIndexLabel">
You'll need to index .....
</EuiTextColor>We'll still have to use But that's a bigger effort that spans several teams and i18n should prove itself in practice first. |
||
| <EuiTextColor color="subdued"> | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.label.needToIndex" | ||
| defaultMessage="You'll need to index some data into Elasticsearch before you can create an index pattern." | ||
| /> | ||
| </EuiTextColor> | ||
| ), | ||
| learnHowLink: ( | ||
| <EuiLink href="#/home/tutorial_directory"> | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.label.learnHowLink" | ||
| defaultMessage="Learn how" | ||
| /> | ||
| </EuiLink> | ||
| ), | ||
| getStartedLink: ( | ||
| <EuiLink href="#/home/tutorial_directory/sampleData"> | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.label.getStartedLink" | ||
| defaultMessage="get started with some sample data sets." | ||
| /> | ||
| </EuiLink> | ||
| ) | ||
| }} | ||
| /> | ||
| </p> | ||
| </EuiText> | ||
|
|
||
|
|
@@ -73,7 +95,10 @@ export const EmptyState = ({ | |
| onClick={onRefresh} | ||
| data-test-subj="refreshIndicesButton" | ||
| > | ||
| <FormattedMessage | ||
| id="kbn.management.createIndexPattern.emptyState.checkData.button" | ||
| defaultMessage="Check for new data" | ||
| /> | ||
| </EuiButton> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need to use the service here? Can we use a vanilla-JS module instead?
We should try to avoid adding any code which couples us more tightly with Angular and its DI system, so ease the conversion of this code to React in the future. This comment applies to similar instances elsewhere in this PR.
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's a good point. Thank you.
Currently to apply it we need the changes from the PR #20905. As soon as the PR is merged I will update the code.