Maps API key needed for Static maps

OK, I've changed this to a feature request. The idea would be that you could configure your map formatter so that if you are displaying a static map, you could output the Google Maps API key in the URL.

Meanwhile, you can do this for your own site by overriding the simple-gmap-output.tpl.php file in your theme. To do this:

a) Copy the simple-gmap-output.tpl.php to your theme's template directory.
b) Edit the file in a text editor. Find the line that starts with:

  <img src="http://maps.googleapis.com/maps/api/staticmap?size=

and just before the size= add the following:

key=[YOURAPIKEY]&

That should use your Google maps API key.

See https://developers.google.com/maps/documentation/staticmaps/?csw=1#api_key for reference.

Oh, and you will need to clear the cache after making that change, so that Drupal will find your new theme template file.

Yes, maybe I should add something to the README file about that too (not that anyone reads README files), or maybe to the Manage Display form (which people might actually read). :)

Here's a pass at getting the api key. I've added it as a config page. For my purposes, only one Google account needs the tracking info reported from Google. This can easily be changed to set the api key in the formatter every time a field is added.

I'm open to suggestions about it.

Thanks for the patch!

However, it looks like this patch adds key=whatever to the iframe and map links calls. According to this issue it should only be for static maps, and NOT for these two cases.

Also, the key=whatever addition should only be output if the key has been configured, and left out entirely if there is no key configured.

I also am not sure I favor adding a global option to this module and a new page for it.... but that is probably OK. But if we do use a form, the one in this patch is not following the Drupal standards, and its function name should not start with _ (also it should be a system settings form rather than having a custom submit).

Thanks for the feedback. I'm new to contributing but I need this functionality so I figured I would help out others.
I'll make these updates and resubmit.

Thanks!

Sounds good!

So regarding that last point, if you're new to Drupal programming, you might not know about the Drupal 7 function system_settings_form:
https://api.drupal.org/api/drupal/modules!system!system.module/function/...

You can look at some of the places in Drupal Core that use it, for examples (there's a section on that page that says "22 calls to system_settings_form()" that will show you those places).

I've updated the patch. It's now using the system_settings_form. I didn't know about that function along with a couple guys at work who have been developing on Drupal for a few years. Learned something new :-)

If moving the key to the formatter seems better, I can do that. For my project, it was more appropriate to have a global spot to add it.

Better! The patch needs some cleanup (doesn't follow the Drupal project coding standards). Specific issues:

a) No newline at end of the .install file

b) API docs not following standards (like . at end of sentences). See
https://www/drupal.org/node/1354 for documentation standards.

c)

+function simple_gmap_menu(){
+  $items['admin/config/media/simple_gmap'] = array(
+    'title' => 'Simple Gmap Admin Settings',
+    'description' => "Manage the settings of Google Maps",

The page title violates a lot of our UI text standards: https://www.drupal.org/node/604342 and the description is not really accurate (you're not managing Google Maps really!)

d)

+  $form['simple_gmap_api_key'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Google Map API Key'),
+      '#default_value' => variable_get('simple_gmap_api_key', ''),
+      '#required' => true,
+      '#description' => t('Required. This is the API key generated from !link', array('!link'=> l(t('Google APIs Console'),'https://developers.google.com/maps/documentation/staticmaps/#api_key', array('attributes' => array('target' => '_blank'))))),
+    );

This should definitely not be a required field. Even if it was, you wouldn't put "Required" in the description. I think the field title should also say Google Maps not Google Map, and we generally use sentence case instead of title case for field titles in Drupal.

e) I'm also not sure about the placement of the admin form in the Media section of admin/config. What made you decide to put it there? See https://www.drupal.org/node/549094#configuration for discussion of the categories... sounds like maybe it belongs in Web Services?

Oh the README file probably also needs an update for this patch as well.

See #2810979: Static maps need Google Maps API key as of June/October 2016

As of several months ago, all static maps need an API key, so this is now an urgent bug report that we do not have a way to provide this.

This patch would add the apikey to the fieldformatter.

This general setting might be better suited to be in a config. What do others think about that?

I think leaving the setting as part of the formatter is fine, and in Drupal 8 it is all in configuration. I am not in favor of making a module-wide config screen just for this one setting.

Thanks for the new patch! I missed a couple of things in my review of the earlier patch... just a few minor text things to fix here:

1. +++ b/src/Plugin/Field/FieldFormatter/SimpleGMapFormatter.php
   @@ -60,6 +61,17 @@ class SimpleGMapFormatter extends FormatterBase {
   +      '#title' => $this->t('Apikey'),
   

   This should say:

   Google Maps API key

2. +++ b/src/Plugin/Field/FieldFormatter/SimpleGMapFormatter.php
   @@ -60,6 +61,17 @@ class SimpleGMapFormatter extends FormatterBase {
   +      '#description' => $this->t('Google Maps Static Maps API needs an apikey. Look at the <a href="https://developers.google.com/maps/documentation/static-maps" target="_blank">API page</a> to get one.'),
   

   How about:

   Static maps will not work without an API key. See (URL link) to learn more and obtain a key.

3. +++ b/templates/simple-gmap-output.html.twig
   @@ -24,6 +24,7 @@
   + * - apikey: The apikey needed for static maps.
   

   For the description, let's say:

   Google Maps API key (used for static maps only)

ok, good.

Regarding 2)
I wouldn't add the full URL, since it is quite long. Can we write:

• See here to learn more... or
• Look at the API documentation to learn more... or
• Have a look at the Google Static Maps API to learn more...

(emphasized words represent the link)

Oh sorry, I was unclear in my comment.

Yes, I think the link should be something like:

See the <a href="(the URL)">Static Maps API page</a> for more information.

As a note, we definitely don't want the link text to be something like "here" -- that is not accessible. See:
https://www.drupal.org/node/465106#link-text

Patch modified as discussed.

Thanks -- the latest patch looks good for Drupal 8.

Before we commit it, we have a little test procedure we run through to verify Drupal 8 patches, so we need to test this one, following:
#2543032: Steps to manually test the module.
We need to amend that procedure to include setting the maps API key for the static map.... and as a note, we don't have an automated test for the module because you need to look at the test site to see if the maps are working correctly.

So, someone other than @tjwelde should test it... I can do it later this week if no one else does first.

Then we will need to commit this patch to 8.x and make a similar patch for 7.x (and test that).

Thanks again for the patches and prompt attention to reviews!

I have tested this patch for Drupal 8 and it works fine. See also updates to #2543032: Steps to manually test the module.. Committed. So this is fixed for Drupal 8. Thanks very much!

It now needs a port to Drupal 7.

wrong status

I ported this patch to 7.x and committed it, after testing it manually following #2543032: Steps to manually test the module..

Thanks again @tjwelde!