Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layer refactoring #2266

Merged
merged 14 commits into from Dec 6, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion CHANGELOG.md
Expand Up @@ -7,9 +7,26 @@ Leaflet Changelog

An in-progress version being developed on the `master` branch. Includes `stable` branch fixes.

This version contains a lot of beneficial but potentially breaking changes (especially if you're a plugin author), so please read through the changes carefully before upgrading.

### Layers refactoring

All Leaflet layers (including markers, popups, tile and vector layers) have been refactored to have a common parent, `Layer` class, that shares the basic logic of adding and removing. The leads to the following changes (documented in PR [#2266](https://github.com/Leaflet/Leaflet/pull/2266)):

* Added `Layer` class which all layers added to a map should inherit from.
* Added `add` and `remove` events to all layers.
* Added `pane` option to all layers that can be changed (e.g. you can set `pane: 'overlayPane'` to a tile layer).
* Added `shadowPane` option to markers as well.
* Added `getEvents` method to all layers that returns an `{event: listener, ...}` hash; layers now manage its listeners automatically without having to do this in `onAdd`/`onRemove`.
* Improved performance of adding/removing layers with layers control present (instead of listening to any layer add/remove, the control only listens to layers added in configuration).
* Fixed `FeatureGroup` `getBounds` to work correctly when containing circle markers.
* Removed `Map` `tilelayersload` event.
* Removed `Popup` `open` and `close` events in favor of `add` and `remove` for consistency.
* Moved all layer-related logic in `Map.js` to `Layer.js`.

### TileLayer & Projections refactoring

TileLayer code and everything projections-related has undergone a major refactoring, documented in [#2247](https://github.com/Leaflet/Leaflet/pull/2247). It includes the following changes (in addition to much cleaner and simpler code):
TileLayer code and everything projections-related has undergone a major refactoring, documented in PR [#2247](https://github.com/Leaflet/Leaflet/pull/2247). It includes the following changes (in addition to much cleaner and simpler code):

#### TileLayer-related changes

Expand Down
4 changes: 3 additions & 1 deletion build/deps.js
Expand Up @@ -17,7 +17,9 @@ var deps = {
'geo/crs/CRS.Simple.js',
'geo/crs/CRS.EPSG3857.js',
'geo/crs/CRS.EPSG4326.js',
'map/Map.js'],
'map/Map.js',
'layer/Layer.js'
],
desc: 'The core of the library, including OOP, events, DOM facilities, basic units, projections (EPSG:3857 and EPSG:4326) and the base Map class.'
},

Expand Down
36 changes: 22 additions & 14 deletions spec/suites/map/MapSpec.js
Expand Up @@ -199,32 +199,40 @@ describe("Map", function () {
});
});

function layerSpy() {
var layer = new L.Layer();
layer.onAdd = sinon.spy();
layer.onRemove = sinon.spy();
return layer;
}

describe("#addLayer", function () {

it("calls layer.onAdd immediately if the map is ready", function () {
var layer = { onAdd: sinon.spy() };
var layer = layerSpy();
map.setView([0, 0], 0);
map.addLayer(layer);
expect(layer.onAdd.called).to.be.ok();
});

it("calls layer.onAdd when the map becomes ready", function () {
var layer = { onAdd: sinon.spy() };
var layer = layerSpy();
map.addLayer(layer);
expect(layer.onAdd.called).not.to.be.ok();
map.setView([0, 0], 0);
expect(layer.onAdd.called).to.be.ok();
});

it("does not call layer.onAdd if the layer is removed before the map becomes ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.addLayer(layer);
map.removeLayer(layer);
map.setView([0, 0], 0);
expect(layer.onAdd.called).not.to.be.ok();
});

it("fires a layeradd event immediately if the map is ready", function () {
var layer = { onAdd: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layeradd', spy);
map.setView([0, 0], 0);
Expand All @@ -233,7 +241,7 @@ describe("Map", function () {
});

it("fires a layeradd event when the map becomes ready", function () {
var layer = { onAdd: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layeradd', spy);
map.addLayer(layer);
Expand All @@ -243,7 +251,7 @@ describe("Map", function () {
});

it("does not fire a layeradd event if the layer is removed before the map becomes ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layeradd', spy);
map.addLayer(layer);
Expand All @@ -253,7 +261,7 @@ describe("Map", function () {
});

it("adds the layer before firing layeradd", function (done) {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.on('layeradd', function () {
expect(map.hasLayer(layer)).to.be.ok();
done();
Expand Down Expand Up @@ -299,29 +307,29 @@ describe("Map", function () {

describe("#removeLayer", function () {
it("calls layer.onRemove if the map is ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.setView([0, 0], 0);
map.addLayer(layer);
map.removeLayer(layer);
expect(layer.onRemove.called).to.be.ok();
});

it("does not call layer.onRemove if the layer was not added", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.setView([0, 0], 0);
map.removeLayer(layer);
expect(layer.onRemove.called).not.to.be.ok();
});

it("does not call layer.onRemove if the map is not ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.addLayer(layer);
map.removeLayer(layer);
expect(layer.onRemove.called).not.to.be.ok();
});

it("fires a layerremove event if the map is ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layerremove', spy);
map.setView([0, 0], 0);
Expand All @@ -331,7 +339,7 @@ describe("Map", function () {
});

it("does not fire a layerremove if the layer was not added", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layerremove', spy);
map.setView([0, 0], 0);
Expand All @@ -340,7 +348,7 @@ describe("Map", function () {
});

it("does not fire a layerremove if the map is not ready", function () {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() },
var layer = layerSpy(),
spy = sinon.spy();
map.on('layerremove', spy);
map.addLayer(layer);
Expand All @@ -349,7 +357,7 @@ describe("Map", function () {
});

it("removes the layer before firing layerremove", function (done) {
var layer = { onAdd: sinon.spy(), onRemove: sinon.spy() };
var layer = layerSpy();
map.on('layerremove', function () {
expect(map.hasLayer(layer)).not.to.be.ok();
done();
Expand Down
24 changes: 9 additions & 15 deletions src/control/Control.Layers.js
Expand Up @@ -25,19 +25,13 @@ L.Control.Layers = L.Control.extend({
}
},

onAdd: function (map) {
onAdd: function () {
this._initLayout();
this._update();

map.on('layeradd layerremove', this._onLayerChange, this);

return this._container;
},

onRemove: function (map) {
map.off('layeradd layerremove', this._onLayerChange, this);
},

addBaseLayer: function (layer, name) {
this._addLayer(layer, name);
return this._update();
Expand All @@ -49,6 +43,8 @@ L.Control.Layers = L.Control.extend({
},

removeLayer: function (layer) {
layer.off('add remove', this._onLayerChange, this);

delete this._layers[L.stamp(layer)];
return this._update();
},
Expand Down Expand Up @@ -108,6 +104,8 @@ L.Control.Layers = L.Control.extend({
},

_addLayer: function (layer, name, overlay) {
layer.on('add remove', this._onLayerChange, this);

var id = L.stamp(layer);

this._layers[id] = {
Expand Down Expand Up @@ -143,20 +141,16 @@ L.Control.Layers = L.Control.extend({
},

_onLayerChange: function (e) {
var obj = this._layers[L.stamp(e.layer)];

if (!obj) { return; }

if (!this._handlingClick) {
this._update();
}

var type = obj.overlay ?
(e.type === 'layeradd' ? 'overlayadd' : 'overlayremove') :
(e.type === 'layeradd' ? 'baselayerchange' : null);
var type = e.target.overlay ?
(e.type === 'add' ? 'overlayadd' : 'overlayremove') :
(e.type === 'add' ? 'baselayerchange' : null);

if (type) {
this._map.fire(type, obj);
this._map.fire(type, e.target);
}
},

Expand Down
6 changes: 2 additions & 4 deletions src/control/Control.Scale.js
Expand Up @@ -7,13 +7,11 @@ L.Control.Scale = L.Control.extend({
position: 'bottomleft',
maxWidth: 100,
metric: true,
imperial: true,
updateWhenIdle: false
imperial: true
// updateWhenIdle: false
},

onAdd: function (map) {
this._map = map;

var className = 'leaflet-control-scale',
container = L.DomUtil.create('div', className),
options = this.options;
Expand Down
2 changes: 0 additions & 2 deletions src/control/Control.Zoom.js
Expand Up @@ -16,8 +16,6 @@ L.Control.Zoom = L.Control.extend({
container = L.DomUtil.create('div', zoomName + ' leaflet-bar'),
options = this.options;

this._map = map;

this._zoomInButton = this._createButton(options.zoomInText, options.zoomInTitle,
zoomName + '-in', container, this._zoomIn, this);
this._zoomOutButton = this._createButton(options.zoomOutText, options.zoomOutTitle,
Expand Down
7 changes: 2 additions & 5 deletions src/layer/FeatureGroup.js
Expand Up @@ -4,7 +4,6 @@
*/

L.FeatureGroup = L.LayerGroup.extend({
includes: L.Mixin.Events,

statics: {
EVENTS: 'click dblclick mouseover mouseout mousemove contextmenu popupopen popupclose'
Expand All @@ -15,9 +14,7 @@ L.FeatureGroup = L.LayerGroup.extend({
return this;
}

if ('on' in layer) {
layer.on(L.FeatureGroup.EVENTS, this._propagateEvent, this);
}
layer.on(L.FeatureGroup.EVENTS, this._propagateEvent, this);

L.LayerGroup.prototype.addLayer.call(this, layer);

Expand Down Expand Up @@ -78,7 +75,7 @@ L.FeatureGroup = L.LayerGroup.extend({
var bounds = new L.LatLngBounds();

this.eachLayer(function (layer) {
bounds.extend(layer instanceof L.Marker ? layer.getLatLng() : layer.getBounds());
bounds.extend((layer.getBounds || layer.getLatLng)());
});

return bounds;
Expand Down
36 changes: 9 additions & 27 deletions src/layer/ImageOverlay.js
Expand Up @@ -2,8 +2,7 @@
* L.ImageOverlay is used to overlay images over the map (to specific geographical bounds).
*/

L.ImageOverlay = L.Class.extend({
includes: L.Mixin.Events,
L.ImageOverlay = L.Layer.extend({

options: {
opacity: 1
Expand All @@ -16,30 +15,18 @@ L.ImageOverlay = L.Class.extend({
L.setOptions(this, options);
},

onAdd: function (map) {
this._map = map;
this._animated = this._map.options.zoomAnimation && L.Browser.any3d;

onAdd: function () {
if (!this._image) {
this._initImage();
}

this._getPane().appendChild(this._image);

map.on(this._getEvents(), this);
this.getPane().appendChild(this._image);

this._reset();
},

onRemove: function (map) {
onRemove: function () {
L.DomUtil.remove(this._image);

map.off(this._getEvents(), this);
},

addTo: function (map) {
map.addLayer(this);
return this;
},

setOpacity: function (opacity) {
Expand All @@ -51,13 +38,13 @@ L.ImageOverlay = L.Class.extend({
// TODO remove bringToFront/bringToBack duplication from TileLayer/Path
bringToFront: function () {
if (this._image) {
this._getPane().appendChild(this._image);
this.getPane().appendChild(this._image);
}
return this;
},

bringToBack: function () {
var pane = this._getPane();
var pane = this.getPane();
if (this._image) {
pane.insertBefore(this._image, pane.firstChild);
}
Expand All @@ -73,14 +60,10 @@ L.ImageOverlay = L.Class.extend({
return this.options.attribution;
},

_getPane: function () {
return this._map._panes.overlayPane;
},

_getEvents: function () {
getEvents: function () {
var events = {viewreset: this._reset};

if (this._animated) {
if (this._zoomAnimated) {
events.zoomanim = this._animateZoom;
}

Expand All @@ -89,8 +72,7 @@ L.ImageOverlay = L.Class.extend({

_initImage: function () {
this._image = L.DomUtil.create('img',
'leaflet-image-layer ' +
'leaflet-zoom-' + (this._animated ? 'animated' : 'hide'));
'leaflet-image-layer ' + (this._zoomAnimated ? 'leaflet-zoom-animated' : ''));

this._updateOpacity();

Expand Down