jsplumb / community-edition

The community edition of jsPlumb, versions 1.x - 6.x
https://jsplumbtoolkit.com
Other
224 stars 18 forks source link

Connections not updating position - (detach, delete, readd, reconnect) #946

Closed TACIXAT closed 9 years ago

TACIXAT commented 9 years ago

http://jsfiddle.net/w5umw693/23/

You'll have to paste in 1.7.0+ into jsfiddle, I couldn't find a hosted link to the newer versions to include. It won't let me update with that much Javascript, but it works if you paste it in and hit run.

This works fine in with older versions. The bug is basically:

- connect elements
- detach elements
- delete an element
- add back same element + some extra so positions change
- reconnect

The connection will be for the old positions if you're using 1.7.0+.

Thanks!

sporritt commented 9 years ago

here's an update of that fiddle that links to 1.7.2 on jsplumb.org:

http://jsfiddle.net/w5umw693/24/

sporritt commented 9 years ago

this is not what is causing your issue, but in calls like this you're not actually using the instance:

jsPlumb.connect(struct, instance)

you should execute the method directly on the instance:

instance.connect(struct)

TACIXAT commented 9 years ago

Thanks, I'll change that! I ended up hollowing it out since it didn't seem be having any effect. Makes sense now. That's what I get for copying code from StackOverflow questions.

sporritt commented 9 years ago

ha ha

good old stack overflow.

so there's a few things going on here. Primarily you need to use the remove method on the jsPlumb instance when you take something out of the DOM, as it's the hook jsPlumb needs to properly deregister everything. I have done that in this fiddle (which links to the latest 1.7.3 dev build):

http://jsfiddle.net/262cevx5/6/

The code looks like this:

$('#empty').click(function() {
        //$('#ul_one').empty();

        // use the `remove` method to properly deregister an element from the jsplumb instance.  this method removes all endpoints and connections too, so the detach call would be unecessary.        
        instance.remove("li_two");
        instance.remove("li_one");                
    });

note it doesnt fully work yet! the target is still positioned incorrectly. more on that below.

now obviously there's one key difference between $.fn.empty and jsPlumb.remove, and that is that empty removes all the child nodes of some element whereas remove removes single elements. In this use case, clearing out a list would be tedious with what jsPlumb offers, and it's possibly an argument that I should add an empty method.

i did find a bug when looking at this, too, so that's why it's pointing at the latest dev build.

as for the target, its position is not updated because it needs to be revalidated, as in this fiddle:

http://jsfiddle.net/262cevx5/7/

$('#repopulate').click(function() {
        var listItem = $('<li></li>');   
        listItem.attr('id', 'li_one');
        listItem.text('hello!');
        $('#ul_one').append(listItem);

        listItem = $('<li></li>');   
        listItem.attr('id', 'li_two');
        listItem.text('right!');
        $('#ul_one').append(listItem);

        instance.revalidate("li_four");

    });

That one works as you want it to. With an empty call on jsPlumb it would be better. The requirement to call revalidate niggles me a bit, but I always have to try to tread the fine line between making a simple API and having jsPlumb do too much. I'd be interested to hear your thoughts.

TACIXAT commented 9 years ago

Duplicating empty isn't so difficult in my code.

// something like this
var children = $(parent).children();
$(children).each(funcntion() {
    instance.remove($(this).attr('id'));
});

The revalidate is an interesting issue. I already have connect wrapped in a function that does some validation (check for existing connection, make sure source and target exist, etc.). So I think I can simply insert a revalidate call for both elements into that function. That way they're revalidated whether or not they need it.

Thanks a ton for all the help!

sporritt commented 9 years ago

No problem.

That code would empty out nodes that were direct children but anything nested any deeper than the first level would be lost...which is fine in your setup of course but if I were to add an empty method I'd have to get a bit more involved with the DOM I think.

Incidentally, the remove method takes care of detaching connections for you. On this fiddle you can skip pressing the 'Detach' button and you get the same results:

http://jsfiddle.net/262cevx5/7/

sporritt commented 9 years ago

i couldn't help myself; the idea of an empty method in jsPlumb seemed too good.

so this version of the fiddle uses it:

http://jsfiddle.net/262cevx5/9/

TACIXAT commented 9 years ago

Haha, sick as! I'll grab it tomorrow.

sporritt commented 9 years ago

closing this out as 1.7.3 is being released.