Skip to content
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

Merged
merged 5 commits into from
Sep 22, 2017

Conversation

Bertware
Copy link
Member

@Bertware Bertware commented Dec 6, 2016

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.

Copy link
Member

@Haroenv Haroenv left a 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

@Haroenv
Copy link
Member

Haroenv commented Dec 6, 2016

Tests will need to change I guess.

@pietercolpaert
Copy link
Member

Looks good indeed! Can you fix the test as well?

@pietercolpaert
Copy link
Member

This probably means we can finally get rid of the cookies as well, right?

@Bertware
Copy link
Member Author

Bertware commented Jan 15, 2017

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 ?lang is available, use it as override and update cookie
  • if cookie set and ?lang not set, use cookie
  • if ?lang and cookie not set, use browser language and update cookie

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)

@Bertware
Copy link
Member Author

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)

@Bertware Bertware merged commit 1575956 into iRail:development Sep 22, 2017
@Bertware Bertware deleted the development branch September 22, 2017 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants