Fix removeDrawToolbar(clearFeatures = TRUE) #61

[RCura]

The clearGroup() seems to be a custom function, not in base leaflet, and doesn't seems to work as expected. Replacing it with raw/base leaflet

[bhaskarvk]

It is a custom function of the LayerManager. LayerManager is kinda important to keep track of the layers, so I would rather figure out why the clearLayers function isn't working. I'll spend some time debugging that first before falling back on the base leaflet calls.

In fact I want to make sure that all layers added via leaflet.extras are managed via LayerManager and similarly all controls are added via the ControlManager. Thanks for finding and reporting.

[RCura]

Oups, I don't know how but it added another commit I just made for a feature enhancement proposition. No idea how to split again this PR. The idea of the second commit (singleFeature) is to propose to the user, when creating a new feature, to only keep the last drawn feature, and so, to remove the previous ones. This allows for simple spatial selections, just like a common brush would work in plots

[bhaskarvk]

No worries, I can cherry pick the commits and make changes.

[RCura]

I'm trying to debug. Seems the problem is clearGroup not working because the draw layer isn't correctly "registered" in the layerManager. For example, I created a map with one base group ("test"), made of a providerTiles, and adding DrawToolbar

thismap.layerManager._byGroup
    > Object { test: Object, editableFeatureGroup: Object }
thismap.layerManager._byGroup["test"]
    > Object { 27: Object }
thismap.layerManager._byGroup["editableFeatureGroup"]
    > Object {  }

The editableFeatureGroup is there, but empty.

[bhaskarvk]

That shouldn't matter, I am storing the group name if specified in map._editableFeatureGroupName, so if you specify a targetGroup as part of addDrawToolbar, that's the group name it will use. editableFeatureGroup is used only if you don't specify a targetGroup or a targetLayerId, coz in that case I need to create a new FeatureGroup to store the drawn features.

[RCura]

Yeah, saw that, but the group, whatever his name, isn't correctly added to the groups referenced in _byGroup, and thus, it's layers are not accessible from it.

[RCura]

$.each(a.layerManager._byGroup["test"], function(e){e._layers})
    > Object {27: e}
$.each(a.layerManager._byGroup["editableFeatureGroup"], function(e){e._layers})
    > Object {}
a.layerManager._groupContainers["editableFeatureGroup"]._layers
    > Object {53: e, 61: e}

(a being the map/this object here)

So, the drawn layers are there (53 & 61), but not correctly referenced in _byGroup, and thus, they can't be removed in your clearGroup function as defined in layer-manager.js :

clearGroup(group) {
    // Find all layers in _byGroup[group]
    let groupTable = this._byGroup[group];
    if (!groupTable) {
      return false;
    }

    // Remove all layers. Make copy of keys to avoid mutating the collection
    // behind the iterator you're accessing.
    let stamps = [];
    $.each(groupTable, (k, v) => {
      stamps.push(k);
    });
    $.each(stamps, (i, stamp) => {
      this._removeLayer(stamp);
    });
}

Those layers aren't in _byGroup, so, not in groupTable either, and then not in stamps. So, _removeLayer on those does nothing.

[bhaskarvk]

I see. I need to figure out what needs to be done to fix this. Not sure if this something I need to fix in leaflet.extras or in core leaflet.