Add ability to have a static map

Can you provide more information on how this would work? Where would the static map come from, how would it be generated, and are there any copyright/license implications?

That seems like a fine idea to me. If you would like to work on a patch, that would be great!

Thanks! The patch will be difficult to use as it is... For future reference, the way to make patches that Drupal module maintainers can use easier is outlined here:
http://drupal.org/node/707484
If you have time to follow that process and make a new patch, that would be helpful!

If not, if you could make a zip file (or tar.gz or whatever) with the files from the module that you changed (which would be easier for me to use than the patch you made), that would be fine too. Thanks!

Thanks! That looks like a much better patch, and I can see much better what is going on now...

I can't add this patch to the module as it is though. The reason is that it makes a lot of changes to the existing formatter:
- machine name of the formatter
- machine name of the theme hook
- file name of the theme template
- machine/variable name of some settings
So anyone who is currently using the module would have errors if they upgraded.

What needs to happen is either:
a) Add a new formatter without changing the existing formatter, or
b) Add an option to the existing formatter for static vs. dynamic maps (since all the settings are the same for both, except the information bubble setting not applying to dynamic maps, that probably makes the most sense).

Actually, probably (a) is better. I just noticed that the static maps API allows you to use an API key
https://developers.google.com/maps/documentation/staticmaps/#api_key
so that should probably also be an option for the static map formatter.

I think what makes sense to me is to have another checkbox in the formatter options for "Add a static map", which would go right next to the current one for the dynamic map (and the labels, but not the machine names, on that section would be changed to say "dynamic"):

  $element['embedded_label'] = array(
    '#type' => 'markup',
    '#markup' => '<h3>' . t('Embedded map') . '</h3>',
  );

  $element['include_map'] = array(
    '#type' => 'checkbox',

// this label would change:
    '#title' => t('Include dynamic embedded map'),
    '#default_value' => (int) $settings['include_map'],
  );

  // Add a second checkbox here for the static map, saying "Include static embedded map"

   // Leave the rest the same:

   $element['iframe_width'] = array(
    '#title' => t('Width of embedded map'),
    '#type' => 'textfield',
    '#default_value' => (int) $settings['iframe_width'],
  );

And then the template can just have another variable called $include_static_map, and another if() that prints it out. I think that would be the easiest, since the link is still going to be an option.

I needed to have the ability to have static maps with this module, and also source the address from an Address Field. I'll continue to work on a non hard-coded version of the Address Field sourcing as the method I have used only works for Australian addresses.

This patch is exclusively the static maps part of those changes, reworked to fit with the comments in this thread.

It is a tickbox option as per #10 and works with the existing settings. Also added zoom option of automatic since the static maps use this.

On quick review, this patch looks good! I'll give it a more careful review/test when I can, and get it added to the module. Probably not until next week. Thanks!

It turned out that this patch had a few little problems:
a) PHP notice-level warning - when you add a new setting to a field formatter, you get unset variable warnings if you aren't careful, when viewing existing fields.
b) The Automatic zoom level of 0 you added seems to work OK for static maps, but it does not work for dynamic maps or links, so I took that out.
c) The line you added to the template file that did:

  $zoom = $zoom ? '&zoom=' . $zoom : '';

also changed the $zoom variable for the map link, which screwed that up, so it had to be done a different way.
d) The static map needed to print out & as & and it wasn't doing that.
e) You didn't include the language or the map type. But these do not seem to work the same way as the other maps, and I ran out of time...

Anyway, here's a patch with (a) - (d) fixed, but not (e). I'm not going to commit it until (e) is fixed too -- care to take another go?

Any news about this patch?
Patch posted by jhodgdon doesn't show any map...

I'm mid-stride on a reworking of this which addresses all the issues much more completely. I'm re-arranging the configuration to separate out the specific params that behave differently in a dynamic map and static map, and include most of the simple additional params for the static map.

A note on #13 - in (d) I meant to say that you need to use the entity "& amp ;" (without the spaces) to print out an ampersand in a URL.

Good old & (as &amp). I did not know about the requirement for it to be encoded in attributes and everywhere; http://www.w3.org/TR/xhtml1/#C_12. Learn something new every day.

