Skip to content

WIP: integrate edge creation/deletion#40

Merged
reddigari merged 8 commits intomasterfrom
dev/samir
Apr 7, 2020
Merged

WIP: integrate edge creation/deletion#40
reddigari merged 8 commits intomasterfrom
dev/samir

Conversation

@reddigari
Copy link
Collaborator

@reddigari reddigari commented Apr 4, 2020

Integrates front-end API methods for adding and deleting edges. A few identified bugs:

  • When you delete an edge (by selecting it and hitting the delete key), the edge stays there until you click somewhere else on the workspace. I can't figure out how to access the engine to repaint the canvas from the link model. This also happens when clearing the workspace.
  • Because the API call happens from a listener within the react-diagrams classes, it is also triggered when loading a workflow from file. The server handles it fine (with a message that edge already exists), but ideally we want a way to only fire the callback when an edge is created with the mouse. I think @cesaragv is looking into how to do this (probably in the DefaultPortWidget component?).

This feels a little too buggy to merge, but I wanted to make the commits accessible.

@reddigari
Copy link
Collaborator Author

Given that #41 builds off this and provides solid functionality, I guess we should probably merge this once reviewed, and fix the bugs later.

Copy link
Member

@reelmatt reelmatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the visual bug you mentioned, I think this is good to merge. Should we want to "fix" the bug for a demo, I made a comment on one approach we could take.

To build on the note re: edge API triggering. The same problem seems to be present in reverse where the 'remove edge' API call fails because the Node it was connected to doesn't exist. This shouldn't be a problem in NetworkX, because removing a node also removes all associated edges. So to add to @reddigari's point, I think we want API calls triggered for:

  • Edge creation only when done with the mouse
  • Edge deletion only when 'delete' key is pressed

Copy link
Collaborator

@diegostruk diegostruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is good to merge this one before the other 2 PRs.

@reddigari reddigari merged commit 18d83a2 into master Apr 7, 2020
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.

3 participants