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

selection:changed event handler returns cells already removed in the graph #2466

Closed
xushangning opened this issue Aug 7, 2022 · 4 comments · Fixed by #2472
Closed

selection:changed event handler returns cells already removed in the graph #2466

xushangning opened this issue Aug 7, 2022 · 4 comments · Fixed by #2472

Comments

@xushangning
Copy link

Describe the bug

When a node and an edge are deleted together with Graph.removeCells, the selected property passed by Graph's selection:changed event contains cells that don't exist in the graph.

Your Example Website or App

See the minimal reproducible example below.

Steps to Reproduce the Bug or Issue

  1. Paste the following code into an HTML file and run. 
    <html>
    <head>
        <script src="https://unpkg.com/@antv/x6/dist/x6.js"></script>
    </head>
    <body>
        <div id="container" style="width: 400px; height: 300px"></div>
        <script>
            const graph = new X6.Graph({
                container: document.getElementById('container'),
                resizing: true,
                selecting: {
                    enabled: true,
                },
                keyboard: true,
            });

            graph.on('selection:changed', ({ selected }) => {
                console.log(
                    'Cells returned by selection:changed',
                    selected.map(cell => graph.getCellById(cell.id))
                )
            })

            graph.bindKey(['delete', 'backspace'], () => {
                graph.removeCells(graph.getSelectedCells())
            })

            const data = {
                nodes: [
                    {
                        id: 'node1',
                        x: 40,
                        y: 40,
                        width: 80,
                        height: 40,
                        label: 'Hello',
                    },
                    {
                        id: 'node2',
                        x: 160,
                        y: 180,
                        width: 80,
                        height: 40,
                        label: 'World',
                    },
                ],
                edges: [
                    {
                        source: 'node1',
                        target: 'node2',
                    },
                ],
            }
            graph.fromJSON(data)
        </script>
    </body>
</html>
  2. Open the developer console. In the graph, use command or Ctrl to first select a node and then an edge. Then, press Backspace or Delete to delete them.
  3. In the console, one array of cells returned by selection:changed should contain a null returned by getCellById, indicating that the cell is no longer in the graph.

Expected behavior

selection:changed should always return cells that exist in the graph.

Screenshots or Videos

The following screenshot shows the developer console after a bug is reproduced:

Screen Shot 2022-08-06 at 19 16 34

Platform

  • OS: macOS
  • Browser: Chrome 103.0.5060.134

Additional context

The bug is likely caused by a corner case in Model.removeConnectedEdges. Because a node's connected edges are removed after the node itself but before unselecting the node, the selection still contains the already-removed node. When the selection:changed event is fired by the removal of connected edges, the removed node is contained in the selected property.
@x6-bot
Copy link
Contributor

x6-bot bot commented Aug 7, 2022

👋 @xushangning

Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it.

To help make it easier for us to investigate your issue, please follow the contributing guidelines.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@NewByVector NewByVector mentioned this issue Aug 8, 2022
Merged
15 tasks
@NewByVector
Copy link
Contributor

更新到 1.33.0 版本。

@whatoeat2night
Copy link

已更新

@x6-bot
Copy link
Contributor

x6-bot bot commented Aug 30, 2023

This thread has been automatically locked because it has not had recent activity.

Please open a new issue for related bugs and link to relevant comments in this thread.

@x6-bot x6-bot bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: fix some issue antvis/X6
3 participants
@whatoeat2night @NewByVector @xushangning