Conversation
There was a problem hiding this comment.
Pull request overview
Introduces i18next/react-i18next-based internationalisation into the shared common-frontend package and starts migrating the Disclaimer UI to use translated strings.
Changes:
- Add i18next + browser language detection + react-i18next dependencies and an i18n initialization module with embedded EN/HU resources.
- Update
Disclaimerto uset()/<Trans />for some disclaimer strings. - Adjust
tsupconfig to avoid bundling node_modules dependencies.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/common-frontend/tsup.config.ts | Configures tsup to keep node_modules deps external (imported) during bundling. |
| packages/common-frontend/src/common/i18n.ts | Adds i18next initialization and embedded EN/HU translation resources. |
| packages/common-frontend/src/client/components/Disclaimer.tsx | Migrates parts of the Disclaimer UI from dictionary to i18next translations. |
| packages/common-frontend/package.json | Adds i18next/react-i18next dependencies to the common frontend package. |
| package.json | Adds i18next/react-i18next dependencies at the repo root. |
| package-lock.json | Locks new i18n dependencies and their transitive deps. |
| apps/offline-frontend/src/App.tsx | Imports the i18n init module as a side-effect. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fraknoiadam Huge-huge sorry! I forgot to make this PR to a draft 😢 😳 I just wanted to make it available to follow my progress, but I'm far from doing anything mergable... |
…project's whole scope, finish localising Disclaimer and Header (added a language switcher as well)
… names, improve mobile language selector, rework colour endrelaytable a bit
…through the i18n object + use empty error translation for no login code
|
Oh, it seems I messedc up with something, and reverting them won't work even 😭 |
There was a problem hiding this comment.
It is a very good PR. This is a very hard task, but you managing it very well. I added some comments. Didn't check every file yet, especially not the language selector.
In online round we may don't want to enable multiple languages, only allow Hungarian. I don't know yet how that one could be resolved. Maybe it is better if the language selector is a different PR, and now hard code it to use Hungarian language. But this is just an idea, not a preference from my side.
fraknoiadam
left a comment
There was a problem hiding this comment.
I don't know whether you were waiting for reviews, but I had some time, so I made short review.
Also, please don't close the discussions. I will check whether they are implemented and close them.
| {description} | ||
| <strong>Tudnivalók: </strong>Az "Új próbajáték kezdése" gombra kattintva próbajáték indul, ami a pontozásba nem számít bele. Bátran kérjetek próbajátékot, hiszen ezzel tudjátok tesztelni, hogy jól értitek-e a játék működését. Az "Új éles játék kezdése" gombra kattintva indul a valódi játék, ami már pontért megy. | ||
|
|
||
| <strong>{t('strategy:instructions')}</strong>{t('strategy:instructionDescription')} |
There was a problem hiding this comment.
This one can be also in one t function. You can use formatting instead in the i18n.
There was a problem hiding this comment.
Actually, I disagree, since these two strings are separated visually enough, and also, the header string might be reused somewhere else (I might even wrap the header into a different element)
| .min(0, 'A válasz 0 és 9999 között van') | ||
| .max(9999, 'A válasz 0 és 9999 között van') | ||
| .required('Nem írtál semmi választ!') | ||
| .integer(t('error:integer')) |
There was a problem hiding this comment.
I would write relay.error.integer
There was a problem hiding this comment.
I see your point from one point of view. But on the other hand, I think it shouldn't be relay specific. And I'd collect all errors under error.* since I think we should group messages in a way to have larger groups.
packages/common-frontend/src/client/components/ExcerciseForm.tsx
Outdated
Show resolved
Hide resolved
…x and one namespace in translation resources, remove redundant dependencies from root level package.json, define TS version for VS Code, add missing translation and rework string grouping, refrain from using dangerouslySetInnerHTML with <Trans />
…oad directly into JS from it, packaga.json was only reordered)
Oh, sorry, didn't read your message through carefully enough. I resolve comments, if I implemented them in order to be free from them in VS Code editor, they're using a lot of space -- still if I've dealt with them already. But I'll stop doing this practice then of course. 🙇 |
| const teamstateString = localStorage.getItem(LOCAL_STORAGE_TEAMSTATE); | ||
| if (teamstateString === null) { | ||
| throw new Error('Váratlan hiba történt'); | ||
| throw new Error(i18n.t('unexpected')); |
There was a problem hiding this comment.
| throw new Error(i18n.t('unexpected')); | |
| throw new Error(i18n.t('error.unexpected')); |

Default view
Mobile view
Tasks