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, second try #656

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Add a locale switcher, second try #656

wants to merge 16 commits into from

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Nov 4, 2020

Context

Summary of Changes

  • Add a default URL parameter, locale that will appear as ?locale=[your-locale-code-here] in the URL. For example: /restrooms/new?locale=es is the "Add a New Restroom" page, explicitly in Spanish.
    • Dynamically generated links, such as the ones made with Rails' helpers (for example: link_to), automagically use the URL parameter ?locale=[some_locale_here] now.
  • Use dynamically-generated links throughout the site, replace hard-coded links with dynamically-generated ones.
  • Add locale-switching links to the footer, approach inspired by the footer at https://rubygems.org
  • Enable optional routing to locale-prefixed versions of most URLs in the app. For example: /fr/restrooms/new is the "Add a New Restroom" page, explicitly in French. (Disabled for now, as this wasn't working as intended.)

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

Refuge Restrooms website footer with GitHub, Twitter, Facebook and "Contact us" links, GitHub blurb with link, Patreon blurb with link, and copyleft notice

After

Refuge Restrooms footer, with all of the above, and now with links to all locales at the bottom (English, Español, Filipino/Tagalog, Français, हिन्दी, Italiano, polski, and Português do Brasil)

DeeDeeG and others added 13 commits April 14, 2020 13:53
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.)

Co-authored-by: Mikena Wood <[email protected]>
This reverts commit 85f1998.

Try this pull request again, without the `locale/` URL prefix feature.
Dynamically compose the "Business FAQ" link on the "About" page
and the "About Page" link on the "Business FAQ" page
using Rails link helpers, so that the locale (and any other
default URL parameters) are automatically added to these links.

In other words, "un-hard-code" these links.

(In this commit, updated the Haml source of the About and Business FAQ
pages, and only the English internationalization (I18n) strings.)
Update the formatting of the "About" and "Business FAQ" links
in the various locales other than English.
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Nov 4, 2020

Hi @brunoocasali, I hope you are doing well.

I've been working on finishing the locale switcher feature for Refuge Restrooms. I think it's ready now!

If you are interested in helping with adding tests for locales other than English, I think this could be a reasonable place to do that. It would, of course, be much appreciated.

Best Regards,

- DeeDeeG

brunoocasali
brunoocasali previously approved these changes Nov 4, 2020
Copy link
Contributor

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

🥇 good work @DeeDeeG

After these changes get merged, I can submit a PR to add tests to it!

app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_footer.html.haml Outdated Show resolved Hide resolved
app/views/layouts/_navigation.html.haml Outdated Show resolved Hide resolved
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Nov 6, 2020

@brunoocasali I added the changes you suggested, matching the Rubocop-linted style.

During testing, I found out that there is a problem still. Might you be able to help troubleshoot this?

When starting a new search for restrooms using the search bar, the locale parameter is not preserved. It appears to me that the search results page is generated only with the following parameters (with the example search "San Francisco").

/restrooms?utf8=✓&lat=37.7749295&long=-122.4194155&search=san+francisco

  • utf8
  • lat
  • long
  • search

But no locale parameter.

It's not immediately clear to me how these parameters are generated, as I'm not familiar with that part of the codebase. (I suspect it might be part of Refuge Restrooms' JavaScript code, but I'm not sure.) If I knew where these were generated, then perhaps it would be easier to ensure the locale parameter was preserved there, too, if set.

@brunoocasali
Copy link
Contributor

@DeeDeeG I could help you with this tomorrow, okay? :)

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Nov 7, 2020

That would be great!

I'm getting more confident that it's in app/javascript/packs/lib/ --> geocoder.js or maps.js.erb. Not totally sure, though.

@brunoocasali
Copy link
Contributor

@DeeDeeG checkout the #657 ;)

brunoocasali and others added 2 commits March 9, 2021 14:03
* Create a LocaleService to centralize locale logic

* Create a way to keep track of current_locale even between actions
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Mar 9, 2021

Thanks for reviewing/merging #657, @tkwidmer.

I think the last thing for me to do on this is to check the "new restroom" submission process works properly with these locale switchers. I would like to do the dependency bumps first and get those out as a patch release.

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
3 participants