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.

Sign up for GitHub

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

Jump to bottom

Draw control can't stop drawing after draw more than one shape #1119

Open
J-Bindel opened this issue Jun 19, 2023 · 8 comments · May be fixed by #1133
Open

Draw control can't stop drawing after draw more than one shape #1119

J-Bindel opened this issue Jun 19, 2023 · 8 comments · May be fixed by #1133

Comments

@J-Bindel
Copy link

Hey,

I am using ipyleaflet for my work. I would like to draw rectangles on my map with DrawControl.

from ipyleaflet import Map, DrawControl

map = Map(center=(49.115469, -1.082814), zoom=10)
dc = DrawControl(
    marker={"shapeOptions": {"color": "#0000FF"}},
    rectangle={"shapeOptions": {"color": "#0000FF"}},
    circle={"shapeOptions": {"color": "#0000FF"}},
    circlemarker={},
)
map.add(dc)
display(map)

The first drawing works but when I click again on the rectangle for example, DrawControl won't stop drawing rectangles until I clear them all.

The above code reproduces the bug.

Here is what I get.
Screencast from 19-06-2023 18:39:08.webm
@cr458
Copy link

cr458 commented Jul 5, 2023

+1 also might be somewhat related: I can't draw more than one polygon, the second polygon I try to draw never closes. Similar to.
@J-Bindel what version of python and ipyleaflet are you using?

@J-Bindel
Copy link
Author

J-Bindel commented Jul 6, 2023

I am using:

  • ipyleaflet: 0.17.3
  • Python: 3.8

@mangecoeur
Copy link
Contributor

I see something similar, the draw menu stays open and you have to click save then cancel and it seems to work but its janky.

Checking the javascript console I see the error:

Error serializing widget state attribute:  data [widget.js:492:24](webpack://widgetti/solara-widget-manager8/node_modules/@jupyter-widgets/base/lib/widget.js)
    serialize widget.js:492
    sync widget.js:416
    save Backbone
    save_changes widget.js:549
    layers_to_data DrawControl.js:148
    create_obj DrawControl.js:95
    fire leaflet-src.js:613
    _fireCreatedEvent leaflet.draw.js:8
    _fireCreatedEvent leaflet.draw.js:8
    completeShape leaflet.draw.js:8
    s leaflet-src.js:2692
    (Async: EventListener.handleEvent)
    Te leaflet-src.js:2719
    ke leaflet-src.js:2618
    _createButton leaflet.draw.js:9
    _createActions leaflet.draw.js:9
    _showActionsToolbar leaflet.draw.js:9
    _handlerActivated leaflet.draw.js:9
    fire leaflet-src.js:613
    enable leaflet.draw.js:8
    s leaflet-src.js:2692

My unconfirmed hunch digging into the code is that line 39 in DrawControl.js

    this.feature_group = L.geoJson([], {
      style: function (feature) {
        return feature.properties.style;
      },

is setting a property as a function which fails to be synced by the widget.

@mangecoeur
Copy link
Contributor

also possibly related jupyter-widgets/ipywidgets#3735

@sufyanAbbasi
Copy link

I can reproduce your findings by running ipyleaflet in Jupyer-Lab, editing a polygon and hitting save. This does nothing and the console (with breakpoints) shows the following errors:

Uncaught DOMException: Failed to execute 'structuredClone' on 'Window': function(e){return e.properties.style} could not be cloned.

Which is caught and logged:

Error serializing widget state attribute:  data

resulting in the data attribute not being reflected of the geometries on the JS layer.

This can be traced to the ipywidget serialization logic here:
https://github.com/jupyter-widgets/ipywidgets/blob/e1718c2b3bf0b143580ef87f71c55fbc6ed50a77/packages/base/src/widget.ts#L587

which is corroborated by the above discussion about moving away from structuredClone.

Can we clean up newData before we set it on the model?

https://github.com/jupyter-widgets/ipyleaflet/blob/ddeab67de9dff00df8a14ebb37216277e026d063/js/src/controls/DrawControl.js#L147C5-L147C37

sufyanAbbasi added a commit to sufyanAbbasi/ipyleaflet that referenced this issue Aug 11, 2023
@sufyanAbbasi
Sanitize style attributes in data attribute in DrawControl.js for ups…
a6a8b6b 
…tream serialization.

- Fix for jupyter-widgets#1119

- The main issue is that a `style()` function is added to the GeoJSON object in `create_obj()`, which triggers an error at serialization time: https://github.com/jupyter-widgets/ipywidgets/blob/e1718c2b3bf0b143580ef87f71c55fbc6ed50a77/packages/base/src/widget.ts#L587

- This is because `structuredClone` throws an error for functions: https://web.dev/structured-clone/#features-and-limitations. While the article mentions drawbacks for using `JSON.parse(JSON.stringify(...))`, I believe that this component does not produce any non-primitive properties, so will remove any offending lines.

- Tested by placing a breakpoint and testing directly on the browser.
@sufyanAbbasi sufyanAbbasi linked a pull request Aug 12, 2023 that will close this issue
Draft
@sufyanAbbasi sufyanAbbasi mentioned this issue Aug 19, 2023
Merged
@naschmitz naschmitz mentioned this issue Feb 16, 2024
Open
@joHussien
Copy link

@J-Bindel
Did you find a solution to this problem?

@J-Bindel
Copy link
Author

J-Bindel commented Apr 3, 2024

No, sorry 😔

@mangecoeur
Copy link
Contributor

mangecoeur commented Apr 3, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Sanitize style attribute in DrawControl.js for data attribute serialization sufyanAbbasi/ipyleaflet
5 participants
@mangecoeur @cr458 @sufyanAbbasi @joHussien @J-Bindel