I missed the use of the zoom variable in the link, but I think the method is still required as zoom=0 is very different from the parameter not being used at all. Since this is different functionality and the static map and dynamic link/map have this different set of values, I have taken another tack and separated out some of the functionality entirely. I have kept the existing variables unchanged so it wont break anything, and added guards against the PHP notice. The guards should potentially be added to all items.

One thing I did notice is the use of check_plain with FAPI - it already passes values through check_plain itself so these aren't needed. I've left existing ones in place. The (int) casting should also not be really be needed? The contents of these variables is guaranteed to be TRUE or FALSE, and if someone is playing with variables to break that, they're doing it from outside the interface and deserve what they get. The code even casts twice inside the same function!

Anyway, as a result of all that, the new settings page has been re-arranged into 4 different sections - Embeded Dynamic and Linked map, Embeded Static map, General settings, Link to map - and I have added quite a few new options. This allows the admin to configure the static map, and if it's linked to a dynamic map, also control it's parameters.

I kinda went a little bonkers on changing things so there are a lot of them. Also changed the order of the array keys in the parameters sent to the theme function so that they're in the same order everywhere so that's even more "empty" changes.

The theme function has become ungainly. Three solution spring to mind - replace with render trees; split into three different theme functions for dynamic, static, and link; or move the URL processing into the prep function and pass query params to theme function. Or a combination of all three. Unfortunately, I have exceeded the time I had available for this and I now need to return to work.

Yeah, this patch got a little bit away from me but hopefully it's mostly complete. I've run out of time to work on this so I may have to leave it to someone else to make any changes.

One note: The reason that only some isset() types of checks are needed is that if you add a new setting, people who previously had the module installed and had set up this formatter will see those errors when they update. But for the settings that were present when the module was first installed and formatter was first set up, the isset() is not necessary.

Regarding this patch: It is generally a bad idea to do any kind of logic or programming in the tpl.php file -- the tpl.php file should just have print statements basically, possibly if($foo) print $foo; but nothing more complicated. So all of that programming/logic you put in there needs to be moved into the preprocessing function, and only the DIV you added should be part of the tpl.php file. That will also (hopefully) reduce the number of variables you pass into the tpl.php file.

Also... the patch is hard to follow because you've done so much rearranging of the original code. It is really not helpful to do that -- it makes it really hard to review. Can you please rearrange things back to how they were? For instance:

-      'include_link' => 0,
-      'include_text' => 0,
-      'iframe_height' => 200,
-      'iframe_width' => 200,
       'zoom_level' => 14,
-      'information_bubble' => 1,
-      'link_text' => t('View larger map'),

This makes it look like you've taken out those 6 settings, but really you just moved them somewhere else.

And also please don't make changes like this:

