jawj / OverlappingMarkerSpiderfier-Leaflet

Deals with overlapping markers in the Leaflet maps API, Google Earth-style
253 stars 68 forks source link

Spiderfied marker differs from initial loaded spider marker #49

Closed magicmb closed 4 years ago

magicmb commented 4 years ago

our setup

We (our community project & me aka tech dabbler) have an L.map with various layers of L.markers loading from a dynamic array of events spanning a monthly period. Many of the listed venues hold numerous events during any given period and hence we end up with numerous markers sharing locations that are very nicely spiderfied using the OMS-Leaflet plugin (0.2.6) we run from CDN

Each marker is put into one of several marker Layers depending on day of week or whether the event has been confirmed / expires e.g. here is one that falls on a weekend:

markerWeekEnd = L.marker([lat, lon], { %options% });
markerWeekEnd.addTo(map);
weekend.addLayer(markerWeekEnd);
oms.addMarker(markerWeekEnd);

It took me a while to figure out how to stack the markers correctly so that they shows up in a certain order (1. upcoming markers, 2. other future date markers and 3. any expired markers if applicable) using zIndexOffset as the bringToFront() method does not support L.markers.

the issue

We have everything loading and stacking as required and it all works fine until we unspiderfy/spiderfy. When the markers get re-spiderfied the one that ends up on top is not the one initially displayed.

I took a look at all the OMS issues, not just OMS-Leaflet (i.e. also the OMS version for Google Maps API v3), and found a couple of closed ones that seemed to suggest a similar issue:

OMS should remember original zIndex of marker #76 z-Indices reset #117 (previous fix closes this)

Now as far as I can tell the related fix for these appears to have been applied to OverlappingMarkerSpiderfier/1.0.2/oms.js (also 1.0.3 now I imagine) but perhaps not the OverlappingMarkerSpiderfier-Leaflet/0.2.6/oms.js version.

In any case our subsequent efforts to get around the issues described above did seem to address the zIndex somewhat but not overcome our stacking issue relating to OMS spiderfy.

what we tried

Decided to tweak my marker options to retain the original zIndexOffset settings L.marker([lat, lon], {... zIndexOffset: mIndex, mDay: mDay}) and to see if resetting the zIndex using the retained values could fix my stacking issue

// for both spiderfy & unspiderfy
oms.addListener('spiderfy', function(markers) { 
   markers.forEach(function(marker) { zIndexCheck(marker); });
}
function zIndexCheck(marker){
   L.setOptions( marker, { zIndex: mIndex, zIndexOffset: mIndex });
}

Sadly even though it seem to retain the zIndex values for markers before and after the unspiderfy/spiderfy, the eventual marker that remains on top is not the one that was there before, nor the one with the highest supplied zIndex count (addListener for spiderfy).

I have re-created a stripped down version of our sites functionality on the following jsfiddle and console logged the zIndexes of the initial markers before and after the spiderfy:

Marker Title: 'London Socials #1 (1/2)' startTime:ENDED: 1st Feb zIndex:-10
Marker Title: 'London Socials #2 (14/2)' startTime:Fri 14th Feb zIndex:5054 (Active)
Marker Title: 'London Socials #3 (28/2)' startTime:Fri 28th Feb zIndex:2026

#2 is the active item initially

Spiderfy

[London Socials #1 (1/2)]  index:990
[London Socials #2 (14/2)]  index:5027
[London Socials #3 (28/2)]  index:5013

UnSpiderfy

[London Socials #1 (1/2)]  index:990
[London Socials #2 (14/2)]  index:5027
[London Socials #3 (28/2)]  index:5013 (Active)

#3 is now the active item after spiderfy/unspiderfy (see final elements below)

Final marker element

<div class="awesome-marker-icon-blue awesome-marker leaflet-zoom-animated leaflet-interactive" 
title="London Socials #2 (14/2)" tabindex="0" style="margin-left: -17px; margin-top: -42px; width: 35px; height: 45px; transform: translate3d(83px, 218px, 0px); 
z-index: 5218;"><i class=" fa fa-moon  icon-white"></i></div>

<div class="awesome-marker-icon-blue awesome-marker leaflet-zoom-animated leaflet-interactive" 
title="London Socials #3 (28/2)" tabindex="0" style="margin-left: -17px; margin-top: -42px; width: 35px; height: 45px; transform: translate3d(83px, 218px, 0px); 
z-index: 5218;"><i class=" fa fa-moon  icon-white"></i></div>

And this is as far as I got to, unable to figure out how item 3 with a previously lower zIndex (compared to item 2) managed to end up on top with a new z-index: 5218 which just so happens to be the same new z-index value as for item 2!

​ If you find the time to look at this and perhaps examine the dev console results or anything else in the below jsfiddle below we would really appreciate your feedback.

https://jsfiddle.net/magicmb/17j2z4gy/

​ Environment specific versions: leaflet.js (1.6.0), Leaflet.awesome-markers (2.0.2), OverlappingMarkerSpiderfier-Leaflet (0.2.6)

post scriptum

It goes without saying OMS for Leaflet is a really fabulous project and much appreciated by an undoubtedly growing number of users. Many thanks ❤️

magicmb commented 4 years ago

Turns out it's not exactly a bug with OMS-Leaflet but more an issue with Leaflet and how it handles z-Indexes

Here's an extract from a thread I discovered on gis.stackexchange that was answered by YaFred way back in Oct 2 '14 and still very much relevant today:


Until somebody finds a better solution, here what I would do ...

As you noticed, leaflet is using pixel position to set zIndex (in Marker.js)

pos = this._map._latLngToNewLayerPoint(this._latlng, opt.zoom, opt.center).round();
this._zIndex = pos.y + this.options.zIndexOffset;

What I suggest is to undo leaflet zIndex using setZIndexOffset()

Say you want to set zIndex = 100, you would do

var pos = map.latLngToLayerPoint(marker.getLatLng()).round();
marker.setZIndexOffset(100 - pos.y);

There is a bit of a glitch: you have to do it every time the map is zoomed :( [EDIT: Spiderfied in our case!]

Here is a JSFiddle example (comment the code in adjustZindex() to see the difference)


For my OMS Spiderfy case this was resolved by adding:

function adjustZindex() {
    var markers = oms.getMarkers();
    for (var i = 0, len = markers.length; i < len; i ++) {
      var marker = markers[i];
      var latlng = marker._latlng, mIndexOffset = marker.options.mIndexOffset ;
      var pos = map.latLngToLayerPoint(latlng).round();
      marker.setZIndexOffset(mIndexOffset - pos.y);
    }
  }
  adjustZindex();

and then calling adjustZindex() from my spiderfy listeners

oms.addListener('spiderfy', function(markers) {
    layersBring2Front(); 
    adjustZindex();
  });

So I guess whether OMS should remember original zIndex of marker #76 was resolved in the version of OMS-Leaflet is kinda irrelevant as in all probability it would not have solved our issue.