Posted by Arshad Chummun on April 17, 2010 at 7:34pm
| Project: | GMap Module |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | support request |
| Priority: | normal |
| Assigned: | Arshad Chummun |
| Status: | needs work |
| Issue tags: | extinfowindow, gmap, google maps |
Issue Summary
Hello
I've created a patch for the gmap module. This goes under feature enhancement. It enables the use of the extinfowindow to style the info window for markers. Custom theme can be easily created. I've included a tutorial in the README.txt file in the extinfowindow directory.
This patch is compatible with the latest dev version 6.x-1.x-dev - 2010-Apr-09
To install, apply the patch. Download the extinfowindow attached here. Extract under /sites/all/modules/gmap/thirdparty.
You can enable the extinfowindow at http://example.com/admin/settings/gmap .
Comments
#1
patch is missing ;-(
#2
A working example can be found here : http://www.preprocess.me/map/user
#3
My mistake. uploading patch.
#4
Wow, I'm impressed!
#5
Well I'm afraid there are still a few problems with the patch, it failed to work on current dev.
~/work/tmp/gmap > patch -p0 < gmap_6.patchpatching file gmap.module
Hunk #1 succeeded at 264 (offset 22 lines).
Hunk #2 FAILED at 273.
Hunk #3 succeeded at 1264 (offset 86 lines).
1 out of 3 hunks FAILED -- saving rejects to file gmap.module.rej
patching file js/marker.js
Hunk #1 FAILED at 44.
1 out of 1 hunk FAILED -- saving rejects to file js/marker.js.rej
This is because the patch was made on DRUPAL-6--1-0 the current stable which is pretty old.
However, I could see what was intended so I hacked the bits in to gmap.module and marker.js.
I cannot enable it under admin/settings/gmap, there is no change there nor do I see any code that would alter the settings form, so I suspect there is a bit missing to patch gmap_settings_gui.inc and provide the checkbox (or whatever) to enable extinfowindow.
If you can get that to me, or rework your patch on current dev, DRUPAL-6--1 and post it here I'll have another go ;-)
On the basis of what I've seen on your example site this is probably of interest to plenty of others, just need to get it going right.
#6
ok let me check this asap.
#7
A new patch for the DRUPAL-6--1 branch. I'm attaching the patch, the extinfowindow, and a sample working module.
Thanks
#8
Thanks for the prompt response, I'll have a look shortly.
#9
Patch is clean and it all works. It does not honor 'hide' settings but then the regular balloon does not either so not the fault of extinfowindow ;-)
This is a good addition to gmap.
#10
wow, finally!
#11
Cool! Finally someone did it! Thanks a lot!
#12
is it in dev?
#13
will this go into the module?
#14
awesome patch; thanks.
#15
it seems there's a lot on the maintainers right now but i've included both the patch and a modified dev version here we can test/use.
#16
Patch is being reviewed by rooby. He'll commit asap.
#17
Awesome functionality.
Coincidentally I just had a client site that required this, which further prompted my testing.
Seems to work well without issues using the supplied themes.
I made my own theme though (an exact copy of light) and I got unexpected results when clicking markers.
On marker click the marker had no background and the map scrolled all the way around.
I'm assuming this might just be me from other people reviews so I will see.
Has anyone else seen this behavior?
I'll have a bit better look through it when I'm at home but so far it looks great.
Another thing that would be nice is if the themes could be picked up from the sites theme instead of the gmap module. That would be better for upgrading, but it's something that can be added later, I'm just thinking out loud.
#18
Hello rooby
Thanks for your time. I had the same issue with some themes too. its not in the php but in the css. i solved it by putting a width for the main bubble container. when the width is set to auto, it tends to take infinite width and hence, lose the background and spread over the map width.
#opacity_window{width: 200px;
}
Can you try a width and let me know? if it doesnt work, i'll be glad to help with the theme.
Thanks
#19
Excellent feature patch, thank you!
#20
im bumping this because i've not seen anything on this for some weeks now. let me know where we stand.
#21
As I said in #9, This is a good addition to gmap.
Please consider weaving this in to the next dev release ;-)
#22
Finally got back to it.
I had ideas to expand on this functionality. Check out this version and let me know what you think.
Changes I've made are:
* Made it so that it looks for extinfowindow themes in the current theme in a directory called extinfowindow (this way updating the gmap module won't mess up your themes and it is more in line with how drupal theming generally works)
- If a theme in that directory has the same name as a default theme it will override the default theme. Documentation has been updated.
* Allowed theme selection in views settings so different views can use different themes
* Allowed theme selection in gmap macros (in the format extinfowindow=themename)
- GMAP-MACRO-DICTIONARY.txt has been updated.
* Code cleanup. Mostly regarding the use of spaces.
Note: with the new options it is possible to have different themes on different maps on the same page.
In this case the first theme loaded will be used for all maps to avoid any weirdness that might happen if multiple theme css files are loaded.
(I just realised that if you have the extinfowindow enabled checkbox unchecked for views they will probably still use the extwindow theme specified on the gmap settings page. - I will fix that when I get a few minutes)
#23
Also, I have seen the problem #17 again on a different system.
Even when width is set as mentioned in #18.
To reproduce:
1. Copy a theme (for example: light)
2. Rename the theme directory to test and it's css to test.css
3. Select the theme test
4. When you view your map and click the marker you get odd results. (no background on the pop-up and the map scrolls right around)
Can anyone else reproduce this?
I will investigate further soon.
[edit] Using garland as the theme.
#24
Let me test this. Thanks.
#25
Here is an updated version.
Everything is in the one patch now, just apply the patch and you're good to go.
Changes are:
* Changed so that the extinfowindow plugin does not ship with the module. You have to download it from http://gmaps-utility-library-dev.googlecode.com/svn/tags/extinfowindow/1... - There are instructions for this in the extinfowindow readme and also on the gmap settings screen. I did this so it is the same as all the existing plugins, which require the user to download them separately.
* Fixed the issue I mentioned in #22 where views will still use extinfowindow even if it is unchecked for that view. - To fix this issue I removed the theme selection option from the gmap settings screen. There is no need for this default theme setting because you can specify a theme in a gmap macro and gmap views have an option to choose a theme. So if no theme is specified it drops back to the default google maps pop-up.
- Actually, thinking about that now, maybe there could be a default theme for macros. Views probably don't need a default, although they could use one that is just used as the default value in the views settings form.
If you have any suggestions on how a default setting might work and be useful please voice them. I'm just looking to get the best functionality possible.
I still have to do some testing re the bug in #23.
If anyone else is testing this please give feedback.
It would be great to get it in the next release.
Thanks.
#26
Done some testing on patch in #25
The images are missing from dark and light themes so I copied them in and now they work OK
images and patches don't go well together for obious reasons but thought I'd mention it anyway ;-)
macro:
added |extinfowindow=light to the location_node macro, OK, ditto location_user
added |extinfowindow=light to CCK location macro, OK
followed the instructions for adding a themed infowindow to my theme, works fine'
I don't seem to have a gmap View so did not test that.
Personally I would prefer to be able to state which theme to apply to any infowindow on the whole site (global default) in gmap settings but if that is problematic so be it.
So far so good!
#27
Damn, I knew there was a reason I didn't put it all into one patch last time.
We can easily go the one default option as that was what the original patch did.
I've been thinking the same thing as you, it probably would be better with just one default to keep consistency across your website.
Good to hear the same thing from someone else for confirmation.
Might go back to a cross between the current and the original.
The one default setting from the original + the ability to have your theme in your theme instead of in the gmap folder and not including the plugin in the gmap download.
#28
Here is an update that brings it back to one default theme setting across the site.
Much simpler.
And in the interest of simplification, what do you think about the having the ability to have the extinfowindow themes your theme as opposed to in the gmap folder?
On one hand it is better for updating gmap and keeping in line with having theming in your theme.
On the other it makes the code more complex and because you when you update gmap you already have to be careful of anything you have added to the thirdparty directory so it might be ok for people to also be careful of any extinfowindow themes they might have in there.
I'm tempted by the simplicity of the original system of all in the gmap directory.
#29
I'll test this ASAP, 'real' work keeps getting in the way ;-)
#30
I have installed the version in #28 and it all works but with some interesting quirks
The extinfowindow I selected in admin/settings/gmap works, but the one I poked into my theme does not, even though it appears under the jQuery.extend(Drupal.settings,.... string. hmmm
If I remove the theme version the default continues to prevail and if I add extinfowindow to the gmap macro it is ignored too.
So Drupal.settings are being overridden
#31
Subscribing.
Can this make it to the core gmap module?
#32
Has this made it into core yet?
#33
Nope but it's right near the top of my list of things to get working.
Things outstanding for this are:
1. Re-roll for the current dev (Maybe - I'm not sure if it applies as I haven't checked in a while)
2. As far as I know, the bug in #23 is still present and it can't really go in until that is resolved. Can anyone else reproduce that bug?
3. There is a question posed in #28 regarding where people should have to put their themes.
4. Hutch had some issues in #30 that need investigation.
So after I have finished on the location/gmap single map patches and the gmap polygons patches I will come back to this one.
And then hopefully I can find some time to do some work on the location/gmap D7 versions.
#34
I will revisit #30. I will also reroll patch gmap-774078-28.patch
back as soon as I can find some time ;-)
#35
Ok I'll be on this too. Let's make this happen.
#36
Here is the fresh patch, none of the issues addressed as yet.
#37
subscribing
#38
Awesome, thanks for doing that hutch, I'll review as soon as I can.
#39
Rerolled patch for current dev, please review, this works well for me.
#40
The patch at #39 is not working with the latest dev. Any hints?
edit: nevermind, I just had to select "open info window" in gmap settings
#41
when will this come to dev version and could someone append a current dev version ?
#42
This patch runs cleanly on current CVS
#43
thx for the new patch - how can I activate the ExtInfoWindow ?
#44
You will need the tar.gz in #7
#45
Patch on latest CVS
#46
Thanks for the rerolls hutch.
I was just looking at this again for a client and saw the same problem mentioned in #17 & #23.
The problem seems to be that it breaks if the theme has a tilde (backup) file in it.
So I have the light theme with the light.css file in it and everything works as expected.
After I have edited the light.css file my directory contains:
light.css
light.css~
Now it is broken.
Then if I delete light.css~ it works properly again.
So that seems to be the issue.
I'll have a poke around for the cause of the issue later.
#47
+++ gmap.module 3 Feb 2011 16:09:37 -0000@@ -1278,6 +1283,85 @@ function gmap_views_plugins() {
+ $default_themes = file_scan_directory($default_path, '.css');
+ $themes = file_scan_directory($theme_path . '/extinfowindow', '.css');
This is the cause of that problem.
It should probably be:
+ $default_themes = file_scan_directory($default_path, '.css$');+ $themes = file_scan_directory($theme_path . '/extinfowindow', '.css$');
Powered by Dreditor.
I'll roll a new patch later today.
I'll do another patch review tonight too.
That is also the main issue I had with committing this before so hopefully it can go in soon :)
#48
I have tested the regex change and it works, I added 'dark.css~' and it was ignored.
#49
Here is the extinfowindow tarball again, with unneeded file removed and the js file cleaned up to remove tabs and lines with spaces only. Code is the same.
#50
Here is an updated version of the whole kit.
Changes are (from #45 & #49):
This is because when enabled on the main settings page all maps across the site will use extinfowindow. Then the theme can be either the default as set on the settings page, a theme specified in a macro, or a theme specified in a view.
I have tested with the supplied themes, an overridden 'light' theme in garland and a new custom theme in garland.
All seems to work for me.
The problem hutch reported in #30 may have also been due to the regexp bug.
I'll do a little more testing and see if I can make anything break.
Then I think that's all the outstanding issues.
Patch rolled against current CVS.
#51
Finally got around to looking at this, the patch is fine and it all works except that I cannot get extinfowindow_packed.js to work at all, only extinfowindow.js. In Firefox, not tested anywhere else, not using any aggregation in drupal.
The attached patch is git-style on the latest clone, you will need to use
patch -p1 < gmap-extinfowindow-774078-51.patchorgit apply gmap-extinfowindow-774078-51.patchif you are working in a git clone.#52
I found another annoying issue.
If you have custom extinfowindow themes in your theme and are using a different admin theme you will run into trouble.
When on the admin pages, where the theme selection options are, the list of themes is generated from the current theme (admin theme). But you will have your extinfowindow themes in the main theme, not the admin theme so they don't get listed.
So either that setup needs reworking or removing.
Another solution could be to have extinfowindow and it's themes in sites/all/libraries.
Also, it would be good to change all the thirdparty addons for gmap in into the libraries directory.
So maybe that can be achieved in another issue, which would then provide a solution for this too.
#53
New issue for thirdparty > libraries - #1110048: Use the libraries api module for third part extensions instead of the thirdparty directory
For reference there is also a similar issue for markers - #901596: Use libraries api module for storing markers
#54
#51 's solution worked for me with dev version. Thanks for the patch.
How to override the element's hard coded CSS? For example, if I wanted the infowindow to be opened on the right side of the marker instead of top.
#55
The positioning of the infowindow has to be calculated to allow for the bounds of the map, so this feature would have to be added to extinfowindow.js
#56
patch #51 did the job!
#57
I've applied the #51 patch works like a dream.