projectstorm / react-diagrams

a super simple, no-nonsense diagramming library written in react that just works
https://projectstorm.cloud/react-diagrams
MIT License
8.61k stars 1.17k forks source link

Remove port with it's link and target port #659

Closed stanislav-grin closed 3 years ago

stanislav-grin commented 4 years ago

Hello! First of all, thanks for this awesome project!

I ran into issue when trying to remove port with its link and related (target) port. I'm successed only with removing source port and link, but not target port.

Basically I'm doing some kind of UML editor, but for custom solution (will show it later when it's ready). Here is how it looks now. Pay attention to the node named Address, port contacts and target node Contacts with target port Address[contacts].

image

When I select needed port, I click delete and run next logic:

const port = node.getPort(portId)
const links = port.getLinks()
const link = links[Object.keys(links)[0]] // didn't find another way to get port's link, its always only single one, so I just take first item from the links object, it works. Any ideas much appreciated

const targetPort = link.getTargetPort() // get target port - `Address[contacts]`
const targetNode = model.getNode(targetNodeId) // node with title 'Contacts'

targetNode.removePort(targetPort) // remove target port in target node
targetPort.remove() // another try to remove
link.remove()

node.removePort(port)

engine.repaintCanvas()

But the only items that removed - source port (contacts) and link. Target port Address[contacts] remains. I need this to remove also.

Any help please?

renato-bohler commented 4 years ago

Hey there. I don't immediately see why this isn't working.

Where is this code located? If this is the behaviour you want, maybe it's a good idea to override your link model's remove method to do all of this... maybe something like this:

remove() {
  if (this.sourcePort) {
    this.sourcePort.remove();
    this.sourcePort = null;
  }
  if (this.targetPort) {
    this.targetPort.remove();
    this.targetPort = null;
  }
  super.remove();
}

And about the Any ideas much appreciated comment :sweat_smile: in this case I think its a good idea to create a method on your custom port model, like below, so you can easily get a port's link having the port model.

getLink() {
  const links = Object.values(this.getLinks());
  return links.length > 0 ? links[0] : null;
}
stanislav-grin commented 4 years ago

Hi @renato-bohler,

thanks for your answer! :)

I tried the way you suggest (overriding custom link model's remove), but unfortunatelly no luck.

Where is this code located?

This code is in the custom node widget. There I render node with ports and some additional controls at the bottom (like create/edit/delete port - in my case port = table column). When user selects port, node widget receive information about that (through handler provided in port props) and then user can manipulate with port data. When user select port to delete it, and when port - column that has relation, then relation should be deleted also (target port in another node and link between ports). Source port and link deleted successfully, but target port remains, it is not deleted. I have 100 times checked each variable and ensured that targetPort and targetNode is exactly what it should be.

BTW similar problem I have when creating new port in existing node, then create target port in target node and trying to link it between. I'm succeded in creating only source port, but not the target port and thus (I think) link between them. Not sure if this is related to current topic, but looks for me very similar issue. Also, when I create all nodes initially, all renders properly (like on schreenshot).

Will dive deeper in the core module trying to figure out what happens there...

Thanks for getLink idea!

stanislav-grin commented 4 years ago

I looked at how remove method works (in NodeModel src/entities/node/NodeModel.ts), it works fine when I call it. It sees the port I tell it to delete, it removes it and so on. Then I checked what ports object contains in NodeModel and see that there is no removed port - so it's ok here, but it still renders in view, after canvas is repainted. Got stucked here.

image

stanislav-grin commented 4 years ago

Ook, after deleting I tried to cause the target node to rerender the component using forceUpdate and it works, but I don't understand why repaint does not do it. Also Its hard to make rerender of target node because all this logic is placed in source node widget, and rerender should happen in target (another) node.

renato-bohler commented 4 years ago

Hmmm, that's a little bit of a workaround, but maybe if you request a repaint using requestAnimationFrame or setTimeout it will work? Not sure what's going on aswell =P

requestAnimationFrame(engine.repaintCanvas);
// or
setTimeout(engine.repaintCanvas);
stanislav-grin commented 4 years ago

I managed to solve both problems. For anyone who may have similar problem, my mistake was to wrap my custom NodeWidget in redux's connect decorator (in order to provide actions for removing/editing/creating ports). After I changed this so that the action was provided via serialize in node model, and using the trick above (with setTimout) everything started to work like a charm.

Thanks a lot for your help, @renato-bohler!

The issue can be closed if needed.

UPD setTimout trick may not be required, but for someone it may be helpful

renato-bohler commented 4 years ago

Sweet, nice to hear :smile:

(I don't have the permission to close the issue, though)