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.
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.
Comment | File | Size | Author |
---|---|---|---|
#18 | 2898210-18-language-stuff.patch | 5.65 KB | ChristianAdamski |
#16 | 2898210-16-language-stuff.patch | 5.96 KB | ChristianAdamski |
#15 | geolocation-multilingual-map-option-2898210-15.patch | 8.74 KB | mikedotexe |
| |||
#15 | interdiff-2898210-12-15.txt | 877 bytes | mikedotexe |
#12 | geolocation-multilingual-map-option-2898210-12.patch | 8.74 KB | mikedotexe |
|
Comments
Comment #2
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedComment #3
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedComment #4
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedComment #6
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedIn 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"
Comment #7
ChristianAdamski CreditAttribution: ChristianAdamski commentedI 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.
Comment #8
ChristianAdamski CreditAttribution: ChristianAdamski commentedTwo 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?
Comment #9
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedThanks 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
Comment #10
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedHere'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.
Comment #12
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedNeeded to add
locale
andlanguage
dependencies for that test. Added here.Comment #13
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedComment #14
marassa CreditAttribution: marassa commentedLooks 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.
Comment #15
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedI 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"
Comment #16
ChristianAdamski CreditAttribution: ChristianAdamski commentedAltered 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.
Comment #18
ChristianAdamski CreditAttribution: ChristianAdamski commentedTest altered
Comment #21
ChristianAdamski CreditAttribution: ChristianAdamski commentedLet's see what breaks for others. @mikedotexe Thanks for your work!
Comment #22
mikedotexe CreditAttribution: mikedotexe at Untold Studio commentedGlad to help!