jcasimir / locale_setter

A simple library to set request locale based on a hierarchy of factors
MIT License
118 stars 18 forks source link

Change the way url param works #9

Open somebody32 opened 11 years ago

somebody32 commented 11 years ago

Hi,

I think that the current solution for the url param is a little bit misleading, for example:

Current realisation is:

def default_url_options(options = {})
  if i18n.locale == i18n.default_locale
    options
  else
    {LocaleSetter.config.url_param => i18n.locale}.merge(options)
  end
end

If I am using the site's default locale then all is ok, but if any of my users have a different locale via db locale column (or domain, not so matter) then all links will have a locale param. I think that this is polluting url because we already setup locale via different source.

So I think that maybe is better to add locale param if only it is already exists in url without any checks to the locale.

And then workflow will be like this:

  1. User lands to the main page (default locale will be used)
  2. User logs in (locale will switch to preferred one, no url changes)
  3. User passing locale param to url (or clicks a link that will force it)
  4. All other links will have a locale param

If this is ok and I haven't missed something, then I'll provide a solution for this.

aaronchi commented 11 years ago

I agree. it is annoying that locale param is automatically added to every link. This should only be used when switching between locales and should probably set a session variable to save the choice

assimovt commented 11 years ago

+1

Also would there be a way to make URLs look prettier? So instead of:

http://example.com/articles?locale=jp

They will always be:

http://example.com/jp/articles
steakchaser commented 10 years ago

@assimovt you can still achieve this and use this gem. To do this, you need to setup your routes with a locale scope or simply use this other gem (works in compliment with locale_setter): https://github.com/svenfuchs/routing-filter

zykadelic commented 10 years ago

+1 for this

ludwigschubert commented 9 years ago

Note that #14 solves this issue for me, @jcasimir maybe you could release a new version that includes this PR? I'm currently bundling straight from HEAD to get this fix, and this has been working great in production.