Code

Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#9204 closed (fixed)

Google Maps Marker to change the Icon

Reported by: qingfeng Owned by: jbronn
Component: GIS Version: 1.0
Severity: Keywords: geodjango google map icon gicon
Cc: qingfeng@…, prairiedogg99@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Google Maps Marker to change the Icon

marker = GMarker(str(k.point),icon="http://gmaps-samples.googlecode.com/svn/trunk/m
arkers/blue/blank.png")

Attachments (7)

overlays.patch (1.1 KB) - added by qingfeng 6 years ago.
gicon1.patch (2.6 KB) - added by qingfeng 5 years ago.
GIcon v1.0
icons_and_draggable.diff (5.8 KB) - added by prairiedogg 5 years ago.
GIcon wrapper class with the draggable markers bit from #10072
more_complete_gicon_patch.diff (9.3 KB) - added by prairiedogg 5 years ago.
Improved the documentation for GIcon class and did a slight refactor inside overlays.py to reduce repetition
slightly_updated_gicon_patch.diff (9.4 KB) - added by prairiedogg 5 years ago.
Patch after jbronns most recent update to trunk
global_iconvars_gicon_patch.diff (8.4 KB) - added by prairiedogg 5 years ago.
Moved icon variable definition into the base google maps js template alongside the other global variables.
stand_alone_icon_generation.diff (1.5 KB) - added by prairiedogg 5 years ago.
Broke the statement that generates a maps icons from its marker list into its own stand alone method so that it may be called multiple times during the object's life cycle

Download all attachments as: .zip

Change History (26)

Changed 6 years ago by qingfeng

comment:1 follow-up: Changed 6 years ago by jbronn

  • Keywords google map icon gicon added
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to jbronn
  • Patch needs improvement unset

Your patch looks decent, what do you think about a GIcon class in Python?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by qingfeng

Replying to jbronn:

Your patch looks decent, what do you think about a GIcon class in Python?

GIcon is a javascript object:
http://code.google.com/intl/en/apis/maps/documentation/reference.html#GIcon

Marker of Google Maps can be customized to support the picture

comment:3 Changed 6 years ago by qingfeng

  • Cc qingfeng@… added

comment:4 in reply to: ↑ 2 Changed 5 years ago by jbronn

GIcon is a javascript object:
http://code.google.com/intl/en/apis/maps/documentation/reference.html#GIcon

I know GIcon is a JavaScript object -- I'm saying, do you think it would be appropriate to have a Python wrapper for GIcon (e.g., like the existing wrappers for GPolygon and GPolyline).

comment:5 Changed 5 years ago by qingfeng

This is a new patch:), add GIcon and doctest

>>> GIcon("http://gmaps-samples.googlecode.com/svn/trunk/markers/blue/blank.png")
<django.contrib.gis.maps.google.overlays.GIcon object at 0x132c6d0>

>>> "%s"%GIcon("http://gmaps-samples.googlecode.com/svn/trunk/markers/blue/blank.png")
'icon: eval("icon=new GIcon(G_DEFAULT_ICON);icon.image=\'http://gmaps-samples.googlecode.com/sv
n/trunk/markers/blue/blank.png\';icon")'

Changed 5 years ago by qingfeng

GIcon v1.0

Changed 5 years ago by prairiedogg

GIcon wrapper class with the draggable markers bit from #10072

comment:6 Changed 5 years ago by prairiedogg

I used a different patch (icons_and_draggable.diff) on my local install when I needed to add multiple GIcons to a map, so I thought I'd upload it as an alternative approach. This patch includes a GIcon class which more or less wraps Google's js GIcon class. It also includes some changes to the default map js template to include the icon declarations before the marker declarations so that the markers can be instantiated with the various icon instances.

The API for instantiating GIcons in this patch is slightly different than the one in the existing patch because I made it for the purposes of creating multiple types of icons to go on a single map. I'm sorry this patch also includes the dragable marker bit from #10072. If I'm breaching etiquette here please let me know, I'm new to contributing to OSS.

Look forward to everyone's suggestions!

comment:7 Changed 5 years ago by prairiedogg

OK, that patch certainly works but there's no documentation. I'm happy to clean this up if folks think this is the direction we want to go in.

I think I've come up with one slight API change, I'd appreciate any input on it.

Right now you can instantiate a default icon by with just GIcon() and that will use google's default icon, and specify the variable name "custom" ("_icon" is appened on the default template to try and minimize the possibility of js namespace collision). I think users should be required to specify at least a js variable name when instantiating a GIcon, so as to further minimize the possibility of name space collisions. The new syntax would be GIcon('myvar'), GIcon('var2'), at a minimum, just GIcon() would raise an error, not enough args.

The other keyword arguments wold remain optional.

I'm guessing there's a way for the GIcon class to keep track of all of its instantiated objects and prevent 2 of them from having the same name as well, but that might be overkill. Thoughts?

Changed 5 years ago by prairiedogg

Improved the documentation for GIcon class and did a slight refactor inside overlays.py to reduce repetition

comment:8 Changed 5 years ago by prairiedogg

I just submitted "more_complete_gicon_patch.diff", this implements the previously alluded name argument (although unique names are not enforced). I also did a very small refactor of the overlay argument -> instance variables in overlays.py.

Also, when I ran part of my patch by someone on #python they made 2 suggestions, which I'll just throw out here.

The first was that def statements should not assign empty mutable types, see:

http://effbot.org/pyfaq/why-are-default-values-shared-between-objects.htm

So I changed my code to incorporate that suggestion, I notice that there are some other places that could be updated, but I'll leave it to others who are more knowledgeable to make that determination.

The second suggestion was that the isinstance() method of type checking was not very pythonic. I don't know if that bothers you or not Justin, just thought I'd throw it out there.

Would love to hear any comments.

comment:9 Changed 5 years ago by prairiedogg

  • Cc prairiedogg99@… added

comment:10 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:11 Changed 5 years ago by ericholscher

  • Triage Stage changed from Unreviewed to Accepted

comment:12 Changed 5 years ago by jbronn

  • milestone set to 1.1

Changed 5 years ago by prairiedogg

Patch after jbronns most recent update to trunk

comment:13 Changed 5 years ago by prairiedogg

My last patch is an attempt to bring GIcon up to date after jbronn's most update to trunk. I didn't have a lot of time to work on it, so there's a compromise as a result of the recent addition of js modules and map dom_id namespaces.

Since the addition of modules to the js, markers are, on the default template, added to the {{ js_module }}.

This happens on the template, that is to say, the python marker object doesn't "know" what js namespace it belongs to, which is probably a good thing.

The issue is when GIcons are put as a list into the the Gmarker.icons attribute, they don't "know" to what js_module the marker belongs, and since the marker doesn't necessarily "know" either, there doesn't seem to be a simple way to just call a "render" method on the GMarker and have a correctly namespaced GIcon variable pop out.

The solution was to simply forgo namespacing altogether on the GIcons. In the default maps template they're declared as variables (the name is suffixed by default with "_icon") and referenced directly by that name in the rest of the js template code.

I don't really mind this per-se. I think everyone's use cases are going to vary, I can't anticipate it all, and this will work for simple cases out of the box. I think it makes sense to keep icons and markers "independent" in the python to allow folks who want to do crazy off the beaten path stuff the flexibility to by writing their own templates. My only concern for this patch is that the js variable naming scheme is internally inconsistent. I'll let the maintainers decide if it merits a re-think and I'd be happy to contribute if it does.

Currently markers don't know what map they're assigned to (at least in the python), and that's probably good thing. Icons are the same way (user has to assign them to a marker manually in their view code). My solution was to forgo namespacing the icon object and just declare them as variables, so when the marker renders itself, it doesn't have to know its own ancestry to reference an icon.

I think icons existing outside of map namespace is probably a good thing, because its quite conceivable that folks might want to re-use the same icons on different maps. just my two cents.

Changed 5 years ago by prairiedogg

Moved icon variable definition into the base google maps js template alongside the other global variables.

comment:14 Changed 5 years ago by prairiedogg

After discussion with jbronn, moved icon definitions into the google-base.js template along with the other global variables.

comment:15 Changed 5 years ago by jbronn

  • Resolution set to fixed
  • Status changed from new to closed

(In [10021]) Fixed #9204 -- Added GIcon overlay, allowing customization for icons of GMarker objects. Thanks to qingfeng for initial ticket/patch, and to prairiedogg for final implementation.

comment:16 Changed 5 years ago by jbronn

I modified up the final patch a small bit. I found it redundant to specify markers on both GoogleMap and GMarker, so I removed the icons keyword from GoogleMap.__init__() and have it construct its icons list from the icons (if any) given on the markers. I also changed GIcon.name to GIcon.varname.

Changed 5 years ago by prairiedogg

Broke the statement that generates a maps icons from its marker list into its own stand alone method so that it may be called multiple times during the object's life cycle

comment:17 Changed 5 years ago by prairiedogg

  • Resolution fixed deleted
  • Status changed from closed to reopened

The last minute changes that jbronn did on this patch were correct, removing the icons declaration from the GMap object reduced redundancy, but it broke some of my code because I have cases where I add markers to a GMap instance after instantiation. Since jbronn's patch sets self.icons with a statement in init, any markers with new icons after the Gmap's instantiation won't show up when Gmap.render() is called.

To solve this issue I made a small patch that generates a map instance's icon list twice. Once at instantiation and again at "runtime" (in Gmap.render() ) when the icons are written into js variables on the template (or called at any time manually in client code). It may be possible to remove the method call from init altogether and just calculate which icons belong in the Gmap instance when the instance is rendered. I'll let another pair of eyes go over this one.

comment:18 Changed 5 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

Please open a new ticket for new problems. This issue has been fixed and it confuses the history of the ticket to report a related, but not the same, problem in the same ticket.

comment:19 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.