Blazor-Diagrams / Blazor.Diagrams

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

Event handler removal for the Diagram doesn't work #327

Closed mrakgr closed 1 year ago

mrakgr commented 1 year ago

Repo: https://github.com/mrakgr/helix/blob/b4ff8b87095ce370660101c39dc993bb11760701/Helix.Fun/Helix.Nodes.fs#L140

type HelixDiagramBase(Js : IJSRuntime, opts) as this =
    inherit Diagram(opts)

    let redo = Stack()
    let undo = Stack()

    let mutable is_loaded = false

    let handler_node_add = Action<_>(fun x ->
        printfn "Adding node"
        undo.Push(U_NodeAdded x); redo.Clear())
    let handler_node_remove = Action<_>(fun x ->
        printfn "Removing node"
        undo.Push(U_NodeRemoved x); redo.Clear())
    let handler_link_add = Action<_>(fun x ->
        printfn "Adding link"
        undo.Push(U_LinkAdded x); redo.Clear())
    let handler_link_remove = Action<_>(fun x ->
        printfn "Removing link"
        undo.Push(U_LinkRemoved x); redo.Clear())

    let add_layer_handlers () =
        printfn "Adding handlers"
        this.Nodes.add_Added handler_node_add; this.Nodes.add_Removed handler_node_remove
        this.Links.add_Added handler_link_add; this.Links.add_Removed handler_link_remove

    let remove_layer_handlers () =
        printfn "Removing handlers"
        this.Nodes.remove_Added handler_node_add; this.Nodes.remove_Removed handler_node_remove
        this.Links.remove_Added handler_link_add; this.Links.remove_Removed handler_link_remove

    do add_layer_handlers ()
    do remove_layer_handlers ()

I am working on undo/redo functionality so I thought of suppressing the event firings by removing the handlers, but it doesn't seem like removing them here does anything, as they still get triggered. Due to removal not working, I've run into issues where they get added multiple times.

Anyway, I've tested it and it seems they get fired right away instead of being queued, so that will allow me to use a mutable variable as a switch to suppress them, instead of removing them like this.

zHaytam commented 1 year ago

I'm afraid I have no idea what that code does (I never did F#)... What do you mean by Handler removal?

mrakgr commented 1 year ago

For example...

this.Nodes.remove_Added handler_node_add

This in F# would be equivalent to...

this.Nodes.Added -= handler_node_add;

...in C#. The C# language uses the += and -= operators, but the Added event gets compiled down to add_Added and remove_Added, and this is how we can access them from F#. For some reason the removal is not working as it should.

zHaytam commented 1 year ago

I'm not sure this would be an issue of the library then no? It works fine in C#

mrakgr commented 1 year ago

Yeah, it works fine in C#, and on a toy example in F#. I am completely perplexed as to why it doesn't on the example above. Maybe some compiler optimization is causing the references to get inlined? Anyway, it is not the fault of the library.