Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore/lw 10534 translation package #1154

Merged
merged 10 commits into from
May 17, 2024
Merged

Conversation

vetalcore
Copy link
Contributor

@vetalcore vetalcore commented May 11, 2024

Checklist

  • JIRA -LW-10534
  • Proper tests implemented
  • Screenshots added.

Proposed solution

Move all the translations files and i18n instance into the separate package.

Testing

All the translations should work the same as before.

Screenshots

Attach screenshots here if implementation involves some UI changes

@vetalcore vetalcore self-assigned this May 11, 2024
@vetalcore vetalcore requested review from a team as code owners May 11, 2024 10:06
@vetalcore vetalcore force-pushed the chore/lw-10534-translation-package branch 2 times, most recently from 07a6a36 to 1940227 Compare May 11, 2024 10:08
Copy link

github-actions bot commented May 11, 2024

Allure Report

allure-report-publisher generated test report!

smokeTests: ✅ test report for e7e4d1e4

passed failed skipped flaky total result
Total 30 0 0 0 30

Copy link
Contributor

@tommayeliog tommayeliog left a comment

Choose a reason for hiding this comment

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

LGTM!

@mchappell
Copy link
Collaborator

@vetalcore packages/e2e-tests/src/utils/translationService.ts will need updating to point at the correct browser-extension-wallet and core translation files, this should get the e2e tests running

@vetalcore vetalcore requested a review from a team as a code owner May 13, 2024 12:42
@vetalcore vetalcore force-pushed the chore/lw-10534-translation-package branch from 71f6fea to 5bf0ab2 Compare May 13, 2024 12:43
Copy link
Collaborator

@mchappell mchappell left a comment

Choose a reason for hiding this comment

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

Once storybook changes have been corrected, happy to pass this on, looks like that's the only remaining change left. Great work 🎉

@vetalcore vetalcore force-pushed the chore/lw-10534-translation-package branch 2 times, most recently from a33021c to 21948b6 Compare May 14, 2024 09:24
@vetalcore vetalcore force-pushed the chore/lw-10534-translation-package branch from 1a0625a to e200968 Compare May 14, 2024 10:58
@vetalcore vetalcore force-pushed the chore/lw-10534-translation-package branch from e200968 to 247d393 Compare May 14, 2024 11:30
Copy link
Collaborator

@mchappell mchappell left a comment

Choose a reason for hiding this comment

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

Great job on the refactor, much easier to have all this in one place. Did you ever consider moving all translations into a single file?

@DominikGuzei
Copy link
Member

Yeah i also wonder why the translations are still split up 🤔 since they are already namespaced it would make sense to merge them? (although we would have to namespace some of the extension translations i think)

import styles from './SettingsLayout.module.scss';
import { useTranslation } from 'react-i18next';
Copy link
Member

Choose a reason for hiding this comment

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

im not sure if that's the correct import?

@vetalcore
Copy link
Contributor Author

vetalcore commented May 15, 2024

Yeah i also wonder why the translations are still split up 🤔 since they are already namespaced it would make sense to merge them? (although we would have to namespace some of the extension translations i think)

it's just easier to handle the changes this way, we could easily implement that as a next iteration.

Copy link

@vetalcore vetalcore merged commit 160be35 into main May 17, 2024
15 of 16 checks passed
@vetalcore vetalcore deleted the chore/lw-10534-translation-package branch May 17, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants