react-cytoscapejs icon indicating copy to clipboard operation
react-cytoscapejs copied to clipboard

Improper removal of parent nodes

Open mng12689 opened this issue 4 years ago • 0 comments

Hello! I've recently come across a bug where if nodes are first defined as having a parent node, and then in a subsequent render a) these nodes' 'parent' property are set to 'null' and b) the parent nodes themselves are removed from the 'elements' list, the Cytoscape component will remove both the parent and its children, whereas what I would expect is for the children to remain part of the graph but no longer attached to a parent. I am not very familiar with the underlying implementation, but I think what might be happening here is that the child nodes are not first moved (via eles.move()) to a null parent before the parent nodes themselves are removed (via eles.remove()), causing the unintentional deletion of the parents' child nodes. I can verify that the following implementation first removes deleted nodes before adding or patching nodes: https://github.com/plotly/react-cytoscapejs/blob/master/src/patch.js.

Here is an example implementation to illustrate the issue in a browser:

import React from 'react';
import Cytoscape from 'cytoscape';
import CytoscapeComponent from 'react-cytoscapejs';
import fcose from 'cytoscape-fcose';
Cytoscape.use(fcose);

export default class Graph extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      parents: false
    }
  }

  render() {
    const t1 = [
      { data: { id: 'a', label: 'a', parent: null }},
      { data: { id: 'b', label: 'b', parent: null }},
      { data: { id: 'c', label: 'c', parent: null }},
      { data: { id: 'ab', source: 'a', target: 'b', parent: null }},
      { data: { id: 'ac', source: 'a', target: 'c', parent: null }}
    ];
    const t2 = [
      { data: { id: 'd', label: 'd', parent: null }},
      { data: { id: 'e', label: 'e', parent: null }},
      { data: { id: 'f', label: 'f', parent: null }},
      { data: { id: 'de', source: 'd', target: 'e', parent: null }},
      { data: { id: 'df', source: 'd', target: 'f', parent: null }}
    ];

    // create parent nodes
    const p = [
      { data: { id: 'p1', label: 'p1' }},
      { data: { id: 'p2', label: 'p2' }} 
    ];

    let elem = t1.concat(t2);
    if ( this.state.parents ) {
      elem.forEach((e) => {
        if ( e.data.id < 'd' ) {
          e.data.parent = 'p1';
	} else {
          e.data.parent = 'p2';
        }
      });

      // if this.state.parents === true, add the parent nodes to the element list. 
      elem = elem.concat(p);
    }

    const layout =
      {
        name: 'fcose',
        animate: true,
        animationDuration: 500,
        randomize: false,
        quality: 'proof',
    };
    return (
      <div style={{ width: 800, height: 800, display: 'flex'}}>
        <CytoscapeComponent elements={elem}
                            style={{ display: 'flex', flex: 1, width: 800, height: 800 }}
                            layout={layout} />
        <button onClick={() => this.setState({ parents: !this.state.parents })}>
          {`Toggle, (current: ${this.state.parents})`}
        </button>
      </div>
    );
  }
}

mng12689 avatar Sep 20 '21 21:09 mng12689