Skip to content

add support for InfoWindows#119

Merged
frslm merged 1 commit intogmplot:masterfrom
VictorNorman:master
Jun 21, 2020
Merged

add support for InfoWindows#119
frslm merged 1 commit intogmplot:masterfrom
VictorNorman:master

Conversation

@VictorNorman
Copy link
Copy Markdown
Contributor

Add support for the user to create an InfoWindow for each marker. The InfoWindow can contain HTML, including images and links, lists, etc.

self.gridsetting = [slat, elat, latin, slng, elng, lngin]

def marker(self, lat, lng, color='#FF0000', c=None, title="no implementation"):
def marker(self, lat, lng, color='#FF0000', c=None, title="no implementation", info_window_html=""):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's rename info_window_html to info_window for brevity.

Let's also have its default value be None instead of "" to avoid mutable default values.

f.write('\t\tposition: latlng\n')
f.write('\t\t});\n')
f.write('\t\tmarker.setMap(map);\n')
f.write('\t\tmarkers.push(new google.maps.Marker({\n')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of pushing all markers to some JavaScript list, I'd recommend giving only those markers that need an InfoWindow a unique name. So something like:

var marker_0 = new google.maps.Marker({ ...

And any markers that don't need an InfoWindow can be left alone:

new google.maps.Marker({ ...

mymap.marker(37.429, -122.144, "k")
lat, lng = mymap.geocode("Stanford University")
mymap.marker(lat, lng, "red")
mymap.marker(lat, lng, "red", "<a href='https://www.calvin.edu'>Go to Calvin University</a>")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work? The 4th argument is c.

print("info_window_html transformed to ", info_window_html)
f.write("\t\t\t\tcontent: '%s',\n" % info_window_html)
f.write("\t\t\t});\n")
f.write("\t\t\tinfo_window.open(map, markers[%d]);\n" % self.numMarkers)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can actually avoid naming info_window:

new google.maps.InfoWindow({
    content: <string>
}).open(map, <marker>);

Copy link
Copy Markdown
Contributor

@frslm frslm Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, scratch that - we shouldn't be creating a new InfoWindow inside the onClick handler, otherwise each click will create a new InfoWindow on top of the previous one.

We'll have to create the InfoWindow once, give it a unique name (like we do for Markers), then reference it in the onClick handler.

f.write('\t\t\tposition: latlng\n')
f.write('\t\t}));\n')
if info_window_html != "":
f.write("\t\tgoogle.maps.event.addListener(markers[%d], 'click', function () {\n" % self.numMarkers)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of google.maps.event.addListener(), we can call addListener() on the marker directly:

<marker>.addListener('click', function () {
...
});

f.write("\t\t\tinfo_window = new google.maps.InfoWindow({\n")
# Escape every single quote and every newline in info_window_html.
info_window_html = info_window_html.replace("'", "\\'").replace("\n", "\\n")
print("info_window_html transformed to ", info_window_html)
Copy link
Copy Markdown
Contributor

@frslm frslm May 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might not need this print statement here. If you think it's worth having, then perhaps we can log it using logging.debug to stop it from cluttering the output window if users don't want to see it.


class GoogleMapPlotter(object):

numMarkers = 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define this closer to where it's used? If the same GoogleMapPlotter() object is used to draw twice, then numMarkers will be twice what it should be (this is just one example).

I'd recommend we add an id parameter to write_point(), then write_points() would define the count locally and pass the next marker ID to write_point(). If no ID is passed in (i.e. id = None), then write_point() would simply not give the rendered marker a name (as in var marker_2 = ...).

@frslm frslm added the awaiting changes Review waiting on the author for changes. label Jun 7, 2020
@frslm frslm force-pushed the master branch 5 times, most recently from b86c79b to 980d04c Compare June 9, 2020 01:36
@frslm frslm mentioned this pull request Jun 21, 2020
@frslm frslm merged commit d8db67c into gmplot:master Jun 21, 2020
@frslm frslm removed the awaiting changes Review waiting on the author for changes. label Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants