Skip to content

Use injectI18n Higher-Order Component instead of I18nContext#20542

Merged
yankouskia merged 3 commits intoelastic:masterfrom
yankouskia:feature/inject-i18n-hoc
Jul 13, 2018
Merged

Use injectI18n Higher-Order Component instead of I18nContext#20542
yankouskia merged 3 commits intoelastic:masterfrom
yankouskia:feature/inject-i18n-hoc

Conversation

@yankouskia
Copy link

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@yankouskia yankouskia added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Project:i18n labels Jul 10, 2018
@azasypkin azasypkin removed the request for review from epixa July 11, 2018 16:46
@yankouskia yankouskia requested a review from LeanidShutau July 12, 2018 09:05
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@yankouskia yankouskia requested review from azasypkin and rhoboat and removed request for azasypkin and rhoboat July 12, 2018 11:29
Copy link
Contributor

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, just few nits, thanks!

Also please update PR title (and name of squashed commit) to reflect what is actually done in this PR.

wrap your components into `I18nContext` component. The child of this component
should be a function that takes `intl` object into parameters:

React wrapper provides an ability to inject the imperative formatting API into a React component via its props using `injectI18n` HOC. This should be used when your React component needs to format data to a string value where a React element is not suitable; e.g., a `title` or `aria` attribute. In order to use it you should wrap your component into `injectI18n` function. The formatting API will be provided to the wrapped component via `props.intl`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: HOC --> higher-order component (HOC) factory
nit: should wrap your component into `injectI18n` function --> should wrap your component with `injectI18n` factory


```js
import React from 'react';
import { ReactI18n } from '@kbn/i18n';
Copy link
Contributor

Choose a reason for hiding this comment

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

note: don't forget to update this example in #20513 (or after it's merged, up to you).

Copy link
Author

Choose a reason for hiding this comment

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

@azasypkin yes, believe it would be better to update this one after merging #20513

* />
* )}
* </I18nContext>
* HOC is used for injecting intl prop into wrapped component
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: same note about HOC vs HOC factory here as well.

@yankouskia yankouskia changed the title Propose injectI18n HOC Use injectI18n Higher-Order Component instead of I18nContext Jul 12, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@LeanidShutau LeanidShutau left a comment

Choose a reason for hiding this comment

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

LGTM. Message syntax is not affected, so no changes are needed to the extraction tool.

@yankouskia yankouskia merged commit 92774b7 into elastic:master Jul 13, 2018
@yankouskia yankouskia deleted the feature/inject-i18n-hoc branch July 13, 2018 09:22
LeanidShutau pushed a commit to LeanidShutau/kibana that referenced this pull request Aug 1, 2018
…#20542)

* add implementation of I18nContext, docs for injectI18n hoc

* remove i18nContext wrapper, add docs for react components as classes
LeanidShutau added a commit that referenced this pull request Aug 1, 2018
…#21529)

* add implementation of I18nContext, docs for injectI18n hoc

* remove i18nContext wrapper, add docs for react components as classes
@LeanidShutau
Copy link
Contributor

6.x/6.5: 8f7142f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported Project:i18n Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v6.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants