Merge pull request #348 from kneemer/bind-unbind-popup-fixes

Preserve popup options when applying to new features
Esri · Sep 30, 2014 · 1eedc5e · 1eedc5e
2 parents f8cc8ba + 15a37b3
commit 1eedc5e
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 3 deletions.
diff --git a/spec/Layers/FeatureLayer/FeatureLayerSpec.js b/spec/Layers/FeatureLayer/FeatureLayerSpec.js
@@ -39,6 +39,18 @@ describe('L.esri.Layers.FeatureLayer', function () {
     }
   }];
 
+  var multiPolygon = [({
+      type : 'Feature',
+      id: 1,
+      geometry: {
+        type: 'MultiPolygon',
+        coordinates: [[[[-95, 43], [-95, 50], [-90, 50], [-91, 42], [-95, 43]]], [[[-89, 42], [-89, 50], [-80, 50], [-80, 42]]]]
+      },
+      properties: {
+        time: new Date('Febuary 1 2014').valueOf()
+      }
+  })];
+
   beforeEach(function(){
     layer = L.esri.featureLayer('http://services.arcgis.com/mock/arcgis/rest/services/MockService/MockFeatureServer/0', {
       timeField: 'time',
@@ -183,6 +195,28 @@ describe('L.esri.Layers.FeatureLayer', function () {
     expect(layer.getFeature(3)._popup.getContent()).to.equal('ID: 3');
   });
 
+  it('should preserve popup options when binding popup to new or existing features', function(){
+    layer.bindPopup(function(feature){
+      return 'ID: ' + feature.id;
+    }, {minWidth: 500});
+
+    layer.createLayers([{
+      type: 'Feature',
+      id: 3,
+      geometry: {
+        type: 'Point',
+        coordinates: [-123, 46]
+      },
+      properties: {
+        time: new Date('Febuary 24 2014').valueOf()
+      }
+    }]);
+
+    expect(layer.getFeature(1)._popup.options.minWidth).to.equal(500);
+    expect(layer.getFeature(2)._popup.options.minWidth).to.equal(500);
+    expect(layer.getFeature(3)._popup.options.minWidth).to.equal(500);
+  });
+
   it('should unbind popups on features', function(){
     layer.bindPopup(function(feature){
       return 'ID: ' + feature.id;
@@ -192,6 +226,18 @@ describe('L.esri.Layers.FeatureLayer', function () {
     expect(layer.getFeature(2)._popup).to.equal(null);
   });
 
+  it('should unbind popups on multi polygon features', function(){
+    layer = L.esri.featureLayer('http://services.arcgis.com/mock/arcgis/rest/services/MockService/MockFeatureServer/0', { timeField: 'time' }).addTo(map);
+
+    layer.createLayers(multiPolygon);
+    layer.bindPopup(function(feature){
+      return 'ID: ' + feature.id;
+    });
+    layer.unbindPopup();
+    expect(layer.getFeature(1).getLayers()[0]._popup).to.equal(null);
+    expect(layer.getFeature(1).getLayers()[1]._popup).to.equal(null);
+  });
+
   it('should iterate over each feature', function(){
     var spy = sinon.spy();
     layer.eachFeature(spy);
@@ -238,4 +284,4 @@ describe('L.esri.Layers.FeatureLayer', function () {
     expect(spy.getCall(0).args[0].foo).to.equal('bar');
     expect(spy.getCall(0).args[0].type).to.equal('click');
   });
-});
+});
diff --git a/src/Layers/FeatureLayer/FeatureLayer.js b/src/Layers/FeatureLayer/FeatureLayer.js
@@ -84,7 +84,7 @@ L.esri.Layers.FeatureLayer = L.esri.Layers.FeatureManager.extend({
 
         // bind a popup if we have one
         if(this._popup && newLayer.bindPopup){
-          newLayer.bindPopup(this._popup(newLayer.feature, newLayer));
+          newLayer.bindPopup(this._popup(newLayer.feature, newLayer), this._popupOptions);
         }
 
         if(this.options.onEachFeature){
@@ -177,6 +177,7 @@ L.esri.Layers.FeatureLayer = L.esri.Layers.FeatureManager.extend({
 
   bindPopup: function (fn, options) {
     this._popup = fn;
+    this._popupOptions = options;
     for (var i in this._layers) {
       var layer = this._layers[i];
       var popupContent = this._popup(layer.feature, layer);
@@ -188,7 +189,16 @@ L.esri.Layers.FeatureLayer = L.esri.Layers.FeatureManager.extend({
   unbindPopup: function () {
     this._popup =  false;
     for (var i in this._layers) {
-      this._layers[i].unbindPopup();
+      var layer = this._layers[i];
+      if (layer.unbindPopup) {
+        layer.unbindPopup();
+      } else if (layer.getLayers) {
+        var groupLayers = layer.getLayers();
+        for (var j in groupLayers) {
+          var gLayer = groupLayers[j];
+          gLayer.unbindPopup();
+        }
+      }
     }
     return this;
   },