-
Notifications
You must be signed in to change notification settings - Fork 17
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
Set language from browser, accept enter key in planner #242 #243 #253
Conversation
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.
Looks great to me
Tests will need to change I guess. |
Looks good indeed! Can you fix the test as well? |
Added tests for language, special cases. Fixed broken tests iRail#253
This probably means we can finally get rid of the cookies as well, right? |
While fixing the tests, I also added some new tests, to ensure that special cases are handled correctly. The language detection has been changed, instead of locale_lookup we're now using our own method. This was needed to support headers like "da, nl;q=0.9, en;q=0.2", where danish is not supported but Dutch and English are. While locale_lookup would fail, our own method will select nl in this case. About the cookies: since users can still override the language, the cookies are still used. Otherwise, the override (?lang=) would have to be present in every url. Currently the cookie is set:
If someone would block cookies however, he could still use the site based on the accept-language header. (instead of always returning to the language selection) |
Cookies will have to stay required. When using a device frome someone else with a different browser language, users should still be able to pick their language (looking up trains while at a friend, for example). However, maybe we can keep all cookie code in the language middleware, including the pick language redirect (this redirect is currently still located in the home controller). Also note that cookies to remember a language are allowed in the updated cookie law. (since widely used for user expierience, and not used for tracking) |
Resolves issues #242 and #243.
Note:
\Locale::lookup is a php function from the php_intl module. The page won't show if this module is not available.