[Canvas] Converting workpad header components to typescript and adding i18n by poffdeluxe · Pull Request #45274 · elastic/kibana

poffdeluxe · 2019-09-10T15:34:00Z

Summary

Converting a few more of the workpad header components to typescript and adding i18n.
Setup new "units" i18n file for unit labels

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~- [ ] Documentation was added for features that require explanation or tutorials~~
~~- [ ] Unit or functional tests were updated or added to match the most common scenarios~~
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-09-10T15:34:02Z

Pinging @elastic/kibana-canvas

x-pack/legacy/plugins/canvas/i18n/components.ts

elasticmachine · 2019-09-10T17:19:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-10T23:14:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-10T23:35:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

clintandrewhall

There are a lot of things to confirm/tweak, so I'm just going to comment for now... hit me up when everything's addressed and I'll review again. I'd love to see @bmcconaghy or someone from #i18n to take a look at the plurals and keys.

x-pack/legacy/plugins/canvas/i18n/components.ts

clintandrewhall · 2019-09-12T16:15:06Z

x-pack/legacy/plugins/canvas/i18n/components.ts

If these strings have already been translated, this will blow up in i18n parsing.

That's true but I only merged in the first version of these i18n strings yesterday. Do they translate that quickly? #44943

We extract the labels a few weeks before FF and at FF so they are not translated yet. Additionally the CI should fail if the translation files need a fix. Which can be fixed in the PR by running node/scripts i18n_check.js --fix

x-pack/legacy/plugins/canvas/i18n/components.ts

x-pack/legacy/plugins/canvas/i18n/units.ts

clintandrewhall · 2019-09-12T16:17:37Z

...y/plugins/canvas/public/components/workpad_header/control_settings/auto_refresh_controls.tsx

This doesn't look properly formatted. Are you sure your IDE is configured properly?

I'm not sure I see the formatting issue? The linter seems happy with it

...y/plugins/canvas/public/components/workpad_header/control_settings/auto_refresh_controls.tsx

...legacy/plugins/canvas/public/components/workpad_header/control_settings/control_settings.tsx

.../legacy/plugins/canvas/public/components/workpad_header/control_settings/custom_interval.tsx

clintandrewhall · 2019-09-12T16:29:19Z

x-pack/legacy/plugins/canvas/public/components/workpad_header/workpad_export/disabled_panel.tsx

nit: This is syntactic sugar around i18n, so we should probably extract out to a dictionary.

Yeah I was curious about how to handle this case since the string needs to pass in a component with the <EuiCode>kibana.yml</EuiCode> block. My thought was maybe we don't want to import / use components in the i18n string dictionaries but I can always make that happen. I suppose alternatively I can always pass the <EuiCode>kibana.yml</EuiCode> to the string function. Any preference?

elasticmachine · 2019-09-12T17:31:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-13T00:00:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

Bamieh

Translations LGTM. Wrote a couple of comments

Bamieh · 2019-09-16T14:30:08Z

x-pack/legacy/plugins/canvas/i18n/components.ts

We extract the labels a few weeks before FF and at FF so they are not translated yet. Additionally the CI should fail if the translation files need a fix. Which can be fixed in the PR by running node/scripts i18n_check.js --fix

Bamieh · 2019-09-16T14:32:10Z

x-pack/legacy/plugins/canvas/i18n/units.ts

+  time: {
+    getSecondsText: (seconds: number) =>
+      i18n.translate('xpack.canvas.units.time.seconds', {
+        defaultMessage: '{seconds} {seconds, plural, one {second} other {seconds}}',


You can write this instead

{seconds, plural, one {# second} other {# seconds}

the # here will return the value of seconds;

elasticmachine · 2019-09-16T17:08:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-16T18:17:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-16T19:52:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

clintandrewhall

LGTM

…g i18n (elastic#45274) * Converting workpad header components to typescript and adding i18n * i18n for custom interval * Refactor of i18n ids and better time text * Fixing help example * Refactor of some naming * More string refactor and finishing workpad export * Addressing some PR feedback * Changing plural format * Quick fix * Zoom controls * Fixing id

…g i18n (#45274) (#45927) * Converting workpad header components to typescript and adding i18n * i18n for custom interval * Refactor of i18n ids and better time text * Fixing help example * Refactor of some naming * More string refactor and finishing workpad export * Addressing some PR feedback * Changing plural format * Quick fix * Zoom controls * Fixing id

* master: (33 commits) [easy] Exclude __examples__ from coverage (elastic#45556) [DOCS] Update CCR links (elastic#44012) Use unique junit report filenames again (elastic#45897) [ftr/savedObjects] add simple saved object api client to ftr s… (elastic#45856) New visualization editor Lens (elastic#36437) Sort using unix timestamp value (elastic#43162) [APM] Use POST instead of implicit GET (elastic#45903) [Canvas] Converting workpad header components to typescript and adding i18n (elastic#45274) skip flaky test (elastic#45884) set IS_PIPELINE_JOB in intake jobs (elastic#45850) [Uptime] Fix/issue 48 integration popup closes after refresh (elastic#45759) [Logs UI] Support zoom by brushing in the log rate chart (elastic#45879) [DOCS] Changes name to host (elastic#45798) [ML] Add population job wizard test (elastic#45765) [skip-ci][Maps][File upload] Geojson indexing and styling docs (elastic#41394) remove setTimeoue for state change (elastic#45853) [Graph] Restructure folders and add readme (elastic#45782) [ML] Enhance job id error message (elastic#45349) [SIEM] Do not update state component when they did unmount (elastic#45847) [i18n] sync from 7.4 latest translations (elastic#45823) ...

poffdeluxe requested a review from a team as a code owner September 10, 2019 15:34

poffdeluxe commented Sep 10, 2019

View reviewed changes

x-pack/legacy/plugins/canvas/i18n/components.ts Outdated Show resolved Hide resolved

poffdeluxe force-pushed the i18n-canvas-workpad-header-controls branch from 0c2c7b1 to cf2f081 Compare September 12, 2019 15:43

clintandrewhall reviewed Sep 12, 2019

View reviewed changes

poffdeluxe added 7 commits September 12, 2019 17:22

Converting workpad header components to typescript and adding i18n

ddd0467

i18n for custom interval

97a13c1

Refactor of i18n ids and better time text

14febeb

Fixing help example

74d6fcc

Refactor of some naming

0429210

More string refactor and finishing workpad export

2d9a9b0

Addressing some PR feedback

dc65760

poffdeluxe force-pushed the i18n-canvas-workpad-header-controls branch from cf2f081 to dc65760 Compare September 12, 2019 22:25

Bamieh reviewed Sep 16, 2019

View reviewed changes

poffdeluxe added 3 commits September 16, 2019 10:32

Changing plural format

8bb1efa

Merge branch 'master' into i18n-canvas-workpad-header-controls

85743bb

Quick fix

5f3f3a6

poffdeluxe added 2 commits September 16, 2019 11:15

Zoom controls

d14d639

Fixing id

08bd70e

Merge branch 'master' into i18n-canvas-workpad-header-controls

84017ff

clintandrewhall approved these changes Sep 16, 2019

View reviewed changes

poffdeluxe merged commit 54359e7 into elastic:master Sep 17, 2019

poffdeluxe deleted the i18n-canvas-workpad-header-controls branch September 17, 2019 18:13

poffdeluxe mentioned this pull request Sep 17, 2019

[7.x] [Canvas] Converting workpad header components to typescript and adding i18n (#45274) #45927

Merged

Conversation

poffdeluxe commented Sep 10, 2019

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Sep 10, 2019

Uh oh!

Uh oh!

elasticmachine commented Sep 10, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 10, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 10, 2019

💚 Build Succeeded

Uh oh!

clintandrewhall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clintandrewhall Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

poffdeluxe Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bamieh Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

clintandrewhall Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

poffdeluxe Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clintandrewhall Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

poffdeluxe Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 12, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 13, 2019

💚 Build Succeeded

Uh oh!

Bamieh left a comment

Choose a reason for hiding this comment

Uh oh!

Bamieh Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

Bamieh Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 16, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 16, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 16, 2019

💚 Build Succeeded

Uh oh!

clintandrewhall left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

poffdeluxe Sep 12, 2019 •

edited

Loading

poffdeluxe Sep 12, 2019 •

edited

Loading