Use i18n Keys
End-user friendly description of the problem this fixes or functionality that this introduces
Right now, several instances of plain English language are used in the codebase. Using i18 keys would ensure better inclusivity by enabling users to use different languages.
Link of any specific issues this addresses
Hi @amanape
Does this change seem fine?
If so, I'll make similar changes to the rest of the repo.
Yes, that's the correct way to do it. Don't worry about trying to get the other translations in though, you can focus on English only for now.
I recommend converting this PR to a draft if you're thinking of continuing
Hi @Vaishakh-SM @amanape , just some questions, to start helping on the process
1. do you think its best to keep all the translations in one file?
As a positive I see everithing is centralized and as a negative the file might grow too large.
2. If so, do you think its worth grouping the elements on the Json structure itself?
I see currently the resources are prefixed, so for example CODE_EDITOR$FILE_SAVED_SUCCESSFULLY, CODE_EDITOR$SAVING_LABEL, CODE_EDITOR$SAVE_LABEL. Another approach used in previews projects I've seen is to group things, for example:
{
"CODE_EDITOR": {
"SAVE_LABEL": {
"en": "Save",
"zh-CN": "保存",
}
}
}
Using the resource would be similar using dot notation t(I18nKey.CODE_EDITOR.SAVE_LABEL)
I understand if probably the better choice is to work as it is, because this change is already big enough and changing the standard would require modify the parts that are already translated; I just wanted to bring it up in case you think something is worth changing before starting.
3. What would be the standard to leave the resources pending to be translated?
If you leave the key missing then the system will take the resource of the default language (English) which I think its a good behavior, but leaving the key missing could also reduce visibility for the pending translations
4. How could I support without generating git conflicts?
This one goes more to you @Vaishakh-SM, Im sorry, I have never co-authored a PR here on github, so, how would be the better way? do I push changes to the same branch? do I create a new branch and PR changes to yours, or how is it better to proceed?. Because we would be modifying the same file I don't want to create git conflicts
Thanks
Hey @danicruz0415, thanks for your input. To answer your questions:
- do you think its best to keep all the translations in one file?
No, I do not. The recommended way in the official documentation would be to have a translations file per language. (e.g., en.json contains the English translations, de.json contains the German translations, etc). I definitely think we should look for ways to break down the translation files for the future.
- If so, do you think its worth grouping the elements on the Json structure itself?
Yes, that is a good alternative and I can see it improve DX for the time being. As a side note, since the major UI changes, there are actually a lot of translations that are no longer required, but left over. In reality, there are only a few keys currently being used.
- What would be the standard to leave the resources pending to be translated?
As is. IF we had individual translation files, it would be easy for other individuals to extend it.
- How could I support without generating git conflicts?
I think one strategy would be to agree on certain routes/components to work on, on separate PRs. There will be some conflict since some routes re-use certain components, but it would expected and shouldn't be too difficult to resolve. You can see a basic map of the FE here:
https://github.com/All-Hands-AI/OpenHands/tree/main/frontend#project-structure
Thanks a lot for taking this up! Feel free to open another PR mentioning the routes/components you plan to cover
The strategy to avoid conflicts sounds good to me!
@danicruz0415 I'll take up the routes that you aren't once you open up your PR.
Hi! I've opened this PR: https://github.com/All-Hands-AI/OpenHands/pull/4464 Sorry for the late response I was a bit bussy; I also had an error running the interface locally.
@Vaishakh-SM if its ok by you I can take the "components/modals" folder @amanape can you take a look at the PR to see if it makes sense? sorry I know its a trivial change but I want to be sure 🙈
I leave the error that I was having, and the solution in case someone stumbles upon it, as googling it did not show many results
'VITE_MOCK_API' is not recognized as an internal or external command
It seems that this syntax where you set up an environment variable before running the command is not recognized by windows PowerShell (see this SO Question). The solution is to remove the env variable (change the "dev" script inside the package.json to "dev": "npm run make-i18n && remix vite:dev" and then use a ".env" file to set the VITE_MOCK_API variable to false
@amanape do you think this is worth mentioning on the troubleshoot section of the readme? It might be that is a trivial error just that I am not too familiar with it, so it took me a little to figure it out jeje
@Vaishakh-SM if its ok by you I can take the "components/modals" folder
Sure! Do you plan on taking up any other folders?
Hi @Vaishakh-SM sorry, I wasn't sure which ones needed translation so I mentioned the one I was testing with 🙈
I did a review of the code of all the folders inside the components folder and found the following
🟦 means does not need localization 🟩 means its already localized 🟥 means needs localization
- 📁 buttons 🟦
- 📁 chat 🟩
- 📁 context-menu 🟦
- 📁 file-explorer 🟩
- 📁 form 🟥
- 📁 markdown 🟦
- 📁 modals 🟥
- 📁 project-menu 🟥
- 📁 terminal 🟦
- ⚛️ account-settings-context-menu.tsx🟥
- ⚛️ AgentControlBar.tsx🟥
- ⚛️ AgentStatusBar.tsx 🟩
- ⚛️ Browser.tsx 🟥
- ⚛️ container.tsx 🟥
- ⚛️ controls.tsx 🟦
- ⚛️ editor-actions.tsx 🟥
- ⚛️ error-toast.tsx 🟥
- ⚛️ FileIcons.tsx 🟦
- ⚛️ FolderIcon.tsx 🟦
- ⚛️ IconButton.tsx 🟦
- ⚛️ Jupyter.tsx 🟥
- ⚛️ suggestion-bubble.tsx 🟦
- ⚛️ user-avatar.tsx 🟥
If its ok by you I can take the folders ./form ./modals and ./project-menu and you take the files outside the folders. Im not sure if its around the same number of files but I think it should be close >.<
I am not sure if anything outside the components folder needs any localization. As far as I saw it didn't.
If its ok by you I can take the folders
./form./modalsand./project-menuand you take the files outside the folders. Im not sure if its around the same number of files but I think it should be close >.<
Thanks for the hard work of categorizing the files!
Sorry for the late response. Yes, this seems fine to me. I'll start working on the files outside these folders.
Hello @Vaishakh-SM just checking in to see if this is generally on your radar still.
Hello @Vaishakh-SM just checking in to see if this is generally on your radar still.
Hi! Sorry for the delay. Yes this is still under my radar. I've made the changes required on all the other files but wanted to take a second look before raising the PR. I'm hoping to do this sometime in the coming week.
@Vaishakh-SM if you need any help or a review let me know! 😄
@Vaishakh-SM if you need any help or a review let me know! 😄
Hey Dani! It seems like some checks are failing because the tests look for alt / text of elements. Did you also face this, if so did you also update the tests?
Hey @Vaishakh-SM, there have been a significant amount of app-wide changes and will be a few more by the end of this week. Your branch is out of date and requires these conflicts to be resolved. Unfortunately, since it is a branch on your fork, nobody but you can make write changes to them.
If you're struggling with resolving the conflicts, I would recommend opening a new branch that is synced with the latest changes.
Hey @Vaishakh-SM, there have been a significant amount of app-wide changes and will be a few more by the end of this week. Your branch is out of date and requires these conflicts to be resolved. Unfortunately, since it is a branch on your fork, nobody but you can make write changes to them.
If you're struggling with resolving the conflicts, I would recommend opening a new branch that is synced with the latest changes.
Hi @amanape , I've opened PR#5286 which is synced with latest main. Still seems like some checks are failing (if I understand, this is because some tests try to find the element from the alt texts) but I think we can continue the discussion in that PR (If so I'll close this PR)?
The tests are failing because the text content has changed. This is expected since you're changing hardcoded values to dynamic ones with the help of the i18n library. It is safe to close this PR