plotly / react-cytoscapejs

React component for Cytoscape.js network visualisations
MIT License
470 stars 69 forks source link

Improper removal of parent nodes #87

Open mng12689 opened 2 years ago

mng12689 commented 2 years ago

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>
    );
  }
}