Skip to content

fix popup sticky options #2048

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

Closed
wants to merge 2 commits into from

Conversation

Conengmo
Copy link
Member

Fix #2047

This is a similar issue to #2043, happening after the v0.19.0 release, caused by changes in #2029.

Fix it by using proper boolean values for these options on Popup, not None values.

@Conengmo Conengmo requested a review from hansthen December 12, 2024 18:09
@Conengmo
Copy link
Member Author

@hansthen I'm not sure if we're going the right way by addressing these individually. Looking at https://github.com/python-visualization/folium/pull/2029/files there are lots of places where parse_options was removed, but the default arguments are None, so we are adding null options that were not there before. That means potentially lots more issues like this one.

I think we should go with a simple solution for now, because we have a release out and I'd like to avoid people getting bit by this. Maybe either:

For that first solution, do you reckon that will lead to issues? Is there a use for explicit null arguments in Leaflet?

@hansthen
Copy link
Collaborator

I would prefer to leave None values in the tojavascript because null values are part of the json spec. My feeling is that tojavascript is more generic than the original parse_options, even though it largely serves the same function. Also, I can't exclude the possibility that Leaflet options can be validly null.

I am open to reverting #2029 for now and do a proper release later. However, I did make a simple fix that again removes None values from the options. You could have a look at that to see if you think it is a viable solution. I still have to fix four tests though.

@Conengmo
Copy link
Member Author

Close in favor of #2049

@Conengmo Conengmo closed this Dec 13, 2024
@Conengmo Conengmo deleted the fix-popup-sticky branch December 13, 2024 11:52
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.

Popup sticky stays True in version>=0.19.0
2 participants