feat(DeleteResource): Add wording & i18n #1243
Conversation
|
Could you add this command on the package.json as the other i18n packages. We have a issue with the female context where the key is a variable so the parser can't find it. |
1 similar comment
| @@ -2,15 +2,18 @@ import React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import { componentState } from '@talend/react-cmf'; | |||
| import { ConfirmDialog } from '@talend/react-components'; | |||
| import { translate, Trans } from 'react-i18next'; // eslint-disable-line import/no-extraneous-dependencies | |||
There was a problem hiding this comment.
You should not disable this rule.
It highlight an issue in your dependencies. You have (react-)i18next as dev deps, but that's wrong. You should either add it as peer dependencies or move to to dependencies.
packages/containers/src/translate.js
Outdated
| @@ -0,0 +1,5 @@ | |||
| import { createInstance } from 'i18next'; // eslint-disable-line import/no-extraneous-dependencies | |||
There was a problem hiding this comment.
When you fix the dependencies, you can remove that
| @@ -1,8 +1,19 @@ | |||
| import i18n from 'i18next'; | |||
| import React from 'react'; | |||
| import i18n from 'i18next'; // eslint-disable-line import/no-extraneous-dependencies | |||
There was a problem hiding this comment.
When you fix the dependencies, you can remove that
| return ( | ||
| <ConfirmDialog | ||
| show | ||
| header={this.props.header} | ||
| cancelAction={cancelAction} | ||
| validateAction={validateAction} | ||
| > | ||
| <div>{resourceInfo.label}</div> | ||
| <div> | ||
| <Trans i18nKey={i18nKey}> |
There was a problem hiding this comment.
2 things on i18n :
- add npm scripts to extract translation keys in root package.js
- this trans is not compatible with i18next-parser, I won't block this PR for that since it is really needed and the solution is the good one I think, but we need a solution
There was a problem hiding this comment.
@jsomsanith The next major version of i18next-parser will be compatible with Trans. I work on i18n on the next sprint so I like to update this package to manage it.
There was a problem hiding this comment.
- The npm script is on the container package.json
- Currently yes, there is some work here on this side -> Improved parsing of Trans component i18next/i18next-parser#85
For now, i think this is still the best way to get the translations with html markup.
There is another issue currently, as the Trans component manage to make it work the count attribute but the "context" attribute to declare male or female translation is not working as it is. I try to make a PR on this point in react-i18next ;)
There was a problem hiding this comment.
- it should be added to the root
package.jsonlike the others, so thatyarn extract-i18nextract all packages translations in a single folder. Take a look at root npm scripts
1 similar comment
1 similar comment
Yep, it's back !
What is the problem this PR is trying to solve?
There is just the label of the entity showned
What is the chosen solution to this problem?
Add a sentense ( and translate it )
Please check if the PR fulfills these requirements
[ ] This PR introduces a breaking change