[i18n] support user defined component #5236

ym · 2021-10-04T19:12:09Z

Summary

We noticed that Chrome's built-in translation feature will break any page with EuiTable.

After investigation, I found that it's caused by the <Fragment> component used by EuiI18n and React or Chrome Trasnlate can't work together when we're using <Fragment> rather than a real DOM component here.

I see i18next has the option to use a different component and we can use the same approach in Eui to make a workaround. That's what the PR is for.

Reference:

[1] i18next/react-i18next#1254
[2] facebook/react#17256

Checklist

kibanamachine · 2021-10-04T19:12:10Z

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?

cla-checker-service · 2021-10-04T19:12:12Z

💚 CLA has been signed

thompsongl · 2021-10-04T21:13:02Z

👋 Thanks, @ym!
If you haven't already, please sign the CLA and comment here if/when done.

ym · 2021-10-04T21:13:41Z

👋 Thanks, @ym! If you haven't already, please sign the CLA and comment here if/when done.

Just signed :-)

thompsongl · 2021-10-05T13:54:30Z

jenkins test this

kibanamachine · 2021-10-05T14:24:01Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5236/

thompsongl

Makes sense and looks good!
Only change I'd like to see is a name change. We typically name custom render functions render, so let's use that name across the board here

+ render
- componentRenderFunc

cchaos · 2021-10-05T15:52:54Z

Let's also make sure we add a docs example under https://elastic.github.io/eui/#/utilities/i18n

cee-chen

Some small nit type comments, definitely not a blocker. Thanks for the contribution! ++ to Caroline's comments that it'd be great to see a documentation section for this with an example use case / usage. Let us know if that's something we can help with.

src/components/i18n/i18n.tsx

src/components/i18n/i18n.test.tsx

ym · 2021-10-05T19:46:54Z

Let's also make sure we add a docs example under https://elastic.github.io/eui/#/utilities/i18n

I've just added an example, please check and let me know if there's anything that needs to be improved.

thompsongl · 2021-10-05T21:02:33Z

jenkins test this

kibanamachine · 2021-10-05T21:20:33Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5236/

src-docs/src/views/i18n/context.js

cee-chen · 2021-10-05T22:56:23Z

src/components/context/context.tsx

@@ -26,6 +26,7 @@ export interface I18nShape {
    [key: string]: Renderable<object>;
  };
  mappingFunc?: (value: string) => string;
+  render?: (children: any[]) => FunctionComponent<any>;


I think we should document this fn here is a props docblock at the very minimum (or we could add an entire section to the src-docs view, but I tend to bias towards props docs myself).

Suggested change

render?: (children: any[]) => FunctionComponent<any>;

/**

* Some i18n frameworks don't work with a rendered `<Fragment>`.

* The `render` function allows you to pass in another wrapping node instead, e.g. `<div>`

*/

render?: (children: any[]) => FunctionComponent<any>;

The above is based on my understanding of this feature/PR, feel free to correct me if I'm wrong!

Suggested change

render?: (children: any[]) => FunctionComponent<any>;

render?: (children: any) => FunctionComponent;

@ym Heads up, it looks like most of Greg's Typescript suggestions need to be addressed still

thompsongl

I didn't fully understand the problem yesterday, so apologies for the double review.

See inline comments, but we'll need to make some changes to get this working and understandable. I think adding a new docs section is the right approach.

thompsongl · 2021-10-06T14:46:21Z

CHANGELOG.md

@@ -3,6 +3,8 @@
 - Added `maxWidth` prop to `EuiTour`, made `subtitle` optional, and fixed heading levels and footer background ([#5225](https://github.com/elastic/eui/pull/5225))
 - Updated `tint`, `shade`, `saturate`, `desaturate`, and `makeHighContrastColor` utility functions to maintain color format supplied ([#5230](https://github.com/elastic/eui/pull/5230))
 - Converted generated icon files to Typescript, eliminating the last `.js` files in our source files ([#5212](https://github.com/elastic/eui/pull/5212))
+- Added `render` prop to `EuiI18n` ([#5236](https://github.com/elastic/eui/pull/5236))


We had a release yesterday so this is now in the wrong section. Merge/rebase master and update

thompsongl · 2021-10-06T15:23:44Z

src-docs/src/views/i18n/context.js


  const i18n = {
    mapping: mappings[language],
    formatNumber: (value) => new Intl.NumberFormat(language).format(value),
+    render: useDiv
+      ? () => (children) => <div style={{ display: 'inline' }}>{children}</div>


Suggested change

? () => (children) => <div style={{ display: 'inline' }}>{children}</div>

? (children) => () => <div style={{ display: 'inline' }}>{children}</div>

Based on the types, this is how it'd work

thompsongl · 2021-10-06T15:24:11Z

src/components/context/context.tsx

@@ -26,6 +26,7 @@ export interface I18nShape {
    [key: string]: Renderable<object>;
  };
  mappingFunc?: (value: string) => string;
+  render?: (children: any[]) => FunctionComponent<any>;


Suggested change

render?: (children: any[]) => FunctionComponent<any>;

render?: (children: any) => FunctionComponent;

thompsongl · 2021-10-06T15:24:47Z

src/components/i18n/i18n.tsx

+  valueDefault: DEFAULT;
+  i18nMappingFunc?: (token: string) => string;
+  values?: I18nTokenShape<T, DEFAULT>['values'];
+  render?: (children: any[]) => FunctionComponent<any>;


Suggested change

render?: (children: any[]) => FunctionComponent<any>;

render?: I18nShape['render'];

src-docs/src/views/i18n/context.js

thompsongl · 2021-10-07T17:31:31Z

jenkins test this

thompsongl · 2021-10-07T19:49:28Z

jenkins test this

kibanamachine · 2021-10-07T20:17:35Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5236/

cee-chen

A few more small comments from me, I think we're really close! Thanks for your patience and the many iterations. As a heads up also we'll be OOO tomorrow, but I think this can get merged next week once remaining feedback is addressed :)

cee-chen · 2021-10-07T20:59:17Z

src-docs/src/views/i18n/context.js

 } from '../../../../src/components';

 const mappings = {
  fr: {
    'euiContext.english': 'Anglais',
    'euiContext.french': 'Française',
-    'euiContext.greeting': 'Salutations!',
+    'euiContext.greeting': 'Salutations, {name}!',


It looks like this isn't quite working as expected:

Should we just leave this as-is without the {name}?

render prop will be only called in component interpolation so I guess we have to keep {name} part.

src-docs/src/views/i18n/context.js

cee-chen · 2021-10-07T21:03:35Z

src/components/context/context.tsx

@@ -26,6 +26,7 @@ export interface I18nShape {
    [key: string]: Renderable<object>;
  };
  mappingFunc?: (value: string) => string;
+  render?: (children: any[]) => FunctionComponent<any>;


@ym Heads up, it looks like most of Greg's Typescript suggestions need to be addressed still

thompsongl · 2021-10-14T18:46:59Z

jenkins test this

kibanamachine · 2021-10-14T19:14:04Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5236/

…ic#5229) * Making an explicit check for ID prop in EUI Combobox * Checking for a user-passed ID prop * If no user-passed ID, we'll use the `rootId()` class method to set one * Prepend and append props need an ID to relate label to the input * Adding example for Combobox with prepended label * Confirmed the label `for` attribute is referencing input `id` * Added styles to round the edges of the child `<div>` when we prepend a label * Bumped package.json * Adding CHANGELOG entry and bumping version * Update combo_box.tsx * Updating styles to match other forms with prepend and apppend. * Finishing Combobox CSS and test updates * Simplified the Amsterdam overrides for prepended / appended labels * Added conditional classes to `combo_box.tsx` * Added unit tests for user passed ID, prepended and appended classes * Reordering props in child component. * Final adjustment to ID check in ComboBox * Final updates to styling and documentation * Added CSS rules for compressed, prepend and append border radius * Updated the Display Toggles to include append * Removed a duplicated instruction paragraph * Simplified the logic for a unique ID in combobox.ts * Adding Combobox unique ID to master changelog * Updated Form Control Layout SCSS * The borders were still rounded with prepend and append enabled * Added two more rules to make borders square for full height and compressed * Move to _combo_box.scss Co-authored-by: cchaos <caroline.horn@elastic.co>

@see

* Install Cypress code coverage dependencies * Set up required babel+istanbul config * Set up Cypress code coverage config @see https://github.com/cypress-io/code-coverage#instrument-unit-tests * Add script for combining cypress & jest code coverage -into a single json & html report * Misc cleanup/ignores - Ignore new `.nyc_output` dir generated by cypress/istanbul - Improve Jest config collectCoverageFrom !ignores: - DRY out component+services folder patterns with a {,} glob - Ignore new Cypress .spec files and .testenv files for jest coverage - eslintignore the cypress folder, since they're all .js anyway and it has a decent amount of idiosyncracies + cleanup unused eslint-disable + cleanup cypress comment boilerplate + remove unused paths from webpack config * Add wiki documentation * [PR feedback] Move nyc config to own file * [PR feedback] Switch combine coverage script to .sh

…c#5272) * Add remark-breaks and line break plugin to MarkdownEditor * Update changelog

elastic#5283) * Moved Border tokens to its own page * Figured out a way to get subsections in the sidenav working * Added the new `_values` under `customizing/` * [EuiTableRowCell] Actually support `valign` property manually * [EuiTable*] Fixing some props and mobile options * Slightly better responsive table styles

* omit unsupported types * resolve console errors * CL

…failures (elastic#5317) * Add Cypress failed test retries to CI run mode * Increase cy.wait on flaky context menu tests (just in case) * More hardening

* use useJsonVars * use variable

* Added Typography page * Deleting all old Sass typography

…unt values (elastic#5313) * Allow the datagrid to expand&contract in response to different row counts, when no pagination is present * changelog

… theme (elastic#5320) * Fix large density not increasing in fontSize on Amsterdam theme * Add changelog entry * Switch to Amsterdam override

…to` (elastic#5281) * Remove isHeightSame check * Remove same height check from shouldComponentUpdate - if we're not checking for sameHeight in componentDidUpdate, we also likely don't need this anymore, and all dynamic user changes appear to work without this code * [Proposed refactors] - Rename `recalculateRowHeight` to `setAutoRowHeight` - DRY out setAutoRowHeight to be reusable by both the resize observer and during props update (NB: This leads to some repetition with the height being obtained twice, but will not be an issue shortly as I'll be refactoring/removing the cell wrapper observer in a future PR) - Refactor rowHeightUtils.isAutoHeight to accept an undefined rowHeightsOptions * Improve unit tests * Add changelog entry * [PR feedback] Fix `isAutoHeight` false positive - occurs if the `defaultHeight` is auto but a specific `rowHeights` line override is not auto TODO: Write a unit test for this in the future * [PR feedback/Discover testing] Trigger component update when rowHeightsOptions changes * [cleanup] Remove now-unused compareHeights util + clean up mock rowHeightUtils as well * [revert] rename setAutoRowHeight back to recalculateRowHeight - lineCount in elastic#5284 will shortly be using the row height cache in addition to auto, so I'm making the fn name less specific again * Add enqueue/timeout to recalculateRowHeight updates - to avoid race conditions with cache being cleared * Remove clearHeightsCache completely * Unset row+column height values on unmount (elastic#3) * Add unit test for new unsetRowHeight + add optional isAutoHeight check * Remove hidden columns from row height cache * Early return in getRowHeight Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

* add euiautosizer * use euiautosizer * use euiautosizer testenv mock * CL * docs * props table * simplify wrapper * add data-eui attr to testenv mock

* Added Sizing page * Add subsections and use new _values * Deleting all old Sass typography * Added callout in Sass page and easier customization of ValuesTable columns * Cleanup including adding `valign` as a top level configuration

thompsongl · 2021-10-29T14:52:45Z

My PR idea didn't work. It was my fault so I've opened a new PR with a clean git state: #5337

ym force-pushed the master branch from 775b3fa to 1381fcf Compare October 4, 2021 21:39

thompsongl suggested changes Oct 5, 2021

View reviewed changes

[i18n] support user defined component

a5a542a

cee-chen reviewed Oct 5, 2021

View reviewed changes

src/components/i18n/i18n.tsx Outdated Show resolved Hide resolved

src/components/i18n/i18n.tsx Outdated Show resolved Hide resolved

src/components/i18n/i18n.test.tsx Outdated Show resolved Hide resolved

ym force-pushed the master branch 3 times, most recently from 72515ad to 49d238e Compare October 5, 2021 19:27

[EuiI18n] Move arguments of lookupToken to lookupTokenOptions

21efaa8

ym force-pushed the master branch from 49d238e to 21efaa8 Compare October 5, 2021 19:28

cee-chen reviewed Oct 5, 2021

View reviewed changes

thompsongl suggested changes Oct 6, 2021

View reviewed changes

ym force-pushed the master branch from cc34cc8 to 7a34453 Compare October 6, 2021 15:44

cee-chen reviewed Oct 7, 2021

View reviewed changes

ym force-pushed the master branch from 7a34453 to 3dad7e2 Compare October 13, 2021 00:03

[EuiI18n] Add documents on how to use i18n.render

763c496

1Copenut and others added 26 commits October 29, 2021 10:24

update i18ntokens

648345e

40.0.0

d5cee9c

Updated documentation.

19e069e

[EuiMarkdownEditor] Add remark-breaks and line break plugin (elasti…

cdbbf80

…c#5272) * Add remark-breaks and line break plugin to MarkdownEditor * Update changelog

Fix CL from elastic#5272

394529c

[Docs] Update EuiDatePicker types (elastic#5318)

5ef7599

* omit unsupported types * resolve console errors * CL

[Cypress] Add flakey test retries + harden intermittent context menu …

d95b1da

…failures (elastic#5317) * Add Cypress failed test retries to CI run mode * Increase cy.wait on flaky context menu tests (just in case) * More hardening

[Docs] Fix Colors guidelines (elastic#5316)

cff0a2f

* use useJsonVars * use variable

Update dependency @elastic/charts to ^38.1.0 (elastic#5321)

d04941e

[Docs] Separated out Typography to its own page (elastic#5300)

a8c8fb7

* Added Typography page * Deleting all old Sass typography

[EuiDataGrid] Allow the grid to expand & contract with changing rowCo…

f4c9cc3

…unt values (elastic#5313) * Allow the datagrid to expand&contract in response to different row counts, when no pagination is present * changelog

[EuiDataGrid] Fix large density not increasing font size on Amsterdam…

5640271

… theme (elastic#5320) * Fix large density not increasing in fontSize on Amsterdam theme * Add changelog entry * Switch to Amsterdam override

Updating Renovate spec for "major" only

9fd21b5

Fix changelog location

cdc18de

40.1.0

8b95187

Updated documentation.

6636fb3

v39.1.2 changelog

edffcfd

[EuiAutoSizer] AutoSizer wrapper and testenv mock (elastic#5278)

b9e962b

* add euiautosizer * use euiautosizer * use euiautosizer testenv mock * CL * docs * props table * simplify wrapper * add data-eui attr to testenv mock

docs

b8b66a8

CL

0d13d53

type updates

3abd616

thompsongl mentioned this pull request Oct 29, 2021

[i18n] Support user defined component #5337

Merged

7 tasks

thompsongl closed this Oct 29, 2021

thompsongl mentioned this pull request Nov 2, 2021

Upgrade EUI to v41.0.0 elastic/kibana#117242

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[i18n] support user defined component #5236

[i18n] support user defined component #5236

ym commented Oct 4, 2021 •

edited

Loading

kibanamachine commented Oct 4, 2021

cla-checker-service bot commented Oct 4, 2021 •

edited

Loading

thompsongl commented Oct 4, 2021

ym commented Oct 4, 2021

thompsongl commented Oct 5, 2021

kibanamachine commented Oct 5, 2021

thompsongl left a comment

cchaos commented Oct 5, 2021

cee-chen left a comment

ym commented Oct 5, 2021 •

edited

Loading

thompsongl commented Oct 5, 2021

kibanamachine commented Oct 5, 2021

cee-chen Oct 5, 2021 •

edited

Loading

thompsongl Oct 6, 2021

cee-chen Oct 7, 2021 •

edited

Loading

thompsongl left a comment

thompsongl Oct 6, 2021

thompsongl Oct 6, 2021

thompsongl Oct 6, 2021

thompsongl Oct 6, 2021

thompsongl commented Oct 7, 2021

thompsongl commented Oct 7, 2021

kibanamachine commented Oct 7, 2021

cee-chen left a comment •

edited

Loading

cee-chen Oct 7, 2021

ym Oct 13, 2021

cee-chen Oct 7, 2021 •

edited

Loading

thompsongl commented Oct 14, 2021

kibanamachine commented Oct 14, 2021

thompsongl commented Oct 29, 2021

	render?: (children: any[]) => FunctionComponent<any>;
	render?: (children: any) => FunctionComponent;

	? () => (children) => <div style={{ display: 'inline' }}>{children}</div>
	? (children) => () => <div style={{ display: 'inline' }}>{children}</div>

	render?: (children: any[]) => FunctionComponent<any>;
	render?: I18nShape['render'];

[i18n] support user defined component #5236

[i18n] support user defined component #5236

Conversation

ym commented Oct 4, 2021 • edited Loading

Summary

Checklist

kibanamachine commented Oct 4, 2021

cla-checker-service bot commented Oct 4, 2021 • edited Loading

thompsongl commented Oct 4, 2021

ym commented Oct 4, 2021

thompsongl commented Oct 5, 2021

kibanamachine commented Oct 5, 2021

thompsongl left a comment

Choose a reason for hiding this comment

cchaos commented Oct 5, 2021

cee-chen left a comment

Choose a reason for hiding this comment

ym commented Oct 5, 2021 • edited Loading

thompsongl commented Oct 5, 2021

kibanamachine commented Oct 5, 2021

cee-chen Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

thompsongl Oct 6, 2021

Choose a reason for hiding this comment

cee-chen Oct 7, 2021 • edited Loading

Choose a reason for hiding this comment

thompsongl left a comment

Choose a reason for hiding this comment

thompsongl Oct 6, 2021

Choose a reason for hiding this comment

thompsongl Oct 6, 2021

Choose a reason for hiding this comment

thompsongl Oct 6, 2021

Choose a reason for hiding this comment

thompsongl Oct 6, 2021

Choose a reason for hiding this comment

thompsongl commented Oct 7, 2021

thompsongl commented Oct 7, 2021

kibanamachine commented Oct 7, 2021

cee-chen left a comment • edited Loading

Choose a reason for hiding this comment

cee-chen Oct 7, 2021

Choose a reason for hiding this comment

ym Oct 13, 2021

Choose a reason for hiding this comment

cee-chen Oct 7, 2021 • edited Loading

Choose a reason for hiding this comment

thompsongl commented Oct 14, 2021

kibanamachine commented Oct 14, 2021

thompsongl commented Oct 29, 2021

ym commented Oct 4, 2021 •

edited

Loading

cla-checker-service bot commented Oct 4, 2021 •

edited

Loading

ym commented Oct 5, 2021 •

edited

Loading

cee-chen Oct 5, 2021 •

edited

Loading

cee-chen Oct 7, 2021 •

edited

Loading

cee-chen left a comment •

edited

Loading

cee-chen Oct 7, 2021 •

edited

Loading