Skip to content
This repository was archived by the owner on Jan 16, 2022. It is now read-only.

feat(i18n): added i18next for user interface translations #432

Merged
priscilawebdev merged 4 commits intomasterfrom
feature/102_added_i18next
Mar 8, 2020
Merged

feat(i18n): added i18next for user interface translations #432
priscilawebdev merged 4 commits intomasterfrom
feature/102_added_i18next

Conversation

@priscilawebdev
Copy link
Copy Markdown
Contributor

@priscilawebdev priscilawebdev commented Feb 2, 2020

Type: Feat

The following has been addressed in the PR: #102

  • Unit or Functional tests are included in the PR:
    Yes they are fixed, but it seems that I need to fix e2e tests now

@priscilawebdev priscilawebdev changed the title WIP: feat(i18n): added i18next feat(i18n): added i18next Mar 2, 2020
Copy link
Copy Markdown
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

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

Great stuff @priscilawebdev , looking forward we can test it with Verdaccio core.

Screen Shot 2020-03-03 at 7 49 22 AM

I've notice few strings are not translated, but nothing big :)

</WrapperLink>
</Grid>
<GridRightAligned item={true} xs={true}>
<GridRightAligned alignItems="center" container={true} item={true} justify="flex-end" xs={true}>
Copy link
Copy Markdown
Member

@juanpicado juanpicado Mar 3, 2020

Choose a reason for hiding this comment

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

Does this PR requires change this alignItems="center" container={true} item={true} justify="flex-end" xs={true}>?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes! it was necessary to do this changes

@@ -1,14 +0,0 @@
/* eslint-disable max-len */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this is being removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I may be mistaken, but I couldn't find where it is being used

}));

jest.mock('i18next', () => {
const translationEN = require('../../i18n/translations/en-US.json');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this? I don't see the connection with the test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in fact they have a connection ... I'm using the English translation file to do the tests ... I didn't want to write consts or a string ... I think it's better if we reuse it ... so let's say I don't I want more "Login", but "Sign in" .. if the id's remained the same (en translations) and only the value changes, we don't need to update the tests

});

new WebpackDevServer(compiler, {
contentBase: `${env.DIST_PATH}`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Copy Markdown
Contributor Author

@priscilawebdev priscilawebdev Mar 8, 2020

Choose a reason for hiding this comment

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

we don't need to wrap it in a string literal. it's the same but without ``

@priscilawebdev priscilawebdev force-pushed the feature/102_added_i18next branch from 7366660 to 4467be9 Compare March 8, 2020 09:31
@priscilawebdev
Copy link
Copy Markdown
Contributor Author

priscilawebdev commented Mar 8, 2020

@juanpicado yes, some words are not being translated, because either they are come from backend as a value, like "JS foundation and contributors", or from a library, '2 years ago' and I need to fix it ...please see:

image

@priscilawebdev priscilawebdev changed the title feat(i18n): added i18next WIP:feat(i18n): added i18next Mar 8, 2020
@juanpicado
Copy link
Copy Markdown
Member

@juanpicado yes, some words are not being translated, because either they are come from backend as a value, like "JS foundation and contributors", or from a library, '2 years ago' and I need to fix it ...please see:

image

ok, if comes from the backend, we will fix that later, thanks for letting me know :)

@priscilawebdev priscilawebdev force-pushed the feature/102_added_i18next branch from 4467be9 to 76e8a02 Compare March 8, 2020 13:50
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 8, 2020

Codecov Report

Merging #432 into master will decrease coverage by 0.37%.
The diff coverage is 79.38%.

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
- Coverage   88.45%   88.08%   -0.38%     
==========================================
  Files         137      140       +3     
  Lines         953      982      +29     
  Branches      196      204       +8     
==========================================
+ Hits          843      865      +22     
- Misses         99      106       +7     
  Partials       11       11
Impacted Files Coverage Δ
src/utils/test-enzyme.ts 100% <ø> (ø) ⬆️
src/utils/login.ts 100% <ø> (ø) ⬆️
src/utils/constants.ts 100% <ø> (ø) ⬆️
src/components/Link/Link.tsx 100% <ø> (ø) ⬆️
...mponents/DetailSidebar/DetailSidebarFundButton.tsx 100% <ø> (ø) ⬆️
src/components/Author/styles.ts 100% <ø> (ø) ⬆️
...components/DetailContainer/DetailContainerTabs.tsx 100% <100%> (ø) ⬆️
src/components/NotFound/NotFound.tsx 100% <100%> (ø) ⬆️
src/components/Header/HeaderMenu.tsx 100% <100%> (ø) ⬆️
src/components/LoginDialog/LoginDialogHeader.tsx 83.33% <100%> (+3.33%) ⬆️
... and 38 more

@priscilawebdev priscilawebdev force-pushed the feature/102_added_i18next branch from 76e8a02 to a0c0186 Compare March 8, 2020 13:54
@priscilawebdev priscilawebdev changed the title WIP:feat(i18n): added i18next feat(i18n): added i18next Mar 8, 2020
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 8, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@verdacciobot
Copy link
Copy Markdown

Thanks for your PR, the @verdaccio/ui package will be accessible from here for testing purposes:

npm install @verdaccio/ui-theme@v0.3.14-0b29d84-pr432.0 --registry https://registry.verdaccio.org

Copy link
Copy Markdown
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

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

Marvelous 🎏 👏

@priscilawebdev priscilawebdev changed the title feat(i18n): added i18next feat(i18n): added i18next for user interface translations Mar 8, 2020
@priscilawebdev priscilawebdev merged commit 7428384 into master Mar 8, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/102_added_i18next branch March 8, 2020 15:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants