Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RefreshClusters VS singleMarkerMode: true #581

Closed
oldrich-s opened this issue Oct 18, 2015 · 6 comments
Closed

RefreshClusters VS singleMarkerMode: true #581

oldrich-s opened this issue Oct 18, 2015 · 6 comments

Comments

@oldrich-s
Copy link

When using singleMarkerMode: true, the refreshClusters function should also run iconCreateFunction for all the end markers, not only cluster markers.

As it is now the cluster markers are updated but normal markers are not updated.

@ghybs
Copy link
Contributor

ghybs commented Oct 18, 2015

Hi,

Good point.

We could add this functionality, or we could take the opportunity to re-visit this option singleMarkerMode to have real clusters containing the single markers, so that the behaviour is more consistent (see for example issue #391).

I guess the less effort rule will guide us towards the first option...
Any thoughts?

@oldrich-s
Copy link
Author

Well, I do not know leaflet that much but to me it seems strange that you cannot use singleMarkerMode with disableClusteringAtZoom. Personally I would like to disable mark clusters on maximum zoom - I do not like spidering functionality.

What I try to achieve is to have fully dynamic styling of markers and marker clusters.

Let say that you mark bridges on a map. Each bridge has bad or good state (or yet another state). Each state of the bridge has a particular styling (bad state == red colour, good state == green colour). The mark clusters should be styled according to the worst state of bridges inside that cluster. State of the bridges can change at any time and should be immediately reflected in the map. How would you solve such particular case? I use singleMarkerMode and style both markers and marker clusters according to the bridge states.

@ghybs
Copy link
Contributor

ghybs commented Oct 18, 2015

The "incompatibility" between singleMarkerMode and disableClusteringAtZoom is purely due to how these options are implemented, and is in fact just a visual artefact. In the case of singleMarkerMode, it appears that the "trick" to simply override the single marker icon was chosen on purpose (#42).

It is understandable to do it that way, and it sounds to fit for your use case as well: it conveniently avoids having to style your individual markers, while using the exact same iconCreateFunction to get similar style as clusters. E.g. you have a 1 bridge picture when single, and "stacked bridgeS" picture when clustered.

If using singleMarkerMode as it is now fits your need, the disableClusteringAtZoom is actually compatible: it prevents several markers from merging together past that zoom level, even if their style still looks like single child clusters. Hence you no longer need spiderfication.

If on the contrary your individual markers needed differentiated styling (shape of the bridge let's say), then you would prefer normal mode, which is already covered by refreshClusters as it is today. With that mode, you would have to change the marker icon yourself to reflect your change of state, before calling the method to propagate the change to clusters.

So if my understanding is correct, there is no strong need for real clusters containing single markers, and the trick to style the markers with the same iconCreateFunction covers more use cases.
In that case, the refreshClusters method should definitely also re-draw the individual markers.

Is that what you mean?

I think the situation would be more clear if the option singleMarkerMode had been called with something like "markersIconOverride" or "restyleMarkersLikeClusters". The current name make us believe that it creates clusters even for a single marker (which was the original intention according to #42), whereas it actually just replaces the markers icon. The README looks to have a good description of this, but the option name is still misleading. Unfortunately once an API is released, it is almost impossible to change it except through a major revision.

Does that make sense?

ghybs added a commit to ghybs/Leaflet.markercluster that referenced this issue Oct 19, 2015
Following issue Leaflet#581, improved refreshClusters() method so that it detects singleMarkerMode and re-draws all passed markers as well, not only parent clusters.

Also created the singleMarkerModeSpec test suite to test singleMarkerMode option, added an extra test in RefreshSpec test suite to cover this case, and added the new suite into spec/index.
@oldrich-s
Copy link
Author

Hi,

thanks for the explanation. I have just tested you latest commit and it works perfectly! Thanks a lot for your help.

@ghybs
Copy link
Contributor

ghybs commented Oct 20, 2015 via email

@danzel
Copy link
Member

danzel commented Oct 21, 2015

Completed by @ghybs in #583

@danzel danzel closed this as completed Oct 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants