FeatureGroup: sublayers only listens for events that the FeatureGroup is listening for

[luiscamachopt]

Hi
Currently, UI and vector layers allow for click events to reach the map (if no listeners are added). L.GeoJSON, L.MultiPolyline and L.MultiPolygon do not - their sublayers grab all events even if they are not listening for any events themselves.

This commit makes sublayers to only listen for events that the featureGroup is listening for.

Changed L.GeoJSON and MultiPoly so that the subclasses of FeatureGroup call the parent constructor instead of setting _layers directly (also because L.FeatureGroup.initialize now does more than just setting that)

This PR also acomplishes the target of #1107

[mourner]

Thanks for the pull, I'll think about this. Seems a bit complicated for a rare use case, perhaps we could simplify the approach somehow...

[luiscamachopt]

Hi

With regards to simplifying it, we could add a new method ("eventTypes()") to L.Mixin.Events that returned a space-seperated string with the event types we're listening for and use that in L.FeatureGroup.addLayer and L.FeatureGroup.removeLayer.

Then we wouldn't need the initialize method nor the changes to GeoJSON.js and MultiPoly.js (even though I think we should keep these last two) and the L.FeatureGroup.addEventListener method would be slightly simpler.

With regards to it being a rare use case, it's not only a case of events reaching the map, but also inconsistency between diferent types of vector layers. If you have overlapping vector layers in which some have click event listeners and other do not, the behaviour will vary depending on the layers being GeoJSON, MultiPolygon, MultiPolyLine or any other vector layer. So you could argue that it is a bug...

Also, from #1107 it seems there are performance benefits from not having listeners on all sub layers when they are not necessary.

[danzel]

An alternative would be to allow overriding the events string when creating a FeatureGroup.
Code should be much smaller and be able to get the same performance improvement. Requires a bit more thought on behalf of the dev to get the perf increase however.

[luiscamachopt]

@danzel it's true it would't require overriding of addEventListener in LayerGroup.

I initially went that route but then had to add the option to L.GeoJSON, L.MultiPolyline, L.MultiPolygon and when calling L.GeoJSON.geometryToLayer (and maybe other methods).
It was an option that's wasn't quite clear why it should be there (it wasn't necessary in other "similar" classes) and seemed weird in L.MultiPolyline, L.MultiPolygon as these already receive options that are applied to each of the sub geometries, but this would be an option that wouldn't propagate...

Because of these points I thought it made more sense to just override addEventListener in LayerGroup. Keeps the API cleaner

[mourner]

Closing in favor of a more universal approach #2311