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

Fix geojson issue with marker cluster #1188

Closed

Conversation

fullonic
Copy link
Contributor

This PR fixed #1129
My understating of the cause of this issue is because we are currently adding a geojson with a null value to cluster marker layer and only later adding the geojson data.

var {{ this.get_name() }} = L.geoJson(null, {

And because of this, the following code produces a empty map:

import folium
folium.__version__
  '0.10.0'
from folium.plugins import MarkerCluster
m = folium.Map()
mc = MarkerCluster()
geojson = folium.GeoJson("random_points.geojson")
mc.add_child(geojson)
mc.add_to(m)
m.save("cluster.html")

The cluster.html html code:

(....)

  var marker_cluster_e6eb5734160c404c8ba80bee9e023ff6 = L.markerClusterGroup(
      {}
  );
  map_555133c3ff134e328f848608cd78fce6.addLayer(marker_cluster_e6eb5734160c404c8ba80bee9e023ff6);

function geo_json_93c25184bf6049d2a7b7b92750ddd587_onEachFeature(feature, layer) {
  layer.on({
      click: function(e) {
          map_555133c3ff134e328f848608cd78fce6.fitBounds(e.target.getBounds());
      }
  });
};
var geo_json_93c25184bf6049d2a7b7b92750ddd587 = L.geoJson(null, {
      onEachFeature: geo_json_93c25184bf6049d2a7b7b92750ddd587_onEachFeature,

}).addTo(marker_cluster_e6eb5734160c404c8ba80bee9e023ff6);  # **Here we are adding the empty geojson layer**

geo_json_93c25184bf6049d2a7b7b92750ddd587.addData({GEOJSON_DATA});
(....)

However, with the same python code, if we change the html code and add the geojson data at the same time we create the geojson layer, as bellow, the markers are displayed on map.

var geo_json_93c25184bf6049d2a7b7b92750ddd587 = L.geoJson({**GEOJSON_DATA**}, {
      onEachFeature: geo_json_93c25184bf6049d2a7b7b92750ddd587_onEachFeature,

}).addTo(marker_cluster_e6eb5734160c404c8ba80bee9e023ff6);

This PR fixed this issue and at the same time makes it possible have data not embed on html file when an url is passed as data source.

Copy link
Member

@Conengmo Conengmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, linking geojson to marker cluster is broken at the moment. To be honest I didn't know we offered that functionality. Thanks for the well-written problem description.

I tried your fix, but it seems it breaks LayerControl when using GeoJson normally and with non-embedded data. While looking into this I came up with an alternative approach. I'll make a PR, maybe you can check it out, let me know what you think? #1190 Then we can discuss how to move on.

}});
var json_url = $.ajax({url: {{ this.embed_link|tojson }}, dataType: 'json', async: true })
$.when(json_url).done(function() {
var geo_json_32b06f4a0e6d419c8f501b4e6c43bc57 = L.geoJson(json_url.responseJSON
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that this breaks LayerControl for regular geojson use cases. This variable has to be declared and defined outside of the ajax call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in fact it breaks. I wasn't aware of it. I just tried your PR and works as expected 👍

{%- endif %}
}).addTo({{ this._parent.get_name() }});
var options = {
{%- if this.smooth_factor is not none %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you remove the indentation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be automatic linting, I didn't see it. Sorry

{{ this.get_name() }}.addData(data);
}});
var json_url = $.ajax({url: {{ this.embed_link|tojson }}, dataType: 'json', async: true })
$.when(json_url).done(function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can link this to the ajax call directly: $.ajax().done(function() {});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried? Because the problem I had was related with async ajax request and because of that I used a variable to check if the request was finished. I don't have experience with JS. I just searched about it and I found that solution in a blog post.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work here, and is also listed in the examples in the jQuery documentation, so assume it should be fine.

@Conengmo
Copy link
Member

This problem has been fixed in #1190.

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

Successfully merging this pull request may close these issues.

Unable to cluster a geojson layer
2 participants