GeoJSON: option to keep layer highlighted when popup is open

[GiuseppeGalilei]

Problem

Currently, the highlighting option available for GeoJson layers is active only when hovering with the mouse above the layer.
I'm working on a project where it is useful to keep the highlighting active after the user clicks on a layer, a popup is opened and the mouse is moved around. Then simply closing the popup will deactivate the highlighting.

Solution

To achieve so, the boolean parameter "popup_keep_highlighted" has been added to the "folium.GeoJson" function, and a test has also been implemented to check if the user tries to use it without a popup, in which case an error is raised.
Then, based on this boolean parameter, the template gets slightly modified when dealing with the different mouse and popup events.
The standard behaviour of highlighting when hovering remains untouched.
But when popup_keep_highlighted=True: the popupopen and popupclose event control activation and deactivation of the highlighting, while mouseout deactivates the highlighting only if the popup is closed and mouseover remains untouched,

Example of usage

        layer = folium.GeoJson(
                       path,
                       name = name,
                       highlight_function=lambda feature: {
                            "color": ("red"),
                            "opacity": 1,
                            "weight": 6
                            },
                        popup_keep_highlighted=True,
                        popup=popup,
                        style_function=lambda feature: {
                            "color": "black",
                            "weight": 4,
                            "opacity":0.5,
                        },
                  )

Demo

You can see the user first hovering over the track, then clicking on it to display the popup and moving the mouse elsewhere, to showcase the different behaviour.

before.mp4

after.mp4

It's my first pull request and I don't have much experience with this project, any advice is warmly welcomed.

[Conengmo]

Interesting! I think your PR is really clear, so no comments there now. I’ll review it shortly.

[Conengmo]

I think this is a nice addition. I've added some comments to hopefully sharpen the change a little bit. Do you have time to look into those?

Optionally, you could also consider adding an example to our documentation. But it's not required to get this PR merged.

[GiuseppeGalilei]

Ok, thank you!
I'll fix the problems you pointed out, shortly.
I'll be happy to contribute to the documentation, however you'll need to wait a few days for that.

[Conengmo]

I'll be happy to contribute to the documentation, however you'll need to wait a few days for that.

that’s great! Alright let’s not let that hold up this PR then though.

[GiuseppeGalilei]

Done, can you review it?

[Conengmo]

Code looks good! I'll give it a try still though before merging this one. But no further work needed.

[Conengmo]

I found a bug when using GeoJsonPopup, where the highlight won't go away as long as any popup is open. Steps to reproduce are: click on a state, then hover over other states.

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"])

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

m.show_in_browser()

@GiuseppeGalilei probably we need something on line 578 that checks whether the popup is open on the current element, not on the entire geojson object. Could you take a look?

[GiuseppeGalilei]

Sorry for the bug. For my use case I build the map in a sligthly different way and I didn't consider this possibility. I'll take a look at it!

[GiuseppeGalilei]

So, the issue was that, in the map you used, every popup was bound to a common layer which contains all the geojson information. While, in my case, I show some gps tracks on a map where each track has its own layer and popup.

I found a solution which seems to work in both cases:
On popup open, the popup is bound to the correct target layer. While on popup close it is unbound.
This way the condition of whether to reset the style on mouseout can be defined upon the popup bound to the correct target layer.

What do you think?

[Conengmo]

That makes sense! I think your solution is good. Now I want to look whether it's possibly better to change something in GeoJsonPopup. There we bind the popup to the entire geojson, and put logic per element in the function. I'm curious if it's better to bind the popup to each element to begin with. So we don't have to do that post-hoc.

If you're interested you could take a look at that. I don't know how you are timewise, but you're very welcome to. Otherwise I'll take a look myself.

[GiuseppeGalilei]

Yeah, to be honest I had the same idea.
I tried to look into that, but it seemed something that could impact quite a bit of codebase and maybe project structure.
So, given that I'm fairly new to the project and especially to javascript, I decided to adopt the cleanest and simplest approach (codewise).

Unfortunately I'm quite busy at the moment.
Maybe, given that you know the project throughly, you can brainstorm your ideas here and I'll try to give you some inputs if I can. I'm very curious to hear your insights, especially regarding how to integrate changes while keeping the design philosophy consistent.

[Conengmo]

Yeah, to be honest I had the same idea.
I tried to look into that, but it seemed something that could impact quite a bit of codebase and maybe project structure.
So, given that I'm fairly new to the project and especially to javascript, I decided to adopt the cleanest and simplest approach (codewise).

That makes a lot of sense, and I can totally agree with that. Let's say I'll give it a look over just to make sure. That will inform whether we go ahead with this approach and/or we add some more changes.

[Conengmo]

@GiuseppeGalilei I looked into this a bit, and it seems pretty straight forward to bind the popups to each feature instead of the entire geojson. I made a PR, could you maybe review it? #1844

Also, about this PR here, I'm now thinking maybe this shouldn't be a parameter, but we should just enable this behavior by default. Is there a use-case for keeping the old behavior? In what case would someone not want the highlight enabled when a popup is open?

[GiuseppeGalilei]

@Conengmo Awesome! Yes, you can add me as a reviewer, I'm curious about the process. I'll try to take a look at it and test it later in the day.
About this PR default behaviour, I suppose I agree with you, but I don't know whether the old behaviour may be preferred in some use cases. So, I guess, the safer route would be to leave a parameter and default it to True, just to leave the option.
Anyway, I'll think more about it later in the day.

[GiuseppeGalilei]

#1844 progress will inform how to continue developing this.
As of now, it removes the 'bind/unbind' necessity for geojson but leaves behind regular popups, which still need the older method. It would be better to uniform this behaviour.

[Conengmo]

Thanks for your contribution @GiuseppeGalilei! I'm happy we got to explore this problem and I'm also happy with your solution. Happy holidays!

[GiuseppeGalilei]

Thank you @Conengmo! It was my first pull request and I really enjoyed the whole process. Happy holidays to you!
In the coming days I'll try to write the relative documentation.