-
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
feat(presenter): make i18n locale extractable from url #335
feat(presenter): make i18n locale extractable from url #335
Conversation
…on-and-vike-language
…on-and-vike-language
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 looks very good. Please check if you can unify the language defintions so that it wold be easier to add new languages.
If this is a big overhead which prevents merging, we can merge it and have a new task for that.
…on-and-vike-language
…on-and-vike-language
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.
I can only use http://localhost:3000/en/impressum
but not http://localhost:3000/de/impressum
which I expected to be able.
Since de
is the default language this should not imply a change for the page, still I think every valid language should be acceptable as the language parameter.
In general this is the right approach it just needs to be finished properly to be production ready.
There is a problem resulting from the manual language switch, which might be subject of another PR:
When you are on http://localhost:3000/en/impressum
and select de
in the menu the URL does not change, which then sets the content unaligned with the given url - so assume the following user flow:
- User goes to page
- Wants to send link to friend
- Notices - oh friend only speaks german
- Switches language
- Sends link
-> This scenario falls short here.
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.
A further test revealed that page navigation loses the locale.
If you are on http://localhost:3000/en/impressum
and navigate to http://localhost:3000/
locale is de again. This is actually an regression from previous functionality where the locale set in the browser was preserved.
Co-authored-by: Ulf Gebhardt <ulf.gebhardt@webcraft-media.de>
Co-authored-by: Ulf Gebhardt <ulf.gebhardt@webcraft-media.de>
Co-authored-by: Ulf Gebhardt <ulf.gebhardt@webcraft-media.de>
Co-authored-by: Ulf Gebhardt <ulf.gebhardt@webcraft-media.de>
🍰 Pullrequest
Issues
Todo