Leaflet / Leaflet.markercluster

Marker Clustering plugin for Leaflet
MIT License
3.93k stars 995 forks source link

Add markercluster Identification Property #905

Open codeofsumit opened 6 years ago

codeofsumit commented 6 years ago

Hi all! I'm the author of leaflet drawing plugin leaflet.pm and I'm currently trying to make our two plugins work together (https://github.com/codeofsumit/leaflet.pm/issues/316).

If developers are using both our plugins, spiderlegs are editable and drag & edit features affect the whole marker group instead of single markers. Also the cluster-marker becomes editable. image

As far as I see it, leaflet.pm should basically only consider "core" layers/markers and ignore helper markers of any plugins. I consider the cluster-marker and the spiderlegs to be helpers of the markercluster plugin.

My issue currently is to identify layers from markercluster. I'm checking against the _group and _markerCluster property. I could not find any way to identify a Spiderleg layer, so I additionally check markers for _spiderLegs and remove leaflet.pm classes after their initialization. This is far from ideal and sprinkles markercluster specific if-else statements across the library code.

Suggestion: In leaflet.PM I've added pmTempLayer: true to any helper-layer that is created by the library for others to identify and ignore, if necessary. Would it make sense to introduce a similar property like markerclusterHelper: true to the cluster-marker and spiderlegs to easily identify them? With that, I could check for that property and ignore these layers.

Alternative If you would like to add leaflet.PM compatibility to markercluster from your side, you could also add the option pmIgnore: true to all cluster-marker and the spiderlegs and they're ignored automatically by leaflet.pm. I'd be happy to create a PR for this if this is an option for you.

What do you think?

danzel commented 6 years ago

Neat plugin. I take it that leaflet.PM automatically make all layers on the map editable?

It would be nice not to put plugin specific mods in either of our plugins, but I think it should be possible to create a L.MC sub-plugin (Leaflet.MarkerCluster.PMCompat?) that set the pmIgnore property on things created by MCG?

Happy to have extension points added to L.MC to support this if needed.

codeofsumit commented 6 years ago

Hey @danzel - nice idea. I would try and write a sub-plugin for MarkerCluster. Will report back!

codeofsumit commented 6 years ago

@danzel I was able to make editing of my plugin work together with markercluster: markercluster

(spiderlegs are getting ignored and clustermarker updates correctly 👍 )

But I still have some problems with the removal of markers. When I fan out a cluster and delete a marker from the cluster it seems as if markercluster events are getting lost. The icon is not updated, I can't collapse the cluster again, mouse events do not work anymore. No error is thrown though. Any idea what could cause this?

Here are the sub-plugin changes I have so far:

L.MarkerClusterGroup.include({
    originalInit: L.MarkerClusterGroup.prototype.initialize,
    initialize(options) {
        // ignore spiderlegs
        this.options.spiderLegPolylineOptions.pmIgnore = true;

        // ignore this, not sure what it is though 🤔
        this.options.polygonOptions.pmIgnore = true;

        this.originalInit(options);
    },

    // Fire regular layeradd event when adding a layer via markercluster
    _originalAddLayer: L.MarkerClusterGroup.prototype.addLayer,
    addLayer(layer) {
        this._originalAddLayer(layer);

        return this.fire('layeradd', { layer });
    },
});

// add pmIgnore to the cluster marker
L.MarkerCluster.include({
    originalInit: L.MarkerCluster.prototype.initialize,
    initialize(...args) {
        this.options.pmIgnore = true;

        this.originalInit(...args);
    },
});

Any hints are appreciated 👍

danzel commented 6 years ago

How are you deleting the marker? You probably need to remove it from the MarkerClusterGroup layer, but you are probably just removing it from the map?

codeofsumit commented 6 years ago

Yup. They’re removed like normal. I tried to remove them from the markerCluster after they’re removed from the map which did not work. Do you think I have to delete them from mc first and then from the map? Will try this today.

ghybs commented 6 years ago

Hi @codeofsumit,

Indeed, you should remove the Marker from the MCG instead of from the map directly.

Or use Leaflet.FeatureGroup.SubGroup or Leaflet.MarkerCluster.LayerSupport.