Fix for min and max zoom handling

[rowanwins]

Hi guys,

I've had a crack at fixing issue #643 , I'm not sure its the most optimal way and happy to take feedback but it seems to be working ok on the test case provided.

Cheers
Rowan

[patrickarlt]

@rowanwins sorry it has taken me so long to look at this.

Taking a look at the code I the the problem is in this line https://github.com/Esri/esri-leaflet/blob/master/src/Layers/FeatureLayer/FeatureLayer.js#L164.

Basically this works around a case where FeatureGrid fires a cellLeave for a cell that is on the departing zoom level so we don't remove the features from it. Let me mess around locally to see if I can fix this.

[patrickarlt]

@rowanwins ok I have some more insight on this.

Right now all the minZoom and maxZoom options do is control if cell events get fired. The alternative to this behavior is that you keep listening to events on the map even after the layer has been removed. I don't really like this for 2 reasons.

1. There is probably a risk of memory leaks from continuing to listen to map events even when the layer is removed.
2. This starts to break down if I want to add the layer to another map (more then 1 map per page)

Given these complications I think that FeatureGrid is actually just fine and it is probably FeatureManager or FeatureLayer that we need to worry about.

Lets just assume FeatureManager because fixing a bug in FeatureManager will also fix it in the heatmap and cluster layers. So here is how I would fix this.

1. When feature manager gets added to the map have it start listening to zoomend
2. If a zoomend event ends up outside the minZoom/maxZoom range we need to remove all the active features that are rendered not the layer itself.
3. If a zoomend event ends up inside the minZoom/maxZoom rand we need to add all of the features that are inside the current bounds.

Removing features is easy I think you can just call this.removeLayers(this._currentSnapshot) and then set this._currentSnapshot to an empty array. This should remove every feature that is currently active.

Adding the features back is a little harder. For every cell in this._activeCells you need to do the following:

1. Get the cache key for the coords of the cell ` var key = this._cacheKey(coords);
2. If this._cache[key] exists it will be an array of feature IDs.
3. Call this.addLayers(this._cache[key]) to instruct the feature layer to add the layers back.

This approach is more complicated but it avoids FeatureGrid which i think is working as intended and uses FeatureManager which is used to manage the actual representations of the features on the map. It also avoids what might be some sticky scenarios.

[patrickarlt]

@rowanwins if you want to attempt the above by all means do so. Otherwise I can get around to tacking this soon.

[rowanwins]

Gday @patrickarlt

It certainly helps knowing the internals of how FeatureLayer vs FeatureManager vs FeatureGrid etc all interact, I was just kind of guessing a bit! I'm happy to have a play sometime over the next few days to see if I can implement your proposed solution, if I dont make any progress I'll let you know. Thanks for the pointers.

Cheers
Rowan

[patrickarlt]

@rowanwins looks like @jgravois beat you to it with #695. I'm going to close this in favor of his approach.