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

Bugfix drag and drop #5061

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

itepifanio
Copy link

Code PR

This PR fixes #4374. The problem is that the Layout is initialized with no shapes and the drag and drop would trigger a relayout_data with shapes that weren't available at the CompoundArrayValidator (since it's initialized one time).

I'm not that familiar with the codebase, so I added a specific condition for this special case. I'm open to know more about the plotly validator flow if it can improve the code.

The internal calls of the basedatatypes are quite complex, recursive calls everywhere, it's hard to track what's happening. Do you have any docs about this module? I don't think the docstring are enough to understand the big picture.

  • I have read through the contributing notes and understand the structure of the package. In particular, if my PR modifies code of plotly.graph_objects, my modifications concern the codegen files and not generated files.
  • I have added tests (if submitting a new feature or correcting a bug) or
    modified existing tests.
  • For a new feature, I have added documentation examples in an existing or
    new tutorial notebook (please see the doc checklist as well).
  • I have added a CHANGELOG entry if fixing/changing/adding anything substantial.
  • For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).

I actually tried to reproduce the error by writing a test programatically, but it didn't work. I wasn't able to find a way to reproduce the js events in the python code and relying on the relayout event wasn't enough to reproduce it.

def test_plotly_relayout_drag_and_drop_polygons() -> None:
    """
    Validate that the plotly_relayout method can handle drag and drop.
    """

    fig = go.FigureWidget(
        layout=go.Layout(
            showlegend=False,
            autosize=True,
            dragmode="drawrect",
            modebar={
                "add": [
                    "drawclosedpath",
                    "drawcircle",
                    "drawrect",
                    "eraseshape",
                ]
            },
        )
    )
    fig.add_shape(
        type="rect",
        x0=-0.06666666666666662,
        y0=2.4908854166666665,
        x1=2.1500000000000004,
        y1=0.3519965277777778,
    )

    new_shape = {
        "x0": -0.07964149504195264,
        "x1": 3.692616323417239,
        "y0": 3.1297222222222225,
        "y1": 1.213055555555555,
    }

    drag_and_drop_relayout_data = {
        f'shapes[0].{pos}': value for pos, value in new_shape.items()
    }

    fig.plotly_relayout(
        relayout_data=drag_and_drop_relayout_data
    )

    for pos, value in new_shape.items():
        assert fig.layout.shapes[0][pos] == value

@itepifanio itepifanio force-pushed the bugfix-drag-and-drop branch from 400ba7c to fd49c17 Compare March 3, 2025 15:05
@gvwilson gvwilson added feature something new P2 considered for next cycle community community contribution labels Mar 3, 2025
@gvwilson
Copy link
Contributor

gvwilson commented Mar 3, 2025

thanks very much @itepifanio - I'll try to get someone to review this in our next cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FigureWidget doesn't allow draggable polygon
2 participants