-
Notifications
You must be signed in to change notification settings - Fork 2
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
Translate CodeHarbor to German #1143
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1143 +/- ##
==========================================
+ Coverage 86.55% 87.55% +1.00%
==========================================
Files 98 99 +1
Lines 2291 2315 +24
==========================================
+ Hits 1983 2027 +44
+ Misses 308 288 -20 ☔ View full report in Codecov by Sentry. |
3f82047
to
992290b
Compare
I encountered a problem with the flash messages. Apparently there is an encoding problem for German characters "äöü". The flash messages are transferred using HTTP headers which are supposed to be ASCII encoded. I am not sure how we should solve this. One possibility would be to encode flash messages with base64 and decode them on the frontend. However that is not exactly ideal because we could not use the normal flash messages anymore but replace them with a custom wrapper doing the base64 encoding. Edit: I just thought we could base64 encode the flash message in an after_action and realized there already is an after_action handling the flash messages... So I will just use the base64 solution. |
I found out that the base64 part is not even required. We can use "ERB::Util.url_encode(flash_message)" on the Rails side and "decodeURI(request.getResponseHeader("X-Message"))" in the Coffeescript. This even works for Unicode characters :) |
Thanks for describing the flash message encoding issue. I think one of your solutions is fine (e.g., the URL encoding). The problem here is that the mechanism chosen with the |
How do we want to translate the mails users receive when somebody requests access to join their group? The problem I see is that we do not know the preferred locale of the receiver at the time we send the mail. Maybe we should store the last used locale for users? We could also use this to automatically set the last used locale when a user signs in. Edit: I think this could be stored in the User model. What do you think? |
Generally speaking, I see two possibilities here: Either, sending the messages in both languages ("German version below") or storing the desired locale in the user profile (this is the mechanism we chose for openHPI). If you want to go the extra mile, storing it in the profile and updating it on each locale change would hence be the "better" option. |
I would go with the second option. I can probably take some code from CodeOcean so this should not be too difficult. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
app/views/access_request_mailer/send_contribution_request.text.erb
Outdated
Show resolved
Hide resolved
About the commit squashing: I would squash all of my own commits and leave yours as they are if you are okay with that. |
Well, generally, a similar approach is okay. However, I would suggest not squashing too much. What do you think if we mainly squash those that contain changes to the translations and keep those separate, that include (further, relevant) code changes? Specifically, I thought of:
Of course, this is just a suggestion and you do not need to comply with them. My suggestion is mainly based on the idea that each commit should contain something like a self-contained, small, shippable increment. For that, we can also include my first commit "Provide initial German translation" in the squashing and merge it with the rephrasing performed later. The idea to keep some other separate is that I'd argue each of them contains a dedicated "feature" or a "bug fix", or that some "explanation" in the commit might be helpful for further debugging. |
I was wondering whether we want to pass the locale parameter across requests like the rails guide suggests:
We could replace the session locale with that. One advantage of this is that the user keeps their selected locale when logging out. Currently you fall back to either your browser locale or the default locale if you log out because you loose your session. |
d0b0b52
to
d5dfc87
Compare
I now squashed the commits in a way that I think makes sense. What do you think? |
0556fdf
to
cfed5bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your final touches on this PR. The only part we need to check is the localisation of select2, otherwise I think that's fine 👍
Just after pushing the commit I noticed there was another bug in it. I will update it soon. |
c4505af
to
1077b98
Compare
Actually I cannot figure it out. Maybe you see the problem? I pushed my current state. The problem is that only the label select2 gets initialized and the others are not. |
1077b98
to
6d718c3
Compare
All locales were just translated using OpenAI, and are not validated yet. Further, we had to split the translation into several requests, which probably accounts for the different greeting. Those should be unified later
---------------------------------------------------------------------- - using formal address for all German translations - adding translations for devise and simple_form; moving some translations from devise to user - translating select2 - translating group/collection messages - removing user locales that are already in devise
6d718c3
to
d94b312
Compare
d94b312
to
6a58a21
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you very much for your endurance and perfection to finalise this PR! 💯 I've just rebased this branch on the current master, localised two additional strings I discovered and tackled the remaining comment. Now, I am happy to approve and merge it right away 👍
This PR adds the current status of the translation of CodeHarbor to German.
As specified in the first commit, the translations provided here were not checked manually yet and therefore contain some inconsistencies, such as the salutation ("Du" vs "Sie") or the gender form. Before merging, we should unify those (probably "Sie" and gendered nouns with
:
[e.g., "Empfänger:in"]). Further, I omitted the "external" translations, since we can probably borrow them from some other location without the need to review them.The other changes beside the locale files are probably enough to enable an additional language for CodeHarbor and allow users to switch. These parts of the code originate from CodeOcean and are only adjusted where necessary.