DESENG-611: Splitting translations based on language and common #2525
Conversation
Its because of the TODO Statements to replace tenant specific values from database which will done in another story |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2525 +/- ##
==========================================
- Coverage 76.04% 76.02% -0.03%
==========================================
Files 617 617
Lines 22093 22095 +2
Branches 1686 1688 +2
==========================================
- Hits 16801 16798 -3
- Misses 5028 5032 +4
- Partials 264 265 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
met-web/src/App.tsx
Outdated
| @@ -137,17 +138,17 @@ const App = () => { | |||
|
|
|||
| const getTranslationFile = async (languageId: string) => { | |||
| try { | |||
| const translationFile = await import(`./locales/${languageId}/${tenant.id}.json`); | |||
| const translationFile = await import(`./locales/${languageId}.json`); | |||
There was a problem hiding this comment.
Again, here you might expect languageId to only correspond to a language, but now it can also be the word "common". I think we need to update variable names to account for this, or possibly restructure.
There was a problem hiding this comment.
Changed to localeId
met-web/src/App.tsx
Outdated
| const supportedLanguages = Object.values(Languages); | ||
| const supportedLanguages: string[] = Object.values(Languages); | ||
| // Adding keyword `common` into supportedLanguages to fetch locale/common.json | ||
| supportedLanguages.push('common'); |
There was a problem hiding this comment.
In reading this code, one would expect supportedLanguages to be a list of languages. I think adding the common entry could be confusing, although I can see that it simplifies the work a little bit.
For readability, I'd recommend either renaming Languages to LocaleFiles or some other solution that can make things a bit easier to understand.
There was a problem hiding this comment.
Separated the code to load a common translation file
met-web/src/hooks.ts
Outdated
| const translate = useTranslation(); | ||
| const tenantId = sessionStorage.getItem('tenantId'); | ||
| // Every language has its own default and common namespaces | ||
| const translate = useTranslation(['default', 'common']); | ||
|
|
||
| const { t } = translate; | ||
|
|
There was a problem hiding this comment.
The language system in MET is inherently a bit confusing. The comments in this file are great so far, and maybe we could add a higher-level one above this function. Would this comment describe the intended behaviour?
| /** | |
| * Look to see if a string has a tenant-level translation (if it's related to an engagement, survey, widget, static | |
| * tenant text, etc.). If none is found, look for a common static text translation found in the common locale file. | |
| **/ |
There was a problem hiding this comment.
Updated the comments
|
Issue #: https://apps.itsm.gov.bc.ca/jira/browse/DESENG-611
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).