cytoscape / cytoscape.js

Graph theory (network) library for visualisation and analysis
https://js.cytoscape.org
MIT License
10.09k stars 1.64k forks source link

Events not firing as expected #1283

Closed pmall closed 8 years ago

pmall commented 8 years ago

It is stated in the documentation :

Part of initialising Cytoscape.js is synchronous and part may be asynchronous. The potentially asynchronous parts are (1) loading data via promises and (2) the initial layout, which may be used for setting the initial positions of nodes. If you use an asynchronous (a.k.a. continuous) layout at initialisation or you load data via promises, you may want to use ready. You can guarantee that the initial layout takes no time if you specify all node positions manually in the elements JSON and use the preset layout — which does nothing by default.

And :

ready : A callback function (function(event){ / ... / }) that is called when Cytoscape.js has loaded the graph and the layout has specified initial positions of the nodes. After this point, rendering can happen, the user can interact with the graph, et cetera.

I generated a network with all node position specified to (0, 0) because for UI purpose I need to start the layout after nodes have been loaded (there is buttons listening to startlayout and stoplayout allowing the user to start/stop the layout run / change the layout). However if I do this :

cytoscape({
    container: container.find('.canvas').get(0),
    elements: data,
    layout: { name: 'preset' },
    ready: function (e) {

        var cy = e.cy;

        cy.on('layoutstart', function (e) {

            // Notice the layoutstart event with the layout name.
            console.log('layoutstart', e.cyTarget.options.name);

        });
        cy.on('layoutstop', function (e) {

            // Notice the layoutstop event with the layout name.
            console.log('layoutstop', e.cyTarget.options.name);

        });

        cy.layout({name: 'cose'});

    },
});

Which is outputting this :

layoutstart cose
layoutstop preset
layoutstop cose

Which completely mess up with the UI because a layout stop event is fired instantly after the cose layout started, then a second layout stop is fired.

It seems obvious it is because the ready callback is called somewhere between preset layout starts and stops. It implies the preset layout is asynchronous whereas the doc states it takes no time, which is counter intuitive.

I should mention I also tried with the done callback of the cytoscape init function with the same result. I also tried to listen to the 'done' event of cytoscape descibed as :

when a new instance of Cytoscape.js is ready to be interacted with and its initial layout has finished running

And to fire cose in this listener. Same result.

So there is no way of properly running a layout manually after the initial layout, which should take no time.

I suggest either the preset layout to be synchronous so the ready callback is run after it stops or the addition of a callback called after the initial layout stops.

I think I can workaround this by initialising my listeners in the stop callback of the preset layout, which is also very counter intuitive.

Edit : I now tried to put my code in the stop callback of the preset layout. Still the same result ! Please fire the events before the callbacks. I should not receive a stoplayout event in the stop callback of a layout, this is a bug.

maxkfranz commented 8 years ago

This is not a bug. You're misusing/misunderstanding ready. ready happens on layoutready for the init layout. Remember the flow of layout events: layoutstart, layoutready, layoutstop.

You're starting another layout before the first layout has had a chance to fire layoutstop.

Just put your code after the init line. There's no reason for you to be using the ready callback for your usecase at all.

pmall commented 8 years ago

Just to be clear, this should do the trick ?

var cy = cytoscape({
    container: container.find('.canvas').get(0),
    elements: data,
    layout: { name: 'preset' },
});

cy.on('layoutstart', function (e) {

   // Notice the layoutstart event with the layout name.
  console.log('layoutstart', e.cyTarget.options.name);

});

cy.on('layoutstop', function (e) {

  // Notice the layoutstop event with the layout name.
  console.log('layoutstop', e.cyTarget.options.name);

});

cy.layout({name: 'cose'});

If so, yes, update the docs. I started my project two years ago and still haven't figured this out.

pmall commented 8 years ago

This is now working, thanks for the explanation.