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

Feature/dashboard translations #24328

Merged

Conversation

pavel06081991
Copy link
Contributor

@pavel06081991 pavel06081991 commented Oct 22, 2018

dashboard translations

Fixes: #22648

@pavel06081991
Copy link
Contributor Author

blocked by #23684

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pavel06081991
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

defaultFocusedButton: ConfirmationButtonTypes.CANCEL,
title: 'Discard changes to dashboard?'
title: i18n('kbn.dashboard.app.changeViewModeConfirmModal.title',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe .title -> .discardChangesTitle?

@@ -268,14 +270,22 @@ app.directive('dashboardApp', function ($injector) {
}

confirmModal(
`Once you discard your changes, there's no getting them back.`,
i18n('kbn.dashboard.app.changeViewModeConfirmModal.description',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe .description -> .discardChangesDescription?


const displayTitle = `${title}${unsavedSuffix}`;
return isEditMode ? 'Editing ' + displayTitle : displayTitle;
if (isEditMode && isDirty && isEditMode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplication of isEditMode in this condition.
I think it can be

if (isEditMode && isDirty) {
    displayTitle = i18n.translate('kbn.dashboard.strings.dashboardUnsavedEditTitle', {
      defaultMessage: 'Editing {title} (unsaved)',
      title,
    });
  } else if (isEditMode) {
      displayTitle = i18n.translate('kbn.dashboard.strings.dashboardEditTitle', {
        defaultMessage: 'Editing {title}',
        title,
      });
  } else {
    displayTitle = title;
  }

@@ -106,7 +107,7 @@ function ResponsiveGrid({
const ResponsiveSizedGrid = sizeMe(config)(ResponsiveGrid);


export class DashboardGrid extends React.Component {
export class DashboardGridUi extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove export.

@@ -106,7 +107,7 @@ function ResponsiveGrid({
const ResponsiveSizedGrid = sizeMe(config)(ResponsiveGrid);


export class DashboardGrid extends React.Component {
export class DashboardGridUi extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove export.

@@ -141,8 +165,12 @@ uiRoutes
FeatureCatalogueRegistryProvider.register(() => {
return {
id: 'dashboard',
title: 'Dashboard',
description: 'Display and share a collection of visualizations and saved searches.',
title: i18n.translate('kbn.dashboard.featureCatalogue.dashboard.title', {
Copy link
Contributor

Choose a reason for hiding this comment

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

kbn.dashboard.featureCatalogue.dashboard.title -> kbn.dashboard.featureCatalogue.dashboardTitle

title: i18n.translate('kbn.dashboard.featureCatalogue.dashboard.title', {
defaultMessage: 'Dashboard',
}),
description: i18n.translate('kbn.dashboard.featureCatalogue.dashboard.description', {
Copy link
Contributor

Choose a reason for hiding this comment

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

-> .dashboardDescription

@@ -194,14 +197,27 @@ export class DashboardListing extends React.Component {
return (
<EuiOverlayMask>
<EuiConfirmModal
title="Delete selected dashboards?"
title={this.props.intl.formatMessage('kbn.dashboard.listing.deleteSelectedDashboardsConfirmModal.title', {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's worth to create a variable for this.props.intl.

@@ -212,14 +228,35 @@ export class DashboardListing extends React.Component {
return (
<React.Fragment>
<EuiCallOut
title="Listing limit exceeded"
title={this.props.intl.formatMessage('kbn.dashboard.listing.listingLimitExceeded.title', {
Copy link
Contributor

Choose a reason for hiding this comment

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

the same as above

but your <strong>listingLimit</strong> setting prevents the table below from displaying more than {this.props.listingLimit}.
You can change this setting under <EuiLink href="#/management/kibana/settings">Advanced Settings</EuiLink>.
{this.props.intl.formatMessage('kbn.dashboard.listing.listingLimitExceeded.description', {
defaultMessage: 'You have {totalDashboards} dashboards, \
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not use \, because browsers behave different with multiline javascript strings by using \.
Moreover in jsx it's necessary to use \ for multiline string in template. This should work:

defaultMessage: 'You have {totalDashboards} dashboards, but your {listingLimitText} setting prevents the table below from displaying more than {listingLimitValue}. You can change this setting under {advancedSettingsLink}.'

@@ -304,7 +375,9 @@ export class DashboardListing extends React.Component {
<EuiFlexItem grow={true}>
<EuiFieldSearch
aria-label="Filter dashboards"
Copy link
Contributor

Choose a reason for hiding this comment

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

This aria-label should be translated as well.

@@ -304,7 +375,9 @@ export class DashboardListing extends React.Component {
<EuiFlexItem grow={true}>
<EuiFieldSearch
aria-label="Filter dashboards"
placeholder="Search..."
placeholder={this.props.intl.formatMessage('kbn.dashboard.listing.searchBar.searchField.placeholder', {
Copy link
Contributor

Choose a reason for hiding this comment

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

-> .searchFieldPlaceholder

@@ -304,7 +375,9 @@ export class DashboardListing extends React.Component {
<EuiFlexItem grow={true}>
<EuiFieldSearch
aria-label="Filter dashboards"
placeholder="Search..."
placeholder={this.props.intl.formatMessage('kbn.dashboard.listing.searchBar.searchField.placeholder', {
defaultMessage: 'Search...',
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 instead of three dots.

@@ -323,7 +396,9 @@ export class DashboardListing extends React.Component {
const tableColumns = [
{
field: 'title',
name: 'Title',
name: this.props.intl.formatMessage('kbn.dashboard.listing.table.titleColumn.title', {
Copy link
Contributor

Choose a reason for hiding this comment

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

-> .titleColumnTitle
and please fix the same in ids below

{!this.props.initialized && 'loading...'}
{!this.props.initialized && <FormattedMessage
id="kbn.dashboard.panel.ebmeddableViewport.loadingLabel"
defaultMessage="loading..."
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 instead of three dots.

}: PanelOptionsMenuProps) {
const button = (
<EuiButtonIcon
iconType={isViewMode ? 'boxesHorizontal' : 'gear'}
color="text"
className={isViewMode && !isPopoverOpen ? 'dshPanel_optionsMenuButton' : ''}
aria-label="Panel options"
aria-label={intl.formatMessage({
id: 'kbn.dashboard.panel.optionsMenu.panelOptionsButton.ariaLabel',
Copy link
Contributor

Choose a reason for hiding this comment

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

-> .panelOptionsButtonAriaLabel

@@ -55,13 +62,21 @@ export function PanelOptionsMenuForm({
value={title}
onChange={onInputChange}
onKeyDown={onKeyDown}
aria-label="Changes to this input are applied immediately. Press enter to exit."
aria-label={intl.formatMessage({
id: 'kbn.dashboard.panel.optionsMenuForm.titleInput.ariaLabel',
Copy link
Contributor

Choose a reason for hiding this comment

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

-> .panelTitleInputAriaLabel

@@ -78,7 +85,10 @@ export class PanelUtils {
static parseVersion(version = '6.0.0') {
const versionSplit = version.split('.');
if (versionSplit.length < 3) {
throw new Error(`Invalid version, ${version}, expected <major>.<minor>.<patch>`);
throw new Error(i18n.translate('kbn.dashboard.panel.invalidVersionErrorMessage', {
defaultMessage: 'Invalid version, {version}, expected <major>.<minor>.<patch>',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put <major>.<minor>.<patch> in values.

@@ -115,7 +131,13 @@ export class DashboardAddPanel extends React.Component {
}

this.lastToast = toastNotifications.addSuccess({
title: `${this.state.selectedTab.name} was added to your dashboard`,
title: this.props.intl.formatMessage({
id: 'kbn.dashboard.topNav.addPanel.toastNotifications.selectedTadAddedToDashboardSuccessMessageTitle',
Copy link
Contributor

Choose a reason for hiding this comment

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

typo -> .selectedTabAddedToDashboardSuccessMessageTitle

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@tsullivan
Copy link
Member

Check the elastic+kibana+pull-request+multijob-intake logs. It looks like there are multiple failures caused by the changes in this PR

Error: Uncaught [Invariant Violation: [React Intl] Could not find required `intl` object. <IntlProvider> needs to exist in the component ancestry.]

This functional test failure also looks like it could be related:

09:09:31    ? debg randomness seed: 1541754571191
09:09:31    ? info Starting tests
09:09:31    ? warn debug logs are being captured, only error logs will be written to the console
09:09:31    ?
09:09:31      ?-: phantom
09:09:31        ?-> "before all" hook
09:09:31        ?-> "before all" hook
09:09:52        ?-: BWC report generation into existing indexes
09:09:52          ?-> "before all" hook
09:09:52          ?-: existing 6_2 index
09:09:52            ?-> "before all" hook
09:09:52            ?-> "before all" hook: load data and add index alias
09:09:54            ?-> multiple jobs posted
09:09:54              ?-> "before each" hook: global before each
09:16:05              ?- ? fail: "phantom BWC report generation into existing indexes existing 6_2 index multiple jobs posted"
09:16:05              ?      Error: expected 500 to equal 200
09:16:05              ?       at Assertion.assert (node_modules/expect.js/index.js:96:13)
09:16:05              ?       at Assertion.be.Assertion.equal (node_modules/expect.js/index.js:216:10)
09:16:05              ?       at Assertion.(anonymous function) [as be] (node_modules/expect.js/index.js:69:24)
09:16:05              ?       at Object.waitForJobToFinish (test/reporting/services/reporting_api.js:39:29)
09:16:05              ?       at <anonymous>
09:16:05              ? 
09:16:05              ? 

@maryia-lapata
Copy link
Contributor

@tsullivan yes, this is because old phantom browser (Safari 7) doesn't support Internationalization API. We are working on this issue #25465.

@maryia-lapata
Copy link
Contributor

Blocked by #25465.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

LGTM

@pavel06081991 pavel06081991 merged commit 7baea1d into elastic:master Nov 20, 2018
@pavel06081991 pavel06081991 deleted the feature/dashboard-translations branch November 20, 2018 07:19
pavel06081991 added a commit to pavel06081991/kibana that referenced this pull request Nov 20, 2018
pavel06081991 added a commit that referenced this pull request Nov 20, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Project:i18n Team:Visualizations Visualization editors, elastic-charts and infrastructure v6.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants