search

yourcelf / olwidget

Javascript library to replace textareas that contain WKT data with editable OpenLayers maps, and a django app using it for django forms and admin.
Other
91 stars 43 forks source link

Another small enhancement: keep a scope var pointing to olwidget #31

Closed elpaso closed 14 years ago

elpaso commented 14 years ago

I did it in my overridden template, but maybe could be added to trunk:

change in templates from new olwidget.InfoMap("{{ id }}", {{ info_array|safe }}, {{ map_opts|safe }}); to var olmap = new olwidget.InfoMap("{{ id }}", {{ info_array|safe }}, {{ map_opts|safe }});

this could be useful in case you want to manipulate olmap in some ways.

yourcelf commented 14 years ago

Hi: I recognize the utility of including a variable name. But I'm a little concerned about polluting the javascript namespace by throwing extra names in that people might not expect. Do you think there is a way that this could be done which would not result in unexpected name clobbering in the javascript?

elpaso commented 14 years ago

Let's make it an option (disabled by default) with the possibility to override the variable name.

yourcelf commented 14 years ago

I've decided that I don't want to add a default scope variable. The main reason: if you are customizing the javascript implementation by using a scope variable, it seems to me that you ought to take full control over the javascript implementation by customizing the template. In terms of code organization, it seems to make sense to encapsulate the template that controls map presentation in one place, rather than splitting the implementation between olwidget's default template and your own template.

One of the important design goals of olwidget is to ensure that multiple maps per page are supported easily. If a variable name is set for each map on a page, these names will collide. Of course, one could add an option to change the variable name or to prevent it from appearing, but this feels to me overly complex, and mixing what seems like it should be a template-based detail with the view code.

So I'm gonna close this as "wontfix". I hope this is cool with you; please feel free to push back if you disagree.

best, Charlie

skyl commented 14 years ago

I think this is fine to close; although var map_{{id}} would certainly be unique. I think that the multitude of solutions for handling this might be best left to the developer. template is a handled kwarg to the different widgets.