What is the correct way to remove jsPlumb's artifacts when deleting their container?

[Jan van der Ven]

Hi all,


I have a div (called block) that acts as the container for the jsPlumb targets and sources. The connectors are attached to td elements that have class mu-is and mu-os. One is for targets, the other is for sources. The user can delete the container. When that happens, I execute the following code:

function deleteBlock(selectedBlock) {
    if (confirm("Remove " + selectedBlock + "?")) {
        $('#' + selectedBlock).find('.mu-os').each(function(){
            jsPlumb.detachAllConnections($(this));
            jsPlumb.unmakeSource($(this), false);
        });
        $('#' + selectedBlock).find('.mu-is').each(function () {
            jsPlumb.detachAllConnections($(this));
            jsPlumb.unmakeTarget($(this), false);
        });
        $('#' + selectedBlock).remove();
    }
}

This removes any connections on the block and leaves no connectors behind as well. All looks well.
However the blocks are draggable. And when I start to drag the block that was connected to the other block or another block that was not connected at all, the following message appears in the log:

TypeError: o is undefined

return {left:o.left / z, top:o.top / z };

Which is line 538 of jsPlumb 1.3.16-all.js.

If I comment out $('#' + selectedBlock).remove(); the block stays on screen of course, but dragging is still possible.

I think there is some more cleanup to do. I just don't know what or where.


Please advise.


Kind regards,


Jan

[Simon Porritt]

i guess it's possible there's a bug here, since it looks like it is trying to paint connections to an element that was removed, but it's hard to say without being able to step through some code.

in 1.4.0 I have added a "remove" function that removes all endpoints and connections for some node and then removes the node from the dom, which should help with situations like this.

[Jan van der Ven]

Dear Simon,


