-
Notifications
You must be signed in to change notification settings - Fork 62
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
i18n support #78
Comments
👋 Please do. Hopefully the frontend code is not to spaghetti. I know strings are thrown around all the places. Even with ternary operators. react-i18n is 👍 to me! |
Hey, What do you think? luismanson@563ffbf |
Hey @luismanson, I will look into it today :) |
@luismanson added a comment: luismanson@563ffbf#commitcomment-83183511 To me it looks great. You do not have to add the loader component for the Suspense, but if you do it would be cool. Feel free to open a PR :) |
PR: #82 |
new PR: #83 Thank you for your contribution :) |
How would I go about adding more languages? I've created a new file |
Hello, you need to set PS: @bjarneo I'll submit my translation in the next day or so. |
If i can figure out at what point the strings are loaded, maybe I can whip up a language detection & automatic switching method. Wish me luck 🤞🏼 |
check this doc: https://react.i18next.com/latest/using-with-hooks the example has some comments about language detection and file loading |
Reopening this issue. One thing to consider here is to have an option to set the default language for the instance. This can be solved by adding an env var called Which means you can fetch it like this: https://github.com/HemmeligOrg/Hemmelig.app/blob/main/src/client/routes/home/index.js#L51 |
Btw, so cool that you guys want to contribute 🎉 |
I have added you both @luismanson and @RainerZufahl as contributors to this repository. Let me know if something is not right about the settings. Should be allowed to create branches and PRs |
Thank you! I've already created a fork of the repo which I'm working in, should I scrap that and create a new branch in this repo? |
@rainerzufahl that is up to you, will work either way |
Wow, thanks! I'll test if works with the spanish json later today. |
automatic language detection & switching is now present via #88. |
How should we handle new strings? I think this has to be as transparent as possible. |
Hm. Interesting. I have to look into this. But, of course, using t('some.string', 'Default string') is a good start, because that means if a language does not have the translation, it will automatically fallback to the default english string? |
Yes, it should. If i understand correctly, this would even work with an empty default LANG setting. |
I think that would be the first approach for this. How would runtime extraction and plugins work with create-react-app? |
I'll have to look into it, i do not know tbh. |
That would mean that the english translations are hardcoded into the program, instead of an external string file, correct? While practical in some ways, that would make adjusting the english strings later on more complicated again. |
Yes, that's right, it's a "good start" as @bjarneo called it and we can all agree that hardcoding stuff is a big no-no... |
Continuing from #212
|
@bjarneo Any luck trying to find out what's causing this? It's pretty hard to onboard people if there's a language barrier (especially with the knowledge that the translation is in fact already present 😅). |
@ltguillaume this PR solves the SECRET_FORCED_LANGUAGE issue: 1404f0a |
Confirmed working. Next step, proper detection of the browser locale. |
Yeah, that would've been something |
Well, when I looked at the code a while back, it seemed like the logic to do that is already there, but perhaps it doesn't actually get the values from request header to properly evaluate them against the available languages, thus defaulting to English (or SECRET_FORCED_LANGUAGE)? |
Any progress with this? Because people can translate all they want, but it doesn't work anyway... |
No, I have not looked into this. |
I'm sorry I can't be of any help. I've tried to fix the code at least three times now, but I don't really understand how the i18n works with regard to Hemmelig. As a result, I keep having to make assumptions and so far, nothing has worked. |
That's great, thank you so much! 😁😁 |
@ltguillaume no worries. Let me know if it does not work as intended. :) |
1a9d84e By the way, I found the following:
|
Yes, that was it. Do not be harsh on yourself. It still has been implemented in a way not needed for Hemmelig as far as I can tell. As for your bullet points, that is something that has to be looked into.
|
In
In
|
Right, that makes sense. It should be an easy fix, I do not know the fix, but it should exclude /signin. Probably just a setting somewhere. |
Not just that. It seems to put the route secret/sigin where the language code should be. |
Yeah, and this application do not support this kind of routing (yet), so I am unsure if it has ever worked for these routes. |
Wait, this is probably just because of |
Yes, I was thinking about removing this, but forgot to try to do so while fixing the navigator. Removed it now, and looks like that solved it: be6ba9e |
This contains the removal of path. |
Yes, I tested it already and it works nicely 🙂 Now everything that faces "normal" end users (create and view secrets) is translatable 😃 except for the errors like Next is "sign up", "sign in", then the "account" part. These strings are also still hardcoded. |
Hi, I'm the guy who asked about translation support in Reddit. Would you consider a PR with some support? Frontend maybe.. I think I can add some basic implementation of react-i18n...
The text was updated successfully, but these errors were encountered: