Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Request for comments #2

Open
wants to merge 18 commits into from

2 participants

@gipi

In this branch I tried to implement some ideas

Media files

Previously the javascript files was included directly in the easy_maps/map.html template but when more than one {% easy_map %} was used, multiple instances of Google maps javascript could create problems (chrome console complains about this).

In @1f0a767 I used the Media class for the widget used in the rendering of the address field.

Settings

Added (@8f71c84 and @bf15559 to be squashed together) a settings.py containing the default to be used when a new instance is created (also used to center the map). Together with the changes in @39645fd is possible to avoid the saving of an instance with an empty address.

Interactivity

I don't like that for see where the marker will be placed I need to save the instance and come back, so I added some javascript functionality: first of all the update button that queries the geolocalization URL (a custom view using JSON that responds with the data used to full the form field)

em1

This is introduced in @5ca4a70 where a geo/ URL is added to the Admin url patterns.

Also I would like to have the coordinates updated when I move the marker on the map (see @d1a0d5a).

Inlines

An admin class to be used with inlines is added with the same functionalities descripted above, this has required some modification on how functions and HTML element classes are named.

There is some example models added in @d1a0d5a that use it.

Multiple markers

The easy_maps tag is improved so to accept also a list of Address instances that will be placed in the map (see @39645fd).

Change list

Since the primary data of this application is geographical data I would like to have a change list page in the admin that contains a map at a zoom level such that all the point are immediately visualized. This (@1755f7d) is a very dirty hack and in future I'd love to implement some interaction between the normal Django table entries and the markers.

Conclusion

This is not code I'm proud of, it's a little messy and should be reorganized but to avoid useless work I need to know what do you think about these ideas.

@kmike
Collaborator

Hi Gianluca,

Thanks for working on this! I'll comment on features now.

Media files

Currently, it is possible to use a custom template with nulled api_js block, e.g.:

# my_map.html
{% extends "easy_maps/map.html" %}
{% block api_js %}{% endblock %}

# user template
<script type="text/javascript" src="https://maps.google.com/maps/api/js?sensor=false"></script>

{% easy_map ... using "my_map.html" %}
{% easy_map ... using "my_map.html" %}

This doesn't require using custom widget for a form field, and this also doesn't require using a form.
1f0a767 brokes backwards-compatibility by requiring user to include <script> tag manually or using a form with AddressWithMapWidget.

I don't want django-easy-maps to be tied to forms, and I really like that it is possible to just drop {% easy_map "<My address>" 300 300 %} and get a map (e.g. at a "Contacts" page).

Currently, the inlined javascript may become an issue if multiple instances of the same map are displayed (because the template is parametrized by map.pk), but this is rarely an issue (and this limitation still apply in your branch).

But using Media in admin widget is a good idea. Maybe use an another template while rendering widgets, the one which assumes the external javascript is loaded? It is also better to put easy_maps javascript files to static/easy_maps/js or static/easy_maps, not to static/js because the latter may clash with user files.

Settings

+1 for defaults in settings, this may be useful.

Interactivity

This is a really useful feature, but I think it is better to move work to the client side. Shouldn't we use javascript geocoder instead of geopy/json api in this case ( see https://developers.google.com/maps/documentation/geocoding/#ReverseGeocoding )? I'd prefer the interactivity to be implemented purely in javascript.

Inlines

+1

Multiple markers

I like the idea. Currently easy_maps tag accepts either string or Address instance; is it possible to have multiple markers with just strings or form fields? What should syntax look like?

Change list

I like the idea.


By the way, https://github.com/gipi/django-easy-maps/blob/list-addresses/easy_maps_tests/test_app/models.py is not how django-easy-maps was intended to be used :) The idea was to write

class Shop(models.Model):
    address = models.CharField(max_length=255)
    brand = models.ForeignKey(Brand)

(that's what existing sites probably have) and then use {% easy_map shop.address 300 300 %} tag to render the map for shop's address; Address model is more like a geocoder cache with some fine-tuning options.

@gipi

Thanks for your reply.

Address model is more like a geocoder cache with some fine-tuning options.

this explains a lot to me and I can say that my intention was to improve the admin-side of this application :)

Media Files

IHMO is better to avoid to include the Google's script in the widget and to tell the user to include it in the main template, otherwise we could add an optional version of the tag like

{% easy_map address 300 300 with_js=False %}

(using a custom template for a common case is annoying).

1f0a767 brokes backwards-compatibility by requiring user to include script tag manually or using a form with AddressWithMapWidget.

could be milestoned for a next major version a change in that, maybe indicating it in the documentation, obviously if you are ok with this.

Currently, the inlined javascript may become an issue if multiple instances of the same map are displayed (because the template is parametrized by map.pk), but this is rarely an issue (and this limitation still apply in your branch).

I don't understand what you mean, could you make an example?

It is also better to put easy_maps javascript files to static/easy_maps/js or static/easy_maps, not to static/js because the latter may clash with user files.

ok, I'll commit this

Interactivity

If we move the geocoding code client side (hence removing geo.py) can be problematic in browsers without javascript enabled.

Multiple markers

I like the idea. Currently easy_maps tag accepts either string or Address instance; is it possible to have multiple markers with just strings or form fields? What should syntax look like?

{% easy_map "address1" "address2" 300 300 %}

I don't understand the form fields stuffs, could you elaborate that?

BTW since there are some things you are ok with, you could indicate me what commits keep (or modify) so that I can reorganize this branch with only them in it in order to allow you to merge and maybe you could open an issue for each argument in this discussion with the guideline of implementations.

@kmike
Collaborator

Hi @gipi and sorry for a long delay!

Media files

I like the idea of "with_js" attribute. However there are 3 possibilities:

a) include all the js (script tag with a link to Google and our js map initialization code);
b) include only our map init code;
c) include nothing.

"with_js" should do (b) - but then it needs a better name because some js would still be included :) Any ideas?

I think we shouldn't change the default (with_js=True) though.

Currently, the inlined javascript may become an issue if multiple instances of the same map are displayed (because the template is parametrized by map.pk), but this is rarely an issue (and this limitation still apply in your branch).
I don't understand what you mean, could you make an example?

I mean passing the same Address instance to several easy_maps tags on the same page. They would have the same pk and js code may become incorrect. I don't know if this is a real-life issue.

Interactivity

Interactivity wouldn't work without js anyway, would it? Client-side requests make more sense because they provide the same capabilities and don't block the server. I think we should do geocoding (user enters the address) both on server side (when interactivity is not available or the address is hard-coded in template) and on client side (when interactivity is enabled), and we should do reverse geocoding (when user clicks the map) only on client side; the UI is a tricky part for geocoding. I think the implementation may include some hidden fields.

Multiple markers

I mean "model fields" :) The syntax for static addresses looks good for me. But what with querysets? For example, we have some Shops in database; each shop has an "address" TextField. How to create a map with their addresses on the same map?


I think that settings for the default map center are ready (btw, I like EASY_MAPS_CENTER = (45.0677201, 7.6825531) more, but feel free to EASY_MAPS_CENTER_LAT if you want); other things need some work. It'll be great if you split the features so that we can discuss them faster.

@gipi

ok, I'll create a branch of each feature; I think for the end of the week maybe :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 2, 2013
  1. @gipi

    Make the marker draggable.

    gipi authored
  2. @gipi
  3. @gipi
Commits on Jan 3, 2013
  1. @gipi

    Allow to update field values without saving.

    gipi authored
    Using an AJAX call we can obtain geolocalized data without saving the model.
    The admin add a 'geo/' URL to call.
  2. @gipi

    Show map also on first editing and set configurable defaults using

    gipi authored
    its own settings file.
  3. @gipi

    Update README.

    gipi authored
  4. @gipi
  5. @gipi

    Add example models.

    gipi authored
  6. @gipi
  7. @gipi

    Put the javascript files into the Media class.

    gipi authored
    In this way is avoid the multiple inclusion of the Google API
    javascript file.
  8. @gipi
  9. @gipi

    Remove useless fields.

    gipi authored
  10. @gipi

    Remove useless dictionary entry.

    gipi authored
  11. @gipi

    Use relative imports.

    gipi authored
  12. @gipi

    Factorize form definition.

    gipi authored
  13. @gipi

    Remove single marker JS code.

    gipi authored
  14. @gipi
  15. @gipi

    Extends change list admin template to show saved addresses on the map.

    gipi authored
    Also modify the Media class.
This page is out of date. Refresh to see the latest.
View
11 README.rst
@@ -17,7 +17,9 @@ Installation
pip install django-easy-maps
Then add 'easy_maps' to INSTALLED_APPS and run ``./manage.py syncdb``
-(or ``./manage.py migrate easy_maps`` if South is in use)
+(or ``./manage.py migrate easy_maps`` if South is in use). Since there are
+some media files needed to be used, you have to collect the static files
+distributed with this application (using ``./manage collectstatic``).
Settings
========
@@ -27,6 +29,13 @@ then create a EASY_MAPS_GOOGLE_KEY in your settings.py file::
EASY_MAPS_GOOGLE_KEY = "your-google-maps-api-key"
+If you need a place where center the map when no address is inserted yet add the
+latitudine and longitude to the EASY_MAPS_CENTER_* variables in your settings.py
+like the following::
+
+ EASY_MAPS_CENTER_LAT = -41.3
+ EASY_MAPS_CENTER_LON = 15.2
+
Usage
=====
View
55 easy_maps/admin.py
@@ -1,16 +1,59 @@
from django.contrib import admin
from django import forms
+from django.conf.urls.defaults import patterns, url
+from django.http import HttpResponse, HttpResponseBadRequest
+from django.views.decorators.csrf import csrf_exempt
from .models import Address
from .widgets import AddressWithMapWidget
+from .geo import geolocalize
+
+import simplejson
+
+class AddressAdminForm(forms.ModelForm):
+ class Meta:
+ widgets = {
+ 'address': AddressWithMapWidget({'class': 'vTextField'})
+ }
class AddressAdmin(admin.ModelAdmin):
- list_display = ['address', 'computed_address', 'latitude', 'longitude', 'geocode_error']
- list_filter = ['geocode_error']
search_fields = ['address']
+ form = AddressAdminForm
+ class Media:
+ js = (
+ 'https://maps.google.com/maps/api/js?sensor=false',
+ 'js/easy_maps.js',
+ )
+
+ def get_urls(self):
+ """Add a view that serves geolocalized data on POST request
+ """
+ urls = super(AddressAdmin, self).get_urls()
+ my_urls = patterns('',
+ url(r'^geo/$', self.admin_site.admin_view(self.get_geo), name='address_json'),
+ )
+ return my_urls + urls
- class form(forms.ModelForm):
- class Meta:
- widgets = {
- 'address': AddressWithMapWidget({'class': 'vTextField'})
+ # FIXME: add CSRF protection look at https://docs.djangoproject.com/en/1.4/ref/contrib/csrf/#ajax
+ # for example in passing a CSRF token
+ @csrf_exempt
+ def get_geo(self, request):
+ """Return a json that will be used to insert correct value
+ into the model form.
+ """
+ if request.method != "POST" or not request.POST.has_key('address') or request.POST['address'] == '':
+ return HttpResponseBadRequest()
+
+ computed_address, latitude, longitude, geocode_error = geolocalize(request.POST["address"])
+ return HttpResponse(simplejson.dumps(
+ {
+ 'computed_address': computed_address,
+ 'latitude': latitude,
+ 'longitude': longitude,
+ 'geocode_error': geocode_error,
}
+ ), content_type='application/json')
+
+class AddressInlineAdmin(admin.StackedInline):
+ extra = 1
+ form = AddressAdminForm
View
20 easy_maps/geo.py
@@ -0,0 +1,20 @@
+from django.conf import settings
+from django.utils.encoding import smart_str
+
+from geopy import geocoders
+
+def geolocalize(address):
+ """From an address return the values needed to fullify an Address model form
+ """
+ try:
+ if hasattr(settings, "EASY_MAPS_GOOGLE_KEY") and settings.EASY_MAPS_GOOGLE_KEY:
+ g = geocoders.Google(settings.EASY_MAPS_GOOGLE_KEY)
+ else:
+ g = geocoders.Google(resource='maps')
+ s_address = smart_str(address)
+ computed_address, (latitude, longitude,) = g.geocode(s_address, exactly_one=False)[0]
+ geocode_error = False
+ except (UnboundLocalError, ValueError,geocoders.google.GQueryError):
+ geocode_error = True
+
+ return computed_address, latitude, longitude, geocode_error
View
35 easy_maps/models.py
@@ -1,29 +1,22 @@
-from django.conf import settings
from django.db import models
-from django.utils.encoding import smart_str
-from geopy import geocoders
+
+from .geo import geolocalize
+from . import settings
+
class Address(models.Model):
address = models.CharField(max_length=255, db_index=True)
computed_address = models.CharField(max_length=255, null=True, blank=True)
- latitude = models.FloatField(null=True, blank=True)
- longitude = models.FloatField(null=True, blank=True)
+ latitude = models.FloatField(default=settings.EASY_MAPS_CENTER_LAT, null=True, blank=True)
+ longitude = models.FloatField(default=settings.EASY_MAPS_CENTER_LON, null=True, blank=True)
geocode_error = models.BooleanField(default=False)
def fill_geocode_data(self):
if not self.address:
self.geocode_error = True
return
- try:
- if hasattr(settings, "EASY_MAPS_GOOGLE_KEY") and settings.EASY_MAPS_GOOGLE_KEY:
- g = geocoders.Google(settings.EASY_MAPS_GOOGLE_KEY)
- else:
- g = geocoders.Google(resource='maps')
- address = smart_str(self.address)
- self.computed_address, (self.latitude, self.longitude,) = g.geocode(address, exactly_one=False)[0]
- self.geocode_error = False
- except (UnboundLocalError, ValueError,geocoders.google.GQueryError):
- self.geocode_error = True
+
+ self.computed_address, self.latitude, self.longitude, self.geocode_error = geolocalize(self.address)
def save(self, *args, **kwargs):
# fill geocode data if it is unknown
@@ -38,3 +31,15 @@ class Meta:
verbose_name = "EasyMaps Address"
verbose_name_plural = "Address Geocoding Cache"
+ def json(self):
+ """Returns a JSON representation of the address data to be used
+ with the javascript in a template.
+ """
+ import simplejson
+ dic = {
+ 'address': self.address,
+ 'computed_address': self.computed_address,
+ 'latitude': self.latitude,
+ 'longitude': self.longitude,
+ }
+ return simplejson.dumps(dic)
View
4 easy_maps/settings.py
@@ -0,0 +1,4 @@
+from django.conf import settings
+
+EASY_MAPS_CENTER_LAT = getattr(settings, 'EASY_MAPS_CENTER_LAT', -34.397)
+EASY_MAPS_CENTER_LON = getattr(settings, 'EASY_MAPS_CENTER_LON', 150.644)
View
59 easy_maps/static/js/easy_maps.js
@@ -0,0 +1,59 @@
+// will contain the boundary of the map.
+var g_lat_long_bound = new google.maps.LatLngBounds();
+
+function easy_maps_set_form_value(id_prefix) {
+ return function (computed_address, lat, lng, error) {
+ document.getElementById(id_prefix + 'computed_address').value = computed_address;
+ document.getElementById(id_prefix + 'latitude').value = lat
+ document.getElementById(id_prefix + 'longitude').value = lng;
+ document.getElementById(id_prefix + 'geocode_error').value = error;
+ };
+}
+
+function easy_maps_bind_button (id_prefix) {
+ django.jQuery.post(
+ // FIXME: this is hardcoded
+ '/admin/easy_maps/address/geo/', {
+ //'{% url admin:address_json %}', {
+ 'address': document.getElementById(id_prefix + 'address').value
+ },
+ function(data) {
+ easy_maps_set_form_value(id_prefix)(
+ data["computed_address"],
+ data["latitude"],
+ data["longitude"],
+ data["geocode_error"]
+ );
+ var center = new google.maps.LatLng(data["latitude"], data["longitude"]);
+ marker.setPosition(center);
+ map.setCenter(center);
+ }
+ );
+
+ return false;
+}
+
+function easy_maps_add_listener(id_prefix, marker) {
+ // update the coordinate on marker dragging
+ google.maps.event.addListener(marker, 'dragend', function(evt) {
+ var ll = marker.getPosition();
+ // FIXME: fix id names
+ document.getElementById(id_prefix + 'latitude').value = ll.lat();
+ document.getElementById(id_prefix + 'longitude').value = ll.lng();
+ });
+}
+
+function easy_maps_add_marker(map, marker) {
+ var latlng = new google.maps.LatLng(marker.latitude, marker.longitude);
+ var marker = new google.maps.Marker({
+ position: latlng,
+ map: map,
+ draggable: true,
+ title: marker.address
+ });
+
+ // add marker's coordinate to the boundary
+ g_lat_long_bound.extend(latlng);
+
+ return marker;
+}
View
6 easy_maps/templates/admin/easy_maps/change_list.html
@@ -0,0 +1,6 @@
+{% extends "admin/change_list.html" %}
+{% load easy_maps_tags %}
+{% block result_list %}
+ {{block.super}}
+ {% easy_map cl.query_set 900 700 %}
+{% endblock %}
View
47 easy_maps/templates/easy_maps/map.html
@@ -1,21 +1,15 @@
-{% with map.latitude|stringformat:"f" as lat %}
-{% with map.longitude|stringformat:"f" as long %}
-
-{% block api_js %}
- <!-- Google Maps API javascript -->
- <script type="text/javascript" src="https://maps.google.com/maps/api/js?sensor=false"></script>
-{% endblock %}
+{% load easy_maps_tags %}
+{% with latitude|stringformat:"f" as lat %}
+{% with longitude|stringformat:"f" as long %}
{% block html %}
<!-- HTML map container -->
- <div id="map-canvas-{{ map.pk }}"
- {% if width and map.latitude and not map.geocode_error %}
- style="width: {{ width }}px; height: {{ height }}px;"
- {% endif %}
+ <div id="map-canvas-{{ id }}"
+ style="width: {{ width }}px; height: {{ height }}px;"
class="easy-map-googlemap">
{% block noscript %}
<noscript>
- <img alt="Map of {{ map.address }}" src="https://maps.google.com/maps/api/staticmap?center={{ lat }},{{ long }}&zoom={{ zoom }}&markers={{ lat }},{{ long }}&size={{ width }}x{{ height }}&sensor=false">
+ <img alt="Map of {{ address }}" src="https://maps.google.com/maps/api/staticmap?center={{ lat }},{{ long }}&zoom={{ zoom }}&markers={{ lat }},{{ long }}&size={{ width }}x{{ height }}&sensor=false">
</noscript>
{% endblock noscript %}
</div>
@@ -24,10 +18,9 @@
{% block map_js %}
<!-- Map creation script -->
<script type="text/javascript">
- function initialize_map_{{ map.pk }}() {
+ function initialize_map_{{ id_safe }}() {
var latlng = new google.maps.LatLng({{ lat }}, {{ long }});
- var mapElem = document.getElementById("map-canvas-{{ map.pk }}");
-
+ var mapElem = document.getElementById("map-canvas-{{ id }}");
{% block map_options_js %}
var mapOptions = {
zoom: {{ zoom }},
@@ -39,20 +32,30 @@
var map = new google.maps.Map(mapElem, mapOptions);
{% block extra_js %}
- var marker = new google.maps.Marker({
- position: latlng,
- map: map,
- title: "{{ map.address }}"
- });
+ {% if markers %}
+ {% for marker in markers %}
+ var marker = easy_maps_add_marker(map, {{marker.json|safe}});
+ {% endfor %}
+
+ {% comment %}use the zoom level passed as argument if there is only one marker{% endcomment %}
+ {% if markers|length > 1 %}
+ // display all the markers
+ // http://blog.shamess.info/2009/09/29/zoom-to-fit-all-markers-on-google-maps-api-v3/
+ map.fitBounds(g_lat_long_bound);
+ {% else %}
+ easy_maps_add_listener("{{ id }}", marker);
+ {% endif %}
+ {% endif %}
+
{% endblock %}
}
{% block map_loading_js %}
// initialize the map after page loading
- google.maps.event.addDomListener(window, 'load', initialize_map_{{ map.pk }});
+ google.maps.event.addDomListener(window, 'load', initialize_map_{{ id_safe }});
{% endblock %}
</script>
{% endblock %}
{% endwith %}
-{% endwith %}
+{% endwith %}
View
30 easy_maps/templatetags/easy_maps_tags.py
@@ -1,14 +1,24 @@
#coding: utf-8
from django import template
from django.template.loader import render_to_string
-from easy_maps.models import Address
+from ..models import Address
+from .. import settings
+
register = template.Library()
@register.tag
def easy_map(parser, token):
"""
The syntax:
+
{% easy_map <address> [<width> <height>] [<zoom>] [using <template_name>] %}
+
+ or
+
+ {% easy_map <addresses> [<width> <height>] [<zoom>] [using <template_name>] %}
+
+ where in the second case you pass a queryset containing the addresses to be
+ visualized.
"""
width, height, zoom, template_name = None, None, None, None
params = token.split_contents()
@@ -42,16 +52,26 @@ def __init__(self, address, width, height, zoom, template_name):
def render(self, context):
try:
- address = self.address.resolve(context)
+ address = self.address.resolve(context) or ''
template_name = self.template_name.resolve(context)
- map, _ = Address.objects.get_or_create(address=address or '')
+ if isinstance(address, basestring):
+ # if not exists the searched address then created an unsaved instance
+ try:
+ address = Address.objects.get(address=address)
+ except:
+ address = Address(address=address)
+
+ address = [address,]
+
context.update({
- 'map': map,
+ 'markers': address,
'width': self.width,
'height': self.height,
+ # FIXME: if the list is empty?
+ 'latitude': address[0].latitude,
+ 'longitude': address[0].longitude,
'zoom': self.zoom,
- 'template_name': template_name
})
return render_to_string(template_name, context_instance=context)
except template.VariableDoesNotExist:
View
21 easy_maps/widgets.py
@@ -2,8 +2,25 @@
from django.template import Template, Context
class AddressWithMapWidget(TextInput):
+ class Media:
+ js = (
+ 'https://maps.google.com/maps/api/js?sensor=false',
+ 'js/easy_maps.js',
+ )
def render(self, name, value, attrs=None):
+ # retrieve the field's id otherwise it's not possible
+ # to use correctly the JS
+ _id = attrs["id"]
+
+ # we assume two conditions on 'id'
+ assert _id.find('id_') == 0
+
+ find_id = _id.find('address')
+ assert find_id > 0
+
+ _id = _id[:find_id]
default_html = super(AddressWithMapWidget, self).render(name, value, attrs)
- map_template = Template("{% load easy_maps_tags %}{% easy_map address 700 200 16 %}")
- context = Context({'address': value})
+ map_template = Template("""<button type='button' onclick='easy_maps_bind_button("{{ id }}")'>update</button>{% load easy_maps_tags %}{% easy_map address 700 200 16 %}""")
+
+ context = Context({'id': _id, 'id_safe': _id.replace('-', '_'), 'address': value})
return default_html + map_template.render(context)
View
2  easy_maps_tests/settings.py
@@ -28,3 +28,5 @@
# 'devserver',
'south',
)
+
+EASY_MAPS_CENTER = (45.0677201, 7.6825531)
View
26 easy_maps_tests/test_app/models.py
@@ -1 +1,27 @@
#hello, testrunner!
+from django.db import models
+from django.contrib import admin
+
+from easy_maps.models import Address
+from easy_maps.admin import AddressInlineAdmin
+
+class Brand(models.Model):
+ name = models.CharField(max_length=100)
+
+ def count(self):
+ return self.shop_set.count()
+
+class Shop(Address):
+ brand = models.ForeignKey(Brand)
+
+class ShopInlineAdmin(AddressInlineAdmin):
+ model = Shop
+
+class BrandAdmin(admin.ModelAdmin):
+ list_display = ['name', 'count',]
+ model = Brand
+ inlines = [
+ ShopInlineAdmin,
+ ]
+
+admin.site.register(Brand, BrandAdmin)
Something went wrong with that request. Please try again.