Blazor-Diagrams / Blazor.Diagrams

A fully customizable and extensible all-purpose diagrams library for Blazor
https://blazor-diagrams.zhaytam.com
MIT License
1.05k stars 213 forks source link

Link snapping does set ports instead of previewing : unwanted behaviour #166

Open rob-ack opened 2 years ago

rob-ack commented 2 years ago

Link snapping does set ports instead of previewing the new ports position. This will trigger custom behaviour when extending Blazor.Diargrams. I.e. the Links_Added BaseLinkModel.TargetPortChanged callback will be triggered.

Due to the constant adding and remving when links are moved, another case is that 'PortModel.CanAttachTo' implementations can not cleanly test if any link to the port is already attached and preven attaching like if (target.Links.Count > 0) { return false; }

The behaviour should change to only snapping but not also _ongoingLink.SetTargetPort(port); https://github.com/Blazor-Diagrams/Blazor.Diagrams/blob/ef9309c310f26a4828569c26fb8ebefa0278f83b/src/Blazor.Diagrams.Core/Behaviors/DragNewLinkBehavior.cs#L76

zHaytam commented 2 years ago

Hello,

314159265meow commented 2 years ago

Hello, we are kind of having the same problem as @rob-ack.

In our case we have a data model "class", where the storage and most of the validation is. One thing we have to do, is to inform the data model when the user added a new link between two nodes. With link snapping enabled, I can't find a way to get this information from the diagram:

So what I ended up doing is when Links.Added is triggered and the link is not attached, I store the link. Then when Diagram.MouseUp is triggered, I check if there is a stored link and that link is now attached. In that case the user is done dragging the link and has added a new link between two nodes. This works, but doesn't feel very elegant.

PortModel.CanAttachTo is hard to handle because PortModel.Links already contains the link that should be checked. E.g. a simple check to prevent the user from adding the same link twice could look like this:

public override bool CanAttachTo(PortModel port)
{
  foreach (var link in this.Links)
    if (link.TargetPort.Id == port.Id)
      return false;
}

With snapping enabled this doesn't work. My workaround here is to exclude the last link in the collection, because that is the one the user is currently dragging.

public override bool CanAttachTo(PortModel port)
{
  for (var i = 0; i < this.Links.Count - 1; i++)
    if (this.Links[i].TargetPort.Id == port.Id)
      return false;
}  
zHaytam commented 2 years ago
  1. If I understand correctly, you want to know when a link got attached after all the snaps in between right? The very final snap/attachment?
  2. I'm afraid that's how it's gonna be. Logically, when you start dragging a link from port X, that port contains the link, it makes sense programmatically and visually. In your validation, you'll just need to exclude the last link that's all.

We can't really "fake attachment" because that would require a bunch of intermediate code that would only be useful for this specific use case. But if 1. is something that concerns you, then we can maybe think of a separate event for it.

314159265meow commented 2 years ago

1) Correct, it would be very helpful to have have an event when a link is finally attached. 2) Ok, it just feels like some code that might break in the future because it relies on the inner workings of the library. If you, for whatever reason, change the order to the Links collection in the future then this might introduce a hard to find bug. But it's no big issue, the solution works fine and I understand that you don't want to add a bunch of code just to cover this specific case.

Thanks for your help.