python-visualization / folium

Python Data. Leaflet.js Maps.
https://python-visualization.github.io/folium/
MIT License
6.94k stars 2.23k forks source link

GeoJson bind popup/tooltip per feature #1844

Closed Conengmo closed 11 months ago

Conengmo commented 11 months ago

Change that's useful for https://github.com/python-visualization/folium/pull/1836.

When binding GeoJsonPopup/GeoJsonTooltip to the GeoJson, we currently do that to the full object. Which then 'distributes' the bindings to the underlying layers. See https://leafletjs.com/reference.html#featuregroup.

Instead, we want to bind the popup/tooltip to each underlying layer directly. That way we can check whether the popup of each layer is open or not.

I checked the example in our documentation and it functions correctly. Here's also a short snippet to test it with:

import folium
import geopandas
import requests

m = folium.Map(location=[35.3, -97.6], zoom_start=4)

data = requests.get(
    "https://raw.githubusercontent.com/python-visualization/folium-example-data/main/us_states.json"
).json()
states = geopandas.GeoDataFrame.from_features(data, crs="EPSG:4326")

popup = folium.GeoJsonPopup(fields=["name"])
tooltip = folium.GeoJsonTooltip(fields=["name"])

folium.GeoJson(
    states,
    highlight_function=lambda x: {
        "fillOpacity": 0.4,
    },
    popup=popup,
    tooltip=tooltip,
).add_to(m)

m.show_in_browser()
GiuseppeGalilei commented 11 months ago

I tested your changes and it seems to work as expected. I tried to implement the changes in #1836 over your proposed example and managed to get it working without the bind/unbind methods, just e.target.isPopupOpen() was required.

I also tested it over my specific use case (more details in #1836), but it didn't work because there I'm using regular Popup instead of GeoJson ones. So I tried your changes also with regular popups by modifying directly the map html, for one popup it looks like this geo_json_4bd24b389c32577be931064ba7343f7b.eachLayer(function(layer){layer.bindPopup(popup_99c444150b44b79eef413e2974774529)}). And this seems to work as expected.

Do you think this change can also be implemented for regular popups and tooltips? Then, based on this, we can choose the direction to follow for #1836.

Conengmo commented 11 months ago

Thanks for your review @GiuseppeGalilei! And good find. Let's close this PR then and just go with the implementation in https://github.com/python-visualization/folium/pull/1836!