Problem:
Currently there is a textfield setting the Google Maps language parameter site-wide, making it difficult for multilingual sites to change the map's language to match a site visitor's language.
Solution:
Add a checkbox to the geolocation settings allowing users to pass the current Drupal language to Google maps, if applicable.

Checkbox under language parameter to pass Drupal language to Google Maps

Example, a multilingual site has a Google Map showing "Mexico" for the English (en) translation and "Mexique" for the French (fr) translation.

The list is taken from this table on developers.google.com.

In comparing the list of languages shipped with Drupal 8, there are two cases where the Drupal language codes differ from what's expected from Google. There's a small mapping function to account for these differences.

If the visitor's chosen Drupal language is not in the list of available Google Maps languages, it will fall back to the textfield value that currently exists in the geolocation settings.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikedotexe created an issue. See original summary.

mikedotexe’s picture

  • Add checkbox
  • Add to config upon form submission
  • Add to schema's config_object
  • Add list and mapping of available Google Maps language codes
mikedotexe’s picture

Status: Active » Needs review
mikedotexe’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: geolocation-multilingual-map-option-2898210-2.patch, failed testing. View results

mikedotexe’s picture

Status: Needs work » Needs review

In regards to the failed test, I had to set the SIMPLETEST_BASE_URL and SIMPLETEST_DB values.
Locally, I ran this command In the core directory and all tests succeeded:
../../vendor/bin/phpunit ../modules/contrib/geolocation/tests --verbose

This makes me think there's an issue with the d.o. testing, particularly that it doesn't have a way to retrieve the API key, and hence fails.
I believe the failed javascript tests aren't related to this patch (also, I didn't touch any js), so I'm changing the status back to "Needs review"

ChristianAdamski’s picture

I triggered the retest. It just happens. It's something with the JS driver during testing. Or maybe the tests start before the install hook for the test module is executed? Either way, those API key errors just happen randomly.

ChristianAdamski’s picture

Two things about the patch:
- if you have a mapping function for the differences between Google and Drupal, why have the list in too?
- There were vaguely related issues or at least requests before. Could you link those up to this one, if you feel that this issue solves there use-case?

I won't ask for a test, as I don't really see how to test this. Maybe just enable this in a test, to make sure it generally works, without testing the actual translation?

mikedotexe’s picture

Thanks for taking the time to look at this. I'll answer the two questions here, and provide a test I've been working on in a patch soon.

1. I want to provide the list of available language codes from Google so that we know which Drupal languages aren't in the list, and can use the fallback in that case. For instance, if a Drupal site is translated into Afrikaans (af), which is not supported by Google Maps, it will fallback to the value indicated in the geolocations textfield for language.

2. Issues related to this functionality for the Drupal 8 version:
Here's a link to an issue where the proposed solution is to use the current language coming in from the drupalSettings javascript object.
https://www.drupal.org/node/2738617
My patch uses \Drupal::languageManager()->getCurrentLanguage()->getId() instead, as it's building the request URL to Google Maps. Seems like the same use case as this issue I've opened, just a different approach.

This issue mentions the same concept, without a patch:
https://www.drupal.org/node/2754211

And here's the initial feature request:
https://www.drupal.org/node/2837734

mikedotexe’s picture

Here's an interdiff and the final patch.
Turns out I was able to see if the Google Map is returning our language.
There's a link returned that contains the parameter ...&hl=fr... if we're requesting a French map.

Then, as requested in comment #8, to make sure it generally works and doesn't break things, I run some existing tests after enabling this "use current language" checkbox.

I've run this many times locally and it's passing.

Status: Needs review » Needs work
mikedotexe’s picture

Needed to add locale and language dependencies for that test. Added here.

mikedotexe’s picture

Status: Needs work » Needs review
marassa’s picture

Looks good to me!
I would just change the wording in the settings from "site translation language" which sounds somewhat misleading to me to "current page language" or "Interface text language selected for page" (as in Views) which is what the patch is actually doing.

mikedotexe’s picture

I agree with the suggestion in #14 to rename the checkbox.
One of the key function calls of this patch is LanguageManagerInterface::getCurrentLanguage($type = LanguageInterface::TYPE_INTERFACE)

So in the verbiage of Drupal, I've settled on "Use current interface language"

ChristianAdamski’s picture

Altered some wording
Moved setting out of parameters
Fixed some minor things
Removed the whole language code thing. It's a really nice gesture to include it, but this zh-tw <=> zh-hant business has come up before in the Drupal issue queue and common understanding seems to be, screw it. There is mapping available in the language module btw, but it is only handling the other way.

If this goes green, I'll commit it.

Status: Needs review » Needs work

The last submitted patch, 16: 2898210-16-language-stuff.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ChristianAdamski’s picture

Status: Needs work » Needs review
FileSize
5.65 KB

Test altered

Status: Needs review » Needs work

The last submitted patch, 18: 2898210-18-language-stuff.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ChristianAdamski’s picture

Status: Needs work » Fixed

Let's see what breaks for others. @mikedotexe Thanks for your work!

mikedotexe’s picture

Glad to help!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.