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

Add a locale switcher #620

Merged
merged 8 commits into from
Apr 16, 2020
Merged

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 10, 2020

Context

Summary of Changes

  • Add a default URL parameter, :locale that will render as ?locale=[your-locale-code-here] in the URL. E.g. /restrooms/new?locale=es is the "Add a New Restroom" page, explicitly in Spanish.
  • Enable optional routing to locale-prefixed versions of most URLs in the app. E.g. /fr/restrooms/new is the "Add a New Restroom" page, explicitly in French.
  • Add locale-switching links to the footer, approach inspired by the footer at https://rubygems.org
  • Most dynamically-generated links automagically use the locale prefixes (like /pt-BR/) now.
    • Exceptions appear to be... /admin (locale switcher and URL prefixing don't seem to work here.)
    • The site root/homepage / can work with locale prefixes, but the link in the header seems to prefer locale parameters /?locale=[your-locale-code-here]

Checklist

  • Tested Mobile Responsiveness
  • Added Unit Tests
  • CI Passes
  • Deploys to Heroku on test Correctly (Maintainers will handle)
  • Added Documentation (Service and Code when required)

Screenshots

Before

After

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 11, 2020

If anyone knows how to make this test passing again, I'd appreciate it a lot! ❤️

Failures:

  1) the contact process should show a generic contact when contact is not from restroom form
     Failure/Error: visit restroom_path restroom
     
     ActionController::UrlGenerationError:
       No route matches {:action=>"show", :controller=>"restrooms", :locale=>#<Restroom id: 232, name: "Mission Creek Cafe", street: "1800 Market St", city: "San Francisco", state: "CA", accessible: false, unisex: false, directions: "Direction", comment: "Comment", latitude: nil, longitude: nil, created_at: "2020-04-11 00:04:34", updated_at: "2020-04-11 00:04:34", downvote: 11, upvote: 22, country: "US", changing_table: false, edit_id: 232, approved: true>}, missing required keys: [:id]
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/journey/formatter.rb:57:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:744:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:775:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:822:in `url_for'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:273:in `call'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:214:in `call'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:331:in `block (2 levels) in define_url_helper'
     # ./spec/features/contacts_spec.rb:7:in `block (2 levels) in <top (required)>'

Finished in 40.73 seconds (files took 1.95 seconds to load)
62 examples, 1 failure

Failed examples:

rspec ./spec/features/contacts_spec.rb:4 # the contact process should show a generic contact when contact is not from restroom form

It seems like when visit restroom_path restroom is invoked here, a garbled :locale value gets piped in that has way too much unrelated data in it, and so it isn't valid as a locale. And the app gets rather unhappy about it.


Viewing restroom listings, and using the contacts page both work locally in Docker.

I think the test itself is broken here, and the app isn't misbehaving (other than not matching up well with the test anymore).

Since this test is related to using the "contact" page from a restroom listing, and that functionality was removed in #487, I'm somewhat tempted to just disable or greatly simplify this test.

@mi-wood
Copy link
Member

mi-wood commented Apr 12, 2020

That test still sort of acts as a regression tests and tests navigation to Contact.

Right now, it's implicitly filling in the parameters of restroom_path and setting :locale to the restroom. You can explicitly set it as restroom_path(:id => restroom.id).

https://stackoverflow.com/questions/31224170/no-route-matches-action-show

@DeeDeeG DeeDeeG force-pushed the locale_switcher branch 2 times, most recently from 4c40d19 to 61d31ab Compare April 14, 2020 17:29
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 14, 2020

Thanks, @mi-wood, tests are passing now!

I think this is ready to merge.

Adds a :locale parameter (which should come from the URL, e.g.:
`refugerestrooms.org/?locale=en` or something like that.)
Allows URLs auto-generated by Rails to automatically be expanded
to include "?locale=xyz" or "&locale=xyz"

See: https://guides.rubyonrails.org/v5.2.4/i18n.html#setting-the-locale-from-url-params

For example, this affects URLs made with ActionView URL helpers
('button_to', 'link_to', etc.)

See: https://api.rubyonrails.org/v5.2.4/classes/ActionView/Helpers/UrlHelper.html

(Hard-coded URLS will generally not get the parameter added.)
Use a link_to helper to dynamically create the homepage link,
because URL helpers now automatically include the 'locale=' parameter.
Use the 'api_docs_path' helper, rather than hard-coding '/api/docs'.

(This is for the navigation header, under the "Resources" drop-down.)
Adds an optional locale prefix for almost every page in the app.

See: https://guides.rubyonrails.org/v5.2.4/i18n.html#setting-the-locale-from-url-params

You can now visit e.g. '/es/restrooms/new' and you will see the
"Submit a New Restroom" page in Spanish.

All links in the app auto-generate this locale prefix,
so users can specify a locale via visiting a specific locale prefix,
and the links in the app will not misdirect them into another locale.

(You can still visit URLs without a locale prefix,
like '/restrooms/new', and the app will simply auto-detect the locale
based on the preferred languages in your browser settings.)

This preserves the existing URLs in working order,
and any links out there on the web, or in people's bookmarks,
will still work.)

The home page does not always get the prefix:
- The home page link in nav is just '/?locale=[I18n.locale]'
  - You can visit '/?locale=es' to see the homepage in Spanish.
  - The homepage link in the nav are auto-generated
    as '/?locale=[I18n.locale]'; This is equivalent in functionality
    to the locale prefixes (e.g. '/es/').
  - The homepage links in the footer are auto-generated as
    '/[I18n.locale]/'.
  - You can visit '/es/' to see the homepage in Spanish.
Adds locale switcher links in the footer,
which displays at the bottom of every page.

These links dynamically link to the current page the user is viewing,
but with the current locale overridden with a specific, new locale.

Uses the Rails API's "ActionDispatch::Request" feature
to get a string containing the last requested page,
(i.e. the page the user is currently viewing).
This includes query parameters, such as "?lat=[num]&long=[num]"

See: https://api.rubyonrails.org/v5.2.4/classes/ActionDispatch/Request.html#method-i-GET

(Parameters can also be derived from the URL,
such as the "es/" in "/es/restrooms/new", if routed properly.)

See: https://guides.rubyonrails.org/routing.html

See these StackOverflow answers/this Wikipedia article for details:
- https://stackoverflow.com/questions/3762430/rails-preserving-get-query-string-parameters-in-link-to
- https://stackoverflow.com/questions/6885990/rails-params-explained
- https://en.wikipedia.org/wiki/Query_string
Spaced out the links a bit, and added bullet-point separators.
Passing the whole array of restroom data was causing the whole
array to be erroneously interpreted as if it was the :locale
prefix in the path.

Explicitly pass :id, and only :id,
to the `restroom_path` route helper for this test.

(We only need the :id from the newly-constructed test restroom
in order to visit the correct path. Other restroom data isn't
needed here anyhow.)
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 16, 2020

Gong to put these changes on staging for a bit, but it seems worth another deploy if they behave well.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 16, 2020

Turns out this does break some of the locale-prefix-less routes, like /about, /business_info, /contact, /signs and the hidden /text. It doesn't break /restrooms/new, /restrooms/[id], or /api/docs though.

locale-prefixed routes are doing fine, such as /en/signs, /fr/contact, /pt-BR/business_info etc.

Either I need to figure out a fix (or someone can explain one to me/commit it themselves) or perhaps revert this PR for now in develop branch.

I find routing in Rails kind of tricky.

DeeDeeG added a commit that referenced this pull request Apr 23, 2020
DeeDeeG added a commit that referenced this pull request Apr 23, 2020
(This PR still needs some work.)

This reverts commit aed3e1e.
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 23, 2020

Reverted for now, as this wasn't working as intended.

This was working with just the ?locale=[your-locale-here] URL parameters, but was still kinda broken with the /[your-locale-here]/ URL prefixes.

I would really like to get the URL prefixes working, but I need to read up more on the Rails router. (If anyone knows how to do this, feel free to step in!)

Also, there are hard-coded HTML anchor (<a>) tags from about to business_info, and vice-versa, but these are inside Internationalization (I18n) strings. I have to look up the interaction between haml and I18n here to properly translate the dynamic link with link_to and [page-here]_path helpers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make a language switcher for the site
2 participants