-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add language picker #143
Add language picker #143
Conversation
@cblgh Can you put this PR in draft mode to indicate its readiness? |
@@ -1,5 +1,8 @@ | |||
# default localiztion file for english | |||
|
|||
# the name of this language, in its language. Used for language picking | |||
LanguageName = "English" |
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.
this new key LanguageName
is used for populating the language picker (the list of languages in the page footer). it would be a good idea to document the translation key alongside other instructions for adding translations to the existing room
this is the test failure i am seeing on my machine:
|
re the test failure, i just checked out your branch and couldn't reproduce. I suspect the npm deps changed. Try running |
The test failure means that |
ok, all that's remaining then is
|
Totally orthogonal but before we need yet another migration, can you add another bool to the settings table for #160 ? column name Update models please and I will add the functions on the roomdb interfaces myself later. Just don't want to touch that migration file again. |
@cryptix gotcha! will do this after the current batch of work i'm doing :) |
i18n.ListLanguages() returns a map, mapping language tags ('en', 'sv') to the names of their corresponding languages (as translated by the language itself). This functionality will be used in the language picker, to present a nice list of the translated languages. I renamed testing.go to conform to go's testing conventions, and added a test for i18n.ListLanguages().
while working on the /set-language route, i noticed that i was getting a csrf error for all /admin views when setting the language, while it worked well on non-admin routes. the issue, it turned out, was that we needed to configure gorilla's csrf feature to set all cookies on the same route. when unconfigured, the set cookies will only be set for the path they are being set at. see more in the gorilla.csrf documentation (in particular the csrf.Path option): https://pkg.go.dev/github.com/gorilla/csrf?utm_source=godoc#Path
to make sure the list of languages is sorted, we now use a slice of TagTranslation{Tag: string, Translation: string} structs, sorted by `TagTranslation.Tag`.
related to #160, requested by cryptix
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.
Some nagging but really like the substance! Good for merge after these changes.
Thanks for the code review @cryptix! I'll get on implementing the remaining changes & fixing the tests :) |
Co-authored-by: Henry <111202+cryptix@users.noreply.github.com> use eh.Handle
42f6cce
to
a159108
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.
Very nice now! LGTM
Closes #45
tasks
LanguageName
defining the language name of a given translation?/change-language
POST requestweb/i18n/helper.go
to query for any language cookies & use the corresponding language translation if the cookie is set>= 2
language translations to pick from/set-language
only working on non-admin pages (bizarre csrf error), fixed in bbd77b4use_subdomain_for_aliases
with a default of1
/set-language
sets a language cookie