iVis-at-Bilkent / cytoscape.js-expand-collapse

A Cytoscape.js extension to expand/collapse nodes for better management of complexity of compound graphs
MIT License
124 stars 33 forks source link

nodes.predecessors does not return all nodes when parent is collapsed #57

Closed mafar closed 6 years ago

mafar commented 6 years ago

JS Fiddle: https://jsfiddle.net/bababalcksheep/njp2jk2j/33/

  1. I am consuming node.predecessors from (http://js.cytoscape.org/#nodes.predecessors)
  2. Click on G , print @ console G Path => F,C,B,A
  3. Collapse Group-CDF then click on G , print @ console G Path => Group-CDF,B,A

What I need : Even when collapsed , how can I get complete path ? which is G Path => F,C,B,A

Problem is before Group-CDF collapse, everything works fine. After collapse, I am expecting same result as nodes are actually not removed , they are just collapsed

      cy.on('tap', function(event) {
        var evtTarget = event.target;
        var node = evtTarget;
        if (evtTarget.isNode()) {
          var predecessors = node.predecessors('node').map(function(_node) {
            return _node.data('name');
          });
          console.log(node.data('name'), ' Path => ', predecessors.join(','));
        }
      });
ugurdogrusoz commented 6 years ago

@mafar Naturally the predecessors function follows the current visible connections. Since expand-collapse is not a core operation, core level traversal functions like predecessors cannot be modified to consider these. However, one could write new traversal functions in this extension that take options to consider "only original edges" and ignore meta edges created as part of expand-collapse operations. You're welcome to implement such functions and we'd appreciate a PR if such functions are available.

mafar commented 6 years ago

cy.json() also fails to include collapsed elements. I will see if i can i can come up with solution. But should not expand-collpase , use visibility none instead of hide so that nodes are not removed ?

I will say, all of core functionaly brakes which requires iteration from cy docs because expand-collpase removes nodes , could hiding them be a solution ?

ugurdogrusoz commented 6 years ago

the whole point here is complexity management. if you just make graph contents invisible but part of the topology, then you're not reducing complexity. collapsed content is reachable (for instance see this API: getCollapsedChildren), you should continue your traversal from those nodes. sorry we don't have any resources to do this at the moment. another way to manage complexity is hide-show: https://github.com/iVis-at-Bilkent/cytoscape.js-view-utilities

mafar commented 6 years ago

Best solution i see for now is to use cy.batch() and write a wrapper function for cytoscape.js-expand-collapse

  1. demo at https://jsfiddle.net/bababalcksheep/9r60vkev/33/
  2. works with node.predecessors('node') example
  3. works with cy.json() , exports all elements even collapsed

@ugurdogrusoz does this approach seems ok ? Any suggestions to improve it? if it is then it can be integrated with cytoscape.js-expand-collapse with better name and documentation

// traversing(cy, 'json') to call cy.json()
// traversing(node, 'predecessors', 'node') to call node.predecessors('node')
 function traversing(eles, fn, selector) {
   var collection;
   // save collection of collapsedNodes so that theycan be restored
   var collapsedNodes = cy.$('.cy-expand-collapse-collapsed-node');
   // redraw once using cy.batch
   cy.batch(function() {
     expandCollapse.expand(collapsedNodes);
     collection = eles[fn](selector);
     expandCollapse.collapse(collapsedNodes); // restore collapsedNodes 
   });
   return collection;
 }
mafar commented 6 years ago

https://jsfiddle.net/bababalcksheep/9r60vkev/77/ test with 100 groups

ugurdogrusoz commented 6 years ago

@kinimesi could you please check the approach?

kinimesi commented 6 years ago

@ugurdogrusoz although this approach works for @mafar's case, it is a naive approach.. @mafar expands collapsed nodes, calls intended function and then collapses the nodes again. In its current state, this approach will not work properly when there are nested collapsed nodes. Moreover the expand operation is costly, specially when there are many nested collapsed nodes.

I do not think it is a good idea to integrate this into the extension. We normally do not expect users to call core level functions on collapsed elements (because the collapsed elements are essentially removed from the model). If users want, they can simulate @mafar's approach in 3 lines (expand, call intended functions, collapse).

@mafar: setting layout to null, fisheye and animate to false, and providing them as options to expand and collapse functions might improve the performance. You can take a look at expandRecursively and collapseRecursively methods. While these methods might help you to handle the case when there are nested collapsed nodes, it still will be complicated to get the whole list of collapsedNodes.

mafar commented 6 years ago

I understand this is ugly approach But i see no other way without modifying source code as I need to use core calls as per my app.

@ugurdogrusoz Please close it if there is no alternative approach