Leaflet.SimpleGraticule
Leaflet.SimpleGraticule copied to clipboard
Possible bug with onRemove
I experienced strange buggy behavior when calling removeLayer on a SimpleGraticule layer, then inspected it and found something odd. I believe this.map was typed instead of this._map.
onRemove: function(map) {
map.off('viewreset '+ this.options.redraw, this.map);
this.eachLayer(this.removeLayer, this);
},
Notice that it's calling map.off, with the input this.map. I don't believe this.map exists, just this._map.
Would something like this make more sense?
onRemove: function(map) {
var graticule = this;
this._map.off('viewreset ' + this.options.redraw, graticule.redraw, graticule);
this.eachLayer(this.removeLayer, this);
},
I think you're right that there's a problem, but I don't think the suggestion you provided makes sense. I don't understand what the map.off line is trying to do. The context has to be the same context used for map.on (http://leafletjs.com/reference-1.0.0.html#map-event)
Try this and see if it resolves your issue. If it works, let's get it merged. Notice the only change is to replace this.map with map which should be the correct context.
onRemove: function(map) {
map.off('viewreset ' + this.options.redraw, map);
this.eachLayer(this.removeLayer, this);
},
Also, based on the contract: http://leafletjs.com/reference-1.0.0.html#map-event
onAdd and onRemove should return this. Note to self to do this as well.
[edited] I spoke too soon. Your solution doesn't seem to work for me. It works if there was no zoom applied, but if there was a zoom applied, it doesn't.
That seems to also work (both solutions do), at least in my one use case.
This section of their docs makes it seem to me like the map.off should basically be on the same things as the map.on.
Also, in their docs for removeEventListener it reads,
"Removes a previously added listener function. If no function is specified, it will remove all the listeners of that particular event from the object. Note that if you passed a custom context to addEventListener, you must pass the same context to removeEventListener in order to remove the listener."
From them:
onAdd: function (map) {
this._map = map;
// create a DOM element and put it into one of the map panes
this._el = L.DomUtil.create('div', 'my-custom-layer leaflet-zoom-hide');
map.getPanes().overlayPane.appendChild(this._el);
// add a viewreset event listener for updating layer's position, do the latter
map.on('viewreset', this._reset, this);
this._reset();
},
onRemove: function (map) {
// remove layer's DOM elements and listeners
map.getPanes().overlayPane.removeChild(this._el);
map.off('viewreset', this._reset, this);
},
Looking closer at the docs, I think I recommend something similar to the following:
onAdd: function(map) {
this._map = map; // Just set inner var
this.redraw(); // Redraw
map.on('viewreset ' + this.options.redraw, this.redraw, this); //On the event 'viewreset x', run the redraw function on this.
this.eachLayer(map.addLayer, map); //For each of this layers, add them to the map
},
onRemove: function(map) {
map.off('viewreset ' + this.options.redraw, this.redraw, this); //On the event 'viewreset x', stop the event to redraw this.
this.eachLayer(map.removeLayer, map);
},