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 locale based on Accept-Language header #420

Merged
merged 5 commits into from
Jan 13, 2018

Conversation

stardust66
Copy link
Contributor

@stardust66 stardust66 commented Jan 11, 2018

This used to be Intimaria#1.

Context

Summary of Changes

  • Set locale based on the Accept-Langauge header of the request, which is usually set by the browser. According to the rails guide on i18n

The Accept-Language HTTP header indicates the preferred language for request's response. Browsers set this header value based on the user's language preference settings, making it a good first choice when inferring a locale.

Now, the language of the page will change to fit with the preferred language setting of the browser.

Change Setting

Chrome Settings
image
screen shot 2018-01-10 at 6 28 37 pm

This is a good option because the header is usually set by the user's
browser language preferences.

Code from http://guides.rubyonrails.org/i18n.html
If the header is nil, default to English.
@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 11, 2018

Thanks for re-doing this.

I did find out while working on #419 that this breaks if the user requests a locale we don't have.

Here is a screenshot when requesting the ru (Russian) locale:
about-russian-locale-no-translations

We need some way of defaulting to something we actually have, probably en, when we don't have the top user-accepted language.

Fortunately there are ways of handling this, such as: https://github.com/iain/http_accept_language That is a gem we can add and then have it select the best locale we provide, selected amongst users' requested locales. I am hoping the gem lets us fall back to a default locale when we match none of the user-accepted languages, but it's not super clear so we would have to test. Seems like most browsers might accept en as the bottom choice, regardless of locale, but we don't want to break if users manually delete the en language in their preferences.

Or if this can be done without installing another gem that's fine by me.

"en"
else
request.env['HTTP_ACCEPT_LANGUAGE'].scan(/^[a-z]{2}/).first
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets the locale to be the top-preferred locale of the user's browser, whether we have that locale translated or not. Unfortunately, if the top-preferred locale is one we don't have, users see the translation key names in the page, rather than translated strings from an actual locale.

I think we should

  • Go through each of the user's preferred locales and use the first one we have.
  • Find a way to revert to en when the user's requested locales aren't ones we have.

Can use a gem to handle this, or do it directly in custom Ruby logic.

The gem automatically chooses the optimal language to set, as the
user usually provides a list of languages in order of preference.
This also makes sure that the locale is set to the default when
the requested locale isn't present.
@stardust66
Copy link
Contributor Author

stardust66 commented Jan 12, 2018

I used the gem you suggested. With this, the locale gets set to the correct value and when a locale that we don't support gets requested, it defaults to the value of config.i18n.default_locale which defaults to :en.

We tell rails which locales we supported by setting config.i18n.available_locales.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 12, 2018

Looks good! Going to give it a quick test with the ru locale again just to see what happens.

Also, I'm interested in possibly using language_region_compatible_from(languages). If it does what it says it's supposed to, then that would support zh_ch and zh_tw, whereas compatible_language_from(languages) apparently would treat both as the same thing. Who knows, maybe someday we will have multiple regional versions of the same language.

@@ -14,17 +14,7 @@ def mobile_filter_header
end

def set_locale
I18n.locale = extract_locale_from_accept_language_header
I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using http_accept_language.language_region_compatible_from(I18n.available_locales)?

According to this README:

language_region_compatible_from(languages): Returns the first of the user preferred languages that is also found in available languages. Finds best fit by matching on primary language first and secondarily on region. If no matching region is found, return the first language in the group matching that primary language.
vs:
compatible_language_from(languages): Returns the first of the user_preferred_languages that is compatible with the available locales. Ignores region.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 12, 2018

Okay, it looks good. This is what I get with the ru locale preferred, followed by es-ES and es, and furthermore, I even deleted en-US and en from my accepted languages.
ru-locale-set-got-english

The site still gave me the en locale (verified in developer tools):
refuge-lang-equals-en

Edit to clarify: That's good, because on this branch we only have English. English is better than displaying only the translation key names!

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 12, 2018

This looks reallly good in further testing. First, I configured the app to believe it supported English, generic Chinese, Chinese specifically for Taiwan [aka traditional Chinese], and Chinese specifically for China [aka simplified Chinese] in config/application.rb.

config.i18n.available_locales = [:en, :zh, 'zh-TW', 'zh-CN']

Here were my results:

Preferred languages What the site gave me Correct behavior?
ru, es-ES, es en ☑️
zh, ru, es-ES, es zh ☑️
zh-CN, zh, ru, es-ES, es zh-CN ☑️
zh-TW, zh-CN, zh, ru, es-ES, es zh-TW ☑️
zh-CN, zh-TW, zh, ru, es-ES, es zh-CN ☑️

Update: more testing

I configured the app to belive it supported only generic English and Chinese specifically for China...

config.i18n.available_locales = [:en, 'zh-CN']

Results:

Preferred languages What the site gave me Correct behavior?
zh-CN, zh-TW, zh, ru, es-ES, es zh-CN ☑️
zh-TW, ru, es-ES, es zh-CN ☑️
zh-TW, ru, es-ES, es, en zh-CN ☑️
en, zh-TW, ru, es-ES, es en ☑️

@stardust66
Copy link
Contributor Author

Thanks for all your testing @DeeDeeG!

@DeeDeeG DeeDeeG mentioned this pull request Jan 12, 2018
@Intimaria
Copy link
Contributor

Wow this is fantastic. I admire all of this work!

Copy link
Contributor

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, and all tests pass in Travis CI.

Thoroughly tested in Vagrant.

Makes us ready for new translations, while being totally backward-compatible to our existing locale, which is just en.

@DeeDeeG DeeDeeG merged commit 868f53f into RefugeRestrooms:develop Jan 13, 2018
DeeDeeG added a commit that referenced this pull request Jan 13, 2018
Deletes extra copy of mobile_filter_header definition in
app/controllers/application_controller.rb

Missed this when reviewing #420.
@stardust66 stardust66 deleted the language-detection branch January 13, 2018 12:33
@stardust66 stardust66 restored the language-detection branch January 13, 2018 12:33
@stardust66 stardust66 deleted the language-detection branch January 30, 2018 02:36
DeeDeeG added a commit to DeeDeeG/refugerestrooms that referenced this pull request Oct 15, 2018
…tion

Set locale based on Accept-Language header.

Uses the http_accept_language gem to intelligently pick
the most-preferred locale we have.

Makes us ready to start accepting and testing new translations.

Still desirable would be to have an in-app language-switcher.
DeeDeeG added a commit to DeeDeeG/refugerestrooms that referenced this pull request Oct 15, 2018
Deletes extra copy of mobile_filter_header definition in
app/controllers/application_controller.rb

Missed this when reviewing RefugeRestrooms#420.
DeeDeeG added a commit to DeeDeeG/refugerestrooms that referenced this pull request Nov 3, 2018
…tion

Set locale based on Accept-Language header.

Uses the http_accept_language gem to intelligently pick
the most-preferred locale we have.

Makes us ready to start accepting and testing new translations.

Still desirable would be to have an in-app language-switcher.
DeeDeeG added a commit to DeeDeeG/refugerestrooms that referenced this pull request Nov 3, 2018
Deletes extra copy of mobile_filter_header definition in
app/controllers/application_controller.rb

Missed this when reviewing RefugeRestrooms#420.
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