diff --git a/spec/suites/layer/PopupSpec.js b/spec/suites/layer/PopupSpec.js index 019ef49006a..3d980a3305f 100644 --- a/spec/suites/layer/PopupSpec.js +++ b/spec/suites/layer/PopupSpec.js @@ -499,4 +499,31 @@ describe('L.Layer#_popup', function () { }); expect(popup.isOpen()).to.be.ok(); }); + + it("does not open for empty FeatureGroup", function () { + var popup = L.popup(); + L.featureGroup([]) + .addTo(map) + .bindPopup(popup) + .openPopup(); + + expect(map.hasLayer(popup)).to.not.be.ok(); + }); + + it("uses only visible layers of FeatureGroup for popup content source", function () { + var marker1 = L.marker([1, 1]); + var marker2 = L.marker([2, 2]); + var marker3 = L.marker([3, 3]); + var popup = L.popup(); + var group = L.featureGroup([marker1, marker2, marker3]) + .bindPopup(popup) + .addTo(map); + + marker1.remove(); + marker3.remove(); + group.openPopup(); + + expect(map.hasLayer(popup)).to.be.ok(); + expect(popup._source).to.be(marker2); + }); }); diff --git a/src/layer/DivOverlay.js b/src/layer/DivOverlay.js index 7bbd37b4198..36c0b241fee 100644 --- a/src/layer/DivOverlay.js +++ b/src/layer/DivOverlay.js @@ -159,38 +159,45 @@ export var DivOverlay = Layer.extend({ return this; }, - _prepareOpen: function (parent, layer, latlng) { - if (!(layer instanceof Layer)) { - latlng = layer; - layer = parent; - } - - if (layer instanceof FeatureGroup) { - for (var id in parent._layers) { - layer = parent._layers[id]; - break; + // prepare bound overlay to open: update latlng pos / content source (for FeatureGroup) + _prepareOpen: function (latlng) { + var source = this._source; + if (!source._map) { return false; } + + if (source instanceof FeatureGroup) { + source = null; + var layers = this._source._layers; + for (var id in layers) { + if (layers[id]._map) { + source = layers[id]; + break; + } } + if (!source) { return false; } // Unable to get source layer. + + // set overlay source to this layer + this._source = source; } if (!latlng) { - if (layer.getCenter) { - latlng = layer.getCenter(); - } else if (layer.getLatLng) { - latlng = layer.getLatLng(); - } else if (layer.getBounds) { - latlng = layer.getBounds().getCenter(); + if (source.getCenter) { + latlng = source.getCenter(); + } else if (source.getLatLng) { + latlng = source.getLatLng(); + } else if (source.getBounds) { + latlng = source.getBounds().getCenter(); } else { throw new Error('Unable to get source layer LatLng.'); } } + this.setLatLng(latlng); - // set overlay source to this layer - this._source = layer; - - // update the overlay (content, layout, ect...) - this.update(); + if (this._map) { + // update the overlay (content, layout, etc...) + this.update(); + } - return latlng; + return true; }, _updateContent: function () { diff --git a/src/layer/Popup.js b/src/layer/Popup.js index f74fe15f892..99bd0ea4fb2 100644 --- a/src/layer/Popup.js +++ b/src/layer/Popup.js @@ -416,10 +416,8 @@ Layer.include({ // @method openPopup(latlng?: LatLng): this // Opens the bound popup at the specified `latlng` or at the default popup anchor if no `latlng` is passed. - openPopup: function (layer, latlng) { - if (this._popup && this._map) { - latlng = this._popup._prepareOpen(this, layer, latlng); - + openPopup: function (latlng) { + if (this._popup && this._popup._prepareOpen(latlng)) { // open the popup on the map this._map.openPopup(this._popup, latlng); } @@ -438,12 +436,12 @@ Layer.include({ // @method togglePopup(): this // Opens or closes the popup bound to this layer depending on its current state. - togglePopup: function (target) { + togglePopup: function () { if (this._popup) { if (this._popup._map) { this.closePopup(); } else { - this.openPopup(target); + this.openPopup(); } } return this; @@ -471,33 +469,25 @@ Layer.include({ }, _openPopup: function (e) { - var layer = e.layer || e.target; - - if (!this._popup) { + if (!this._popup || !this._map) { return; } - - if (!this._map) { - return; - } - // prevent map click DomEvent.stop(e); - // if this inherits from Path its a vector and we can just - // open the popup at the new location - if (layer instanceof Path) { - this.openPopup(e.layer || e.target, e.latlng); + var target = e.layer || e.target; + if (this._popup._source === target && !(target instanceof Path)) { + // treat it like a marker and figure out + // if we should toggle it open/closed + if (this._map.hasLayer(this._popup)) { + this.closePopup(); + } else { + this.openPopup(e.latlng); + } return; } - - // otherwise treat it like a marker and figure out - // if we should toggle it open/closed - if (this._map.hasLayer(this._popup) && this._popup._source === layer) { - this.closePopup(); - } else { - this.openPopup(layer, e.latlng); - } + this._popup._source = target; + this.openPopup(e.latlng); }, _movePopup: function (e) { diff --git a/src/layer/Tooltip.js b/src/layer/Tooltip.js index 2afc3b55fef..fd0320c7d54 100644 --- a/src/layer/Tooltip.js +++ b/src/layer/Tooltip.js @@ -331,10 +331,8 @@ Layer.include({ // @method openTooltip(latlng?: LatLng): this // Opens the bound tooltip at the specified `latlng` or at the default tooltip anchor if no `latlng` is passed. - openTooltip: function (layer, latlng) { - if (this._tooltip && this._map) { - latlng = this._tooltip._prepareOpen(this, layer, latlng); - + openTooltip: function (latlng) { + if (this._tooltip && this._tooltip._prepareOpen(latlng)) { // open the tooltip on the map this._map.openTooltip(this._tooltip, latlng); } @@ -353,12 +351,12 @@ Layer.include({ // @method toggleTooltip(): this // Opens or closes the tooltip bound to this layer depending on its current state. - toggleTooltip: function (target) { + toggleTooltip: function () { if (this._tooltip) { if (this._tooltip._map) { this.closeTooltip(); } else { - this.openTooltip(target); + this.openTooltip(); } } return this; @@ -386,12 +384,12 @@ Layer.include({ }, _openTooltip: function (e) { - var layer = e.layer || e.target; - if (!this._tooltip || !this._map) { return; } - this.openTooltip(layer, this._tooltip.options.sticky ? e.latlng : undefined); + this._tooltip._source = e.layer || e.target; + + this.openTooltip(this._tooltip.options.sticky ? e.latlng : undefined); }, _moveTooltip: function (e) {