From df935091fdcb163e8e04301a5225aec084f5a01a Mon Sep 17 00:00:00 2001
From: Rufus Pollock <rufus.pollock@okfn.org>
Date: Tue, 23 Oct 2012 00:19:59 +0100
Subject: [PATCH] [#244,view/map][m]: support for customizing rendering of map
 features in all ways supported by Leaflet - fixes #244.

* This automatically provides support for things like custom markers via
  pointToLayer etc

* Also a significant bugfix for a bug that surfaced when using different marker
  (like the CircleMarker) - and which took ~30m to track down. Bug was that the
  call to zoom (or, more specifically, call to getBounds) occurred before
  features were added to the map and getBounds for some objects (such as
  circles) requires map to exit (so you can do a projection!)
---
 src/view.map.js       | 62 +++++++++++++++++++++++++++++++++----------
 test/view.map.test.js | 20 ++++++++++++++
 2 files changed, 68 insertions(+), 14 deletions(-)

diff --git a/src/view.map.js b/src/view.map.js
index 8199dae2..d6de52fc 100644
--- a/src/view.map.js
+++ b/src/view.map.js
@@ -131,6 +131,39 @@ my.Map = Backbone.View.extend({
     return html;
   },
 
+  // Options to use for the [Leaflet GeoJSON layer](http://leaflet.cloudmade.com/reference.html#geojson)
+  // See also <http://leaflet.cloudmade.com/examples/geojson.html>
+  //
+  // e.g.
+  //
+  //     pointToLayer: function(feature, latLng)
+  //     onEachFeature: function(feature, layer)
+  //
+  // See defaults for examples
+  geoJsonLayerOptions: {
+    // pointToLayer function to use when creating points
+    //
+    // Default behaviour shown here is to create a marker using the
+    // popupContent set on the feature properties (created via infobox function
+    // during feature generation)
+    //
+    // NB: inside pointToLayer `this` will be set to point to this map view
+    // instance (which allows e.g. this.markers to work in this default case)
+    pointToLayer: function (feature, latlng) {
+      var marker = new L.Marker(latlng);
+      marker.bindPopup(feature.properties.popupContent);
+      // this is for cluster case
+      this.markers.addLayer(marker);
+      return marker;
+    },
+    // onEachFeature default which adds popup in
+    onEachFeature: function(feature, layer) {
+      if (feature.properties && feature.properties.popupContent) {
+        layer.bindPopup(feature.properties.popupContent);
+      }
+    }
+  },
+
   // END: Customization section
   // ----
 
@@ -195,6 +228,15 @@ my.Map = Backbone.View.extend({
         return;
       }
 
+      // this must come before zooming!
+      // if not: errors when using e.g. circle markers like
+      // "Cannot call method 'project' of undefined"
+      if (this.state.get('cluster')) {
+        this.map.addLayer(this.markers);
+      } else {
+        this.map.addLayer(this.features);
+      }
+
       if (this.state.get('autoZoom')){
         if (this.visible){
           this._zoomToFeatures();
@@ -202,11 +244,6 @@ my.Map = Backbone.View.extend({
           this._zoomPending = true;
         }
       }
-      if (this.state.get('cluster')) {
-        this.map.addLayer(this.markers);
-      } else {
-        this.map.addLayer(this.features);
-      }
     }
   },
 
@@ -323,7 +360,7 @@ my.Map = Backbone.View.extend({
         } else {
           return null;
         }
-      } else if (value && value.slice) {
+      } else if (value && _.isArray(value)) {
         // [ lon, lat ]
         return {
           "type": "Point",
@@ -412,14 +449,11 @@ my.Map = Backbone.View.extend({
 
     this.markers = new L.MarkerClusterGroup(this._clusterOptions);
 
-    this.features = new L.GeoJSON(null,{
-        pointToLayer: function (feature, latlng) {
-          var marker = new L.marker(latlng);
-          marker.bindPopup(feature.properties.popupContent);
-          self.markers.addLayer(marker);
-          return marker;
-        }
-    });
+    // rebind this (as needed in e.g. default case above)
+    this.geoJsonLayerOptions.pointToLayer =  _.bind(
+        this.geoJsonLayerOptions.pointToLayer,
+        this);
+    this.features = new L.GeoJSON(null, this.geoJsonLayerOptions);
 
     this.map.setView([0, 0], 2);
 
diff --git a/test/view.map.test.js b/test/view.map.test.js
index 3d8f038f..e429f646 100644
--- a/test/view.map.test.js
+++ b/test/view.map.test.js
@@ -207,6 +207,26 @@ test('Popup - Custom', function () {
   view.remove();
 });
 
+test('geoJsonLayerOptions', function () {
+  var dataset = GeoJSONFixture.getDataset();
+  var view = new recline.View.Map({
+    model: dataset
+  });
+  $('.fixtures').append(view.el);
+  view.geoJsonLayerOptions.point 
+  view.geoJsonLayerOptions.pointToLayer = function(feature, latlng) {
+    var marker = new L.CircleMarker(latlng, { radius: 8 } );
+    marker.bindPopup(feature.properties.popupContent);
+    return marker;
+  }
+  view.render();
+
+  // TODO: test it somehow?
+  expect(0);
+
+  view.remove();
+});
+
 test('MapMenu', function () {
   var dataset = Fixture.getDataset();
   var controls = new recline.View.MapMenu({