Skip to content

fix: geojson layers pmIgnore #1251

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lopezvoliver
Copy link
Contributor

Similar to #1249 , this PR updates jupyter_leaflet so that individual layers created by the LeafletGeoJSONModel get the proper pmIgnore option -- and disable the pm if it's already attached. This fixes the issue that Geoman is able to edit GeoJson layers, even though they inherit the pm_ignore from the base Layer class.

@mangecoeur
Copy link
Contributor

There's probably overlap with this PR #1247

@lopezvoliver
Copy link
Contributor Author

There's probably overlap with this PR #1247

Hey @mangecoeur, I tested an environment with PR #1247 and Geoman was still able to modify GeoJSON layers:

bug_geojson_layer

I think the important part of this PR that solves this issue is this:

            if (pmIgnore && layer.pm) {
              layer.pm.disable();
              delete (layer as any).pm;
            }

@mangecoeur
Copy link
Contributor

mangecoeur commented May 21, 2025

Thanks, actually for my current use i mashed together both my current PR and your fixes into a kind of frakenpackage because I couldn't wait for a new ipyleaflet release. Would really like to land all of these fixes in the main repo instead.

See the branch here (https://github.com/mangecoeur/ipyleaflet/tree/merge_extra_geoman_prs) I was actually in such a rush that I just copy-pasted your file changes rather than merge from your branch 😅. Would rather re-do it clean instead.

@lopezvoliver
Copy link
Contributor Author

Same 😅 -- I have a custom mix and match of recent fixes. Hopefully the maintainers have some time to review these soon 👀

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.

2 participants