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

feat: Add i18n #84

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Add i18n #84

wants to merge 2 commits into from

Conversation

curtisy1
Copy link

This adds react-i18next and a dropdown to dynamically toggle the language anytime.

I also added an English translation which is based on Google Translate since I'm not Chinese and don't speak the language. Should be better than nothing though, with way for improvement by the community.

English is the default language for the translation keys, so new developers will have an easier time reading the code. If you would like the keys to be Chinese instead, I can change it back.

As a bonus, I also fixed the unit tests. Some of them were broken due to the i18n adjustments, while most probably didn't work in the first place due to cloudreveApp not being called correctly.

This adds react-i18next and a dropdown to dynamically toggle the language anytime.

I also added an English translation which is based on Google Translate since I'm not Chinese and don't speak the language. Should be better than nothing though, with way for improvement by the community
Not sure if all of them broke because of the i18n stuff. But simply mocking the i18n and the connected-react-router component did the trick
@HFO4
Copy link
Member

HFO4 commented Oct 10, 2021

Wow, Thanks! This is a really big effort!
I will take some time to review it later.

@curtisy1
Copy link
Author

No worries! I figured if I started using Cloudreve, I needed to add some translation anyway. Although most of it is Google Translate'd for now, so adjusting the translations a bit is probably still necessary

@HFO4
Copy link
Member

HFO4 commented Oct 11, 2021

Thanks for the PR, but I have to close it. We're already working on designing i18n support in the backend side, it is planned to he shipped in next few versions. It seems conflicted with your change.
Also, using Google translated version is not a good idea.
Sorry about that!

@HFO4 HFO4 closed this Oct 11, 2021
@curtisy1
Copy link
Author

Alright, seems reasonable. Thanks for the heads up! If you need any help with getting i18n up and running on the backend side or need someone to proofread, feel free to get in touch.

Also, would you be okay with me leaving the fork open for people to use in the meantime? I think there's a few open issues about it in the backend repo that might benefit from it, even though the translations might be a bit off in some places

@HFO4
Copy link
Member

HFO4 commented Oct 12, 2021

Sure! I'll leave it open till official support shipped.

@HFO4 HFO4 reopened this Oct 12, 2021
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.

2 participants