-  $element['embedded_label'] = array(
+  $element['embedded_dynamic_label'] = array(

Just leave the code that you don't need to change alone. This kind of change can screw things up for people who have done custom theming or form alters etc.

I really had trouble reviewing and evaluating this patch because of these issues. Please make the changes noted here to get the code back to what it was and the patch just including the changes necessary for this feature, and then I'll give it a proper review. Thanks!

There a big comment about the code in the template file deriding myself for having it there but I was trying to keep with the same methodology of passing a zillion variables to the template file. TBH, I actually wanted to completely re-do the entire theming side of it to make it a render tree + 3 theme functions, one for each display type of dynamic, static, and link, which is a bit more along D7 display methods. Unfortunately, that really would stuff anyone using the module around and would probably require bumping the major release number.

The re-arranging of the ordering of variables is to keep everything in the same order throughout the file. I don't have all my changes separated out into changes, re-ordering, etc so I'd have to re-do the changes from scratch and provide a number of sequential patches.

In that redone world, the isset's I'd most likely completely remove by merging the whatever is saved with a fixed array of defaults and thus guaranteeing the array key will always be present.

The 'embedded_dynamic_label' is a flat out goof. That shouldn't have been done.

Unfortunately, I'm time poor at the present so I'll have to get back to this another day. I'd be interested in your thoughts regarding the change in how theming/output is done in the mean time though.

I'm not all that interested in splitting the theme template up, because normally, field formatters have a single template. It's really only about 10 lines as it is anyway for all 3 versions (as long as the logic is gone).

I don't understand why patch #17 is so divergent from #13, which was almost done save one bullet.
Here's a reroll of #13, with the addition of #13.e

I've added 3 lines of code in the .module and changed 1 line in the .tpl to support the difference in map types.
The available types are not different, only the keys.
A simple associated array is used to translate between types - no additional fields are necessary on the widget settings.

(Language support appears to already have been included in #13, and is not different between embed and static. See https://developers.google.com/maps/documentation/staticmaps/#URL_Parameters)

Thanks! That all looks right.... I'll see if I can give it some testing in the next few days (well, probably after Thanksgiving, so next week), and if it is all working, I'll add it to the module. (Assigning to myself for testing, so I don't forget -- I'm a rather busy person...)

Oh, and regarding the language... I think (vague recollection) that in #13 when I said language wasn't included, I actually meant that it wasn't working. In the dynamic maps and map links, for instance, you can set the language to a two-letter code like "de" or "jp" and the language of some things displayed on the maps will change, but I don't think the static maps work with that type of language code... I am not sure, will need to test. Actually, with a static map I am not sure what "language" even means, since there are no controls for zooming etc.?

#17 is so divergent from #13/#11 since because I had to revisit it, I added all of the knobs that static maps has. I also re-arranged the UI for the user so it made sense and then re-arrange the stored variable arrays to match the new ordering too.

This includes the language which does work on certain regional maps to translate the street names. Zoom is fixed but relevant on static maps but has slightly different behaviour than dynamic so was split out. Also added scale, API key, map type (different again), marker size; colour; label.
https://developers.google.com/maps/documentation/staticmaps/

The logic in the template file is actually no different from the code already in there - it's turning things like $static_zoom into '&zoom=14' or '' for use in the URL but it does it on a separate line instead of inline so that the URL line doesn't go for 4000 characters.

#17 is a SIGNIFICANTLY better patch than #13/#11 and it's improvements should be the ones to be looking at it.

OK... I have already outlined why I am not going to look further at #17 until the patch is fixed up, and apparently #21 is not good either as it's missing functionality. If someone wants to submit a good patch, I'll look at that.

the patch in #21, like i said, is an update of #13, and I tried to make as few changes as possible that would enable static maps support.
i'm not sure what functionality it's missing, since jhogdon indicated that the only thing missing in #13 was map types and language.

it seemed to me that language was working as expected, and my testing of the patch in #21 proved satisfactory in the limited testing i performed.
if jhogdon can spell out what is missing, i'll take another shot at updating it.

re other features: I started by re-working #17, but realized about halfway through that it unnecessarily created an entirely separate -- and unnecessary -- widget UI for static maps configuration. The underlying functionality of Google Embedded Maps and Google Static Maps API is almost identical, and there's simply no reason to separate them. So i went back up the thread to #13, which seemed to be aiming towards minimum viability.

if #25 is asking for an implementation of all static maps API features before considering a patch -- scale, api key, marker color, etc. -- that's not something that interests me and someone else can take a stab.

Addition to this feature request: Make an option to have the map itself be a link to Google Maps.

Attaching a patch rerolled against current head

Edit: This is a reroll of #13

I gave this latest patch a thorough test today, and I think it is fine. It doesn't introduce any errors when upgrading the module, and it works well. So I went ahead and added it to the -dev version of the module (which you can get now via revision control/git, or wait a few hours until the next package of the -dev zip files, or wait until I get around to releasing a new version of the module).

Thanks everyone!

Well, I spoke too soon. This patch broke
#1866434: Allow width to be specified in % for responsive designs
so I had to make a modification today. See that other issue for the patch, which is in the repository and will be in the -dev version in about 12 hours.

The problem is that static maps only accept integers for the width, while dynamic maps will accept CSS width designations, and this issue was taking out the '%' etc. for dynamic maps.

Anyway, it's fixed now...

Addition to this feature request: Make an option to have the map itself be a link to Google Maps.

This never got implemented did it?

No, it never did, sorry! We should probably have a new issue: #2320553: Add ability for static map to link to Google maps

Thanks. I think it's a very common functionality to have on a static map.