refactored js to function as a jquery plugin #10
Conversation
| <%= javascript_tag "$('#blacklight-map').blacklight_leaflet_map({geojson_docs: #{serialize_geojson}});" %> |
There was a problem hiding this comment.
Can we make this just $('#blacklight-map').blacklight_leaflet_map(#{serialize_geojson)? I guess it means adding another argument to the function, but I think it's probably good idea
There was a problem hiding this comment.
It can be, the reason I made it that way was so that you could pass options to it in two ways, data-attributes or through "$('#blacklight-map').blacklight_leaflet_map({options:{option1: 'mapstuff'},geojson_docs: #{serialize_geojson}}). This is a typical pattern with leaflet and I was trying to maintain some of that pattern.
There was a problem hiding this comment.
I like passing options, but `blacklight_leaflet_map(geojson_docs, {option1: 'mapstuff'}) would work just as well, wouldn't it?
There was a problem hiding this comment.
Yeah, that makes sense. Will change to that.
There was a problem hiding this comment.
Thanks. It has the added benefit that it's clear you MUST have a set of geojson docs, and that options are... optional.
There was a problem hiding this comment.
ok, works with optional options now $('#blacklight-map').blacklight_leaflet_map(#{serialize_geojson}, {goodOption: 'foo'})
requires geojson_docs, and allows for additional options
|
|
||
| // Move the map so that it centers the clicked cluster TODO account for various size screens | ||
| function offsetMap(e){ | ||
| var mapWidth = $('#blacklight-map').width(); |
There was a problem hiding this comment.
Now that it's a proper plugin, I don't think we need to hard-code #blacklight-map here. I can't remember, off-hand, if it is this here, or something else..\
refactored js to function as a jquery plugin
Resolves issue #2 by using jquery plugin architecture. Eliminates global variables.