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

GeomanDrawControl can modify arbitrary shapes on the map #1215

Closed
riccardoporreca opened this issue Jul 8, 2024 · 4 comments · Fixed by #1220
Closed

GeomanDrawControl can modify arbitrary shapes on the map #1215

riccardoporreca opened this issue Jul 8, 2024 · 4 comments · Fixed by #1220

Comments

@riccardoporreca
Copy link
Contributor

While the recent deprecation of DrawControl in favor of GeomanDrawControl (and its variant DrawControlCompatibility for compatibility) is in general a clear improvement, there is a key behavior difference: Geoman allows modifying any shapes on the map, whereas the legacy control would only allow modifying shapes that are drawn by the control itself.

Modifying other shapes with the legacy control can still be enabled using transform=True, whereas Geoman does not seem to honor such property nor provide a different mechanism to enable/disable editing shapes via ipyleaflet.

It would be desirable to make GeomanDrawControl and/or DrawControlCompatibility consistent with the existing and more natural behavior of DrawControl, or at least document alternative Geoman-specific ways to control what it allows to modify.

The behavior can be easily assessed with a simple example:

from ipyleaflet import *
m =  Map(basemap=basemaps.CartoDB.Positron, zoom = 4)
m.add(DrawControlCompatibility(position='topright'))
m.add(DrawControl())
m.add(LayerGroup(layers = [
        Polygon(locations=[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0]]),
        Polygon(locations=[[0.0, 10.0], [0.0, 20.0], [10.0, 20.0]], transform=True),
]))
pip list
Package            Version
------------------ -----------
anyio              4.4.0
appdirs            1.4.4
asgiref            3.8.1
asttokens          2.4.1
branca             0.7.2
certifi            2024.2.2
click              8.1.7
comm               0.2.2
debugpy            1.8.2
decorator          5.1.1
exceptiongroup     1.2.1
executing          2.0.1
h11                0.14.0
htmltools          0.5.2
idna               3.7
ipykernel          6.29.5
ipyleaflet         0.19.1
ipython            8.18.0
ipywidgets         8.1.3
isort              5.13.2
jedi               0.19.1
Jinja2             3.1.4
jupyter_client     8.6.2
jupyter_core       5.7.2
jupyter-leaflet    0.19.1
jupyterlab_widgets 3.0.11
linkify-it-py      2.0.3
markdown-it-py     3.0.0
MarkupSafe         2.1.5
matplotlib-inline  0.1.7
mdit-py-plugins    0.4.1
mdurl              0.1.2
mypy               1.10.0
mypy-extensions    1.0.0
nest-asyncio       1.6.0
numpy              1.26.4
packaging          24.0
pandas             2.2.2
parso              0.8.4
pexpect            4.9.0
pip                24.0
platformdirs       4.2.2
prompt-toolkit     3.0.36
psutil             6.0.0
ptyprocess         0.7.0
pure-eval          0.2.2
Pygments           2.18.0
pyproj             3.6.1
python-dateutil    2.9.0.post0
python-multipart   0.0.9
pytz               2024.1
pyzmq              26.0.3
questionary        2.0.1
setuptools         59.6.0
shapely            2.0.4
shiny              0.10.2
shinywidgets       0.3.2
six                1.16.0
sniffio            1.3.1
stack-data         0.6.3
starlette          0.37.2
tomli              2.0.1
tornado            6.4.1
traitlets          5.14.3
traittypes         0.2.1
typing_extensions  4.12.0
tzdata             2024.1
uc-micro-py        1.0.3
uvicorn            0.30.0
watchfiles         0.22.0
wcwidth            0.2.13
websockets         12.0
widgetsnbextension 4.0.11
xyzservices        2024.4.0
@iisakkirotko
Copy link
Contributor

Hey @riccardoporreca!

Thanks a lot for the issue, I think we should indeed at least match the old behaviour here. I'll take a look at this next week.

@riccardoporreca
Copy link
Contributor Author

@iisakkirotko, I was digging this a bit myself, and I might have found an easy way to control the Geoman behavior.

According to https://www.geoman.io/docs/getting-started/free-version#exclude-layers

If you want certain layers to be ignored by Leaflet-Geoman, pass pmIgnore: true to
their options when creating them.

I gave it a quick try by simply adding

pm_ignore = Bool(True).tag(sync=True, o=True)

to the definition of the top class Layer

class Layer(Widget, InteractMixin):

and this seems to work well in disabling editing by default, possibly re-enabled via pm_ignore=False. Here my example extend:

from ipyleaflet import *
m = Map(basemap=basemaps.CartoDB.Positron, zoom = 4)
m.add(DrawControlCompatibility(position='topright'))
m.add(LayerGroup(layers = [
    Polygon(locations=[[0.0, 0.0], [0.0, 10.0], [10.0, 10.0]]),
    Polygon(locations=[[0.0, 10.0], [0.0, 20.0], [10.0, 20.0]], transform=True),
    Polygon(locations=[[0.0, 20.0], [0.0, 30.0], [10.0, 30.0]], pm_ignore=False),
    Polygon(locations=[[0.0, 30.0], [0.0, 40.0], [10.0, 40.0]], transform=True, pm_ignore=False),
]))

Note that, in fact, transform=True does not have anything to do with draw controls (bad understanding from my side, sorry).

I am happy to contribute a small PR, with the inclusion of pm_ignore in L.Layer, and mentioning GeomanDrawControl documentation the possibility to define explicitly pm_ignore=False when creating a shape/layer to allow Geoman to modify it.

@iisakkirotko
Copy link
Contributor

iisakkirotko commented Jul 22, 2024

Hey @riccardoporreca!

I was just looking at this myself, and noticed that transform=True indeed doesn't let LeafletDraw edit the layer (and moreover is only available for Polylines and derived shapes). I agree with your conclusion of using pmIgnore/pm_ignore per layer is the best approach. I also fiddled a bit with using L.PM.setOptIn(true), but it seems to result in errors when adding the controls.

If you want to contribute a PR, I'd be happy to take a look at it. Let me know if you need a hand with any of it, you can reach me here, or at [email protected]!

@riccardoporreca
Copy link
Contributor Author

@iisakkirotko, here you go: #1220

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 a pull request may close this issue.

3 participants