I looked a bit deeper. I have code that calls repaintEverything when dragging. It turns out it called the _draw with an element it tried to retrieve from the deleted id. So the element parameter was an empty array:
        _draw = function (element, ui, timestamp) {
            if (element === null || element.length === 0) {
                // this is the workaround
                console.log('draw request for empty element');
                return;
            }
I log this to see if a potential fix would work.

It seems the thing is still present in the endpointsByElement array:
        this.repaintEverything = function() {               
            for (var elId in endpointsByElement) {
                _draw(_getElementObject(elId));
            }
            return _currentInstance;
        };

Consequently I changed unmakeTarget and unmakeSource, and added:
             if (!doNotClearArrays) {
                delete _targetEndpointDefinitions[elid];
                delete _targetEndpointsUnique[elid];
                delete _targetMaxConnections[elid];
                delete _targetsEnabled[elid];
               
                delete endpointsByElement[id]; // this line
            }

The rest of the delete code is as in my first post.

My application works without errors with these changes. No 'draw request for empty element' in the log.

Is this is valid solution? Or did I break something significant?


Kind regards,


Jan

[Simon Porritt]

Hello

That looks pretty good to me.  It explains the issue you were seeing and fixes it - I'm going to make this change in 1.4.0.

thanks!

[Simon Porritt]

oh hang on...no maybe that goes against the intent of the unmakeSource/unmakeTarget functions.  their purpose is to stop elements from behaving as sources or targets, but they are not requests to remove existing endpoints.

things get a little complicated with detachAllConnections, as when a connection is detached it optionally has a list of endpoints that should be detached with it. off the top of my head i feel like any endpoints that are created via makeTarget/makeSource fall into this category, so each detachAllConnections call is causing some endpoints to be deleted.  the other thing that happens is that after a connection is detached a repaint is done, and here's where i suspect the error is occurring: a repaint is trying to draw an element that has gone.

so i kind of like the safety code in the draw function, it seems like a good idea in general.  as for the repaint issues, it might perhaps be worthwhile trying this

jsPlumb.setSuspendDrawing(true);

// do your deletions

jsPlumb.setSuspendDrawing(false, true);

...that function does what the name suggests. the 'true' parameter in the second call says "and do a full repaint after you change this setting".

could you try that and let me know what happens (without the check in _draw...) ?

--
Simon Porritt

+61 (0) 459 762 779

[Jan van der Ven]

Dear Simon,


That can't be it: the error occurs when I start to drag one of the remaining containers around. And not during the delete operation.

I'll have a look at detachAllConnections.

The stack trace is deleteBlock (mine) -- detachAllConnections -- detachAll -- detach -- deleteEndpoint.

In deleteEndpoint a new array for endpointsByElement[e] is constructed. I added the code that deletes the element if the newEndpoints array is empty.

         this.deleteEndpoint = function(object) {
            var endpoint = (typeof object == "string") ? endpointsByUUID[object] : object;           
            if (endpoint) {                   
                var uuid = endpoint.getUuid();
                if (uuid) endpointsByUUID[uuid] = null;               
                endpoint.detachAll();
                if (endpoint.endpoint.cleanup) endpoint.endpoint.cleanup();
                _removeElements(endpoint.endpoint.getDisplayElements());
                _currentInstance.anchorManager.deleteEndpoint(endpoint);
                for (var e in endpointsByElement) {
                    var endpoints = endpointsByElement[e];
                    if (endpoints) {
                        var newEndpoints = [];
                        for (var i = 0; i < endpoints.length; i++)
                            if (endpoints[i] != endpoint) newEndpoints.push(endpoints[i]);

                        if (newEndpoints.length === 0) {
                            // remove the element altogether
                            delete endpointsByElement[e];
                        } else {
                            endpointsByElement[e] = newEndpoints;
                        }
                    }
                }
                if (!jsPlumbAdapter.headless)
                    _currentInstance.dragManager.endpointDeleted(endpoint);                               
            }                                   
        };

This seems to do the trick on a more appropriate place.

Kind regards,


Jan

[Pat]

I've been lightly following along on this thread, but I'm a little unsure of the conclusion. Simon and/or Jan, for the benefit of other users could you summarize what we should know about correctly removing jsplumb artifacts? Thanks so much,

Pat

[Jan van der Ven]

Dear Pat,


I can only tell you about my own scenario.

My endpoints are created like this:
    jsPlumb.makeSources(outputSockets, sourceEndPoints[0]);
    jsPlumb.makeTargets(inputSockets, targetEndPoint);
Where in- and outputSockets are arrays of div, and the source- and targetEndPoint are defined as:
            var sourceEndPoint = {
                isSource: true
                , maxConnections: 10
                , dropOptions: {
                    tolerance: "pointer"
                    , hoverClass: "dropHover"
                }
                , endpoint: ["Dot", { radius: 3 }]
                , hoverPaintStyle: connectorHoverStyle
                , connectorHoverStyle: connectorHoverStyle
            };
            sourceEndPoints[0] = sourceEndPoint;
            targetEndPoint = {
                isTarget: true
                , endpoint: ["Dot", { radius: 3 }]
            };

I let the user create the connections. And let the user delete the container (the block). When that happens I call (mu-os and mu-is are the class names of the divs that hold the source and target):

    $('#' + selectedBlock).find('.mu-os').each(function () {
        jsPlumb.detachAllConnections($(this));
        jsPlumb.unmakeSource($(this), false);
    });
    $('#' + selectedBlock).find('.mu-is').each(function () {
        jsPlumb.detachAllConnections($(this));
        jsPlumb.unmakeTarget($(this), false);
    });
    $('#' + selectedBlock).remove();

That works with just one change in the deleteEndpoint function, as described above:

                        if (newEndpoints.length === 0) {
                            // remove the element altogether
                            delete endpointsByElement[e];
                        } else {
                            endpointsByElement[e] = newEndpoints;
                        }

I also allow the user to start fresh. At first I did $('#drawing').empty(); but that failed the second time. Then I changed to loop over all 'blocks' and call my own delete function, and that worked (I left the empty() statement in, because I am paranoid).

Hope this helps.

Kind regards,


Jan