Fix/not call graph configs inside component did update (#10)

danielcaldas · web-flow · commit bab2c93bd62b · 2017-08-05T00:33:51.000+01:00
* Add fix me note. Handle this as a 🐛

* Update README

* Amazing fix ⭐. Pause simulation to update graph

* Improve condition for zoom config updates

* Rename utils method isEqual to isDeepEqual

* Separate node update logic from config update logic

* Handle static graph data updates

* Absorve node/links positioning when receiving new props

* Error detection for invalid links. Count nodes and links in sandbox

* Update sandbox
diff --git a/README.md b/README.md
@@ -83,22 +83,6 @@ const onClickLink = function(source, target) {
      onMouseOutNode={onMouseOutNode} />
 ```
 
-## TODOs
-This consists in a list of ideas for further developments:
-- Expose a graph property **background-color** that is applied to the svg graph container;
-- Expose d3-force values as configurable such as **alphaTarget** simulation value;
-- Improve opacity/highlightBehavior strategy maybe use a global *background: rgba(...)* value and then set a higher
-value on selected nodes;
-- At the moment highlightBehavior is highlighting the mouse hovered node, its 1st degree connections and their 1st
-degree connections. Make **highlightBehaviorDegree** which consists in a *step value* on the depth that we wish to highlight;
-- Semantic node size. Have a property value in each node that then is used along side config.nodeSize property
-to calculate effective node size in run time;
-- Improve semanticStrokeWidth calculation;
-- On Graph instantiation do a check on all config properties. If there is a "bad property" (name or value) throw
-a custom error (property error checking);
-- Path highlight - highlight a certain set of links and nodes (use case: highlight shortest path between two given nodes);
-- Link mouseover with highlight behavior highlights the intervenient nodes.
-
 ## Contributions
 Contributions are welcome fell free to submit new features or simply grab something from
 the above TODO list.
diff --git a/sandbox/Sandbox.js b/sandbox/Sandbox.js
@@ -86,11 +86,14 @@ export default class Sandbox extends React.Component {
             });
         } else {
             // 1st node
-            this.setState({
+            const data = {
                 nodes: [
                     {id: 'Node 1'}
-                ]
-            });
+                ],
+                links: []
+            };
+
+            this.setState({ data });
         }
     }
 
@@ -100,13 +103,11 @@ export default class Sandbox extends React.Component {
     onClickRemoveNode = () => {
         if (this.state.data.nodes && this.state.data.nodes.length) {
             const id = this.state.data.nodes[0].id;
-            const nodes = this.state.data.nodes.splice(0, 1);
+            this.state.data.nodes.splice(0, 1);
             const links = this.state.data.links.filter(l => l.source !== id && l.target !== id);
+            const data = { nodes: this.state.data.nodes, links };
 
-            this.setState({
-                nodes,
-                links
-            });
+            this.setState({ data });
         } else {
             alert('No more nodes to remove!');
         }
@@ -199,6 +200,7 @@ export default class Sandbox extends React.Component {
                     <button onClick={this.resetNodesPositions} className='btn btn-default' style={btnStyle} disabled={this.state.config.staticGraph}>Unstick nodes</button>
                     <button onClick={this.onClickAddNode} className='btn btn-default' style={btnStyle}>+</button>
                     <button onClick={this.onClickRemoveNode} className='btn btn-default' style={btnStyle}>-</button>
+                    <br/><b>Nodes: </b> {this.state.data.nodes.length}, <b>Links: </b> {this.state.data.links.length}
                     <Graph ref='graph' {...graphProps}/>
                 </div>
                 <div className='container__form'>
@@ -228,7 +230,7 @@ export default class Sandbox extends React.Component {
 
 class JSONContainer extends React.Component {
     shouldComponentUpdate(nextProps, nextState) {
-        return !this.props.staticData && !ReactD3GraphUtils.isEqual(nextProps.data, this.props.data);
+        return !this.props.staticData && !ReactD3GraphUtils.isDeepEqual(nextProps.data, this.props.data);
     }
 
     render() {
diff --git a/src/components/Graph/index.js b/src/components/Graph/index.js
@@ -201,6 +201,21 @@ export default class Graph extends React.Component {
      */
     restartSimulation = () => !this.state.config.staticGraph && this.state.simulation.restart();
 
+    /**
+     * Some integraty validations on links and nodes structure.
+     * @param  {Object} data
+     */
+    _validateGraphData(data) {
+        data.links.forEach(l => {
+            if (!data.nodes.find(n => n.id === l.source)) {
+                Utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.source} is not a valid node id`);
+            }
+            if (!data.nodes.find(n => n.id === l.target)) {
+                Utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.target} is not a valid node id`);
+            }
+        });
+    }
+
     /**
      * Incapsulates common procedures to initialize graph.
      * @param  {Object} data
@@ -209,14 +224,29 @@ export default class Graph extends React.Component {
      * @returns {Object}
      */
     _initializeGraphState(data) {
-        let graph = data || {};
+        this._validateGraphData(data);
+
+        let graph;
+
+        if (this.state && this.state.nodes && this.state.links && this.state.nodeIndexMapping) {
+            // absorve existent positining
+            graph = {
+                nodes: data.nodes.map(n => Object.assign({}, n, this.state.nodes[n.id])),
+                links: {}
+            };
+        } else {
+            graph = {
+                nodes: data.nodes.map(n => Object.assign({}, n)),
+                links: {}
+            };
+        }
 
-        let config = Utils.merge(DEFAULT_CONFIG, this.props.config || {});
+        graph.links = data.links.map(l => Object.assign({}, l));
 
+        let config = Object.assign({}, Utils.merge(DEFAULT_CONFIG, this.props.config || {}));
         let {nodes, nodeIndexMapping} = GraphHelper.initializeNodes(graph.nodes);
         let links = GraphHelper.initializeLinks(graph.links); // Matrix of graph connections
         const {nodes: d3Nodes, links: d3Links} = graph;
-
         const id = this.props.id.replace(/ /g, '_');
         const simulation = GraphHelper.createForceSimulation(config.width, config.height);
 
@@ -230,7 +260,8 @@ export default class Graph extends React.Component {
             d3Nodes,
             nodeHighlighted: false,
             simulation,
-            graphDataChanged: false
+            newGraphElements: false,
+            configUpdated: false
         };
     }
 
@@ -259,33 +290,48 @@ export default class Graph extends React.Component {
         super(props);
 
         if (!this.props.id) {
-            throw Utils.throwErr(this.constructor.name, ERRORS.GRAPH_NO_ID_PROP);
+            Utils.throwErr(this.constructor.name, ERRORS.GRAPH_NO_ID_PROP);
         }
 
         this.state = this._initializeGraphState(this.props.data);
     }
 
     componentWillReceiveProps(nextProps) {
-        const state = this._initializeGraphState(nextProps.data);
+        const newGraphElements = nextProps.data.nodes.length !== this.state.d3Nodes.length
+                              || nextProps.data.links.length !== this.state.d3Links.length;
+
+        if (newGraphElements && nextProps.config.staticGraph) {
+            Utils.throwErr(this.constructor.name, ERRORS.STATIC_GRAPH_DATA_UPDATE);
+        }
+
+        const configUpdated = Utils.isDeepEqual(nextProps.config, this.state.config);
+        const state = newGraphElements ? this._initializeGraphState(nextProps.data) : this.state;
         const config = Utils.merge(DEFAULT_CONFIG, nextProps.config || {});
 
+        // In order to properly update graph data we need to pause eventual d3 ongoing animations
+        newGraphElements && this.pauseSimulation();
+
         this.setState({
             ...state,
             config,
-            graphDataChanged: true
+            newGraphElements,
+            configUpdated
         });
     }
 
     componentDidUpdate() {
-        // If some zoom config changed we want to apply possible new values for maxZoom and minZoom
-        this._zoomConfig();
-
         // If the property staticGraph was activated we want to stop possible ongoing simulation
         this.state.config.staticGraph && this.state.simulation.stop();
 
-        if (!this.state.config.staticGraph && this.state.graphDataChanged) {
+        if (!this.state.config.staticGraph && this.state.newGraphElements) {
             this._graphForcesConfig();
-            this.state.graphDataChanged = false;
+            this.restartSimulation();
+            this.state.newGraphElements = false;
+        }
+
+        if (this.state.configUpdated) {
+            this._zoomConfig();
+            this.state.configUpdated = false;
         }
     }
 
diff --git a/src/err.js b/src/err.js
@@ -1,3 +1,7 @@
 export default {
-    GRAPH_NO_ID_PROP: 'id prop not defined! id property is mandatory and it should be unique.'
+    GRAPH_NO_ID_PROP: "id prop not defined! id property is mandatory and it should be unique.",
+    STATIC_GRAPH_DATA_UPDATE: "a static graph cannot receive new data (nodes or links).\
+    Make sure config.staticGraph is set to true if you want to update graph data",
+    INVALID_LINKS: "you provided a invalid links data structure.\
+    Links source and target attributes must point to an existent node"
 };
diff --git a/src/utils.js b/src/utils.js
@@ -5,7 +5,7 @@
  * that are common across rd3g such as error logging.
  */
 
-// This variable assures that recursive methods such as merge and isEqual do not fall on
+// This variable assures that recursive methods such as merge and isDeepEqual do not fall on
 // circular JSON structure evaluation.
 const MAX_DEPTH = 5;
 
@@ -28,7 +28,7 @@ function _isPropertyNestedObject(o, k) {
  * @memberof utils
  * @return {boolean} returns true if o1 and o2 have exactly the same content, or are exactly the same object reference.
  */
-function isEqual(o1, o2, _depth=0) {
+function isDeepEqual(o1, o2, _depth=0) {
     let diffs = [];
 
     if (_depth === 0 && o1 === o2) {
@@ -43,7 +43,7 @@ function isEqual(o1, o2, _depth=0) {
         const nestedO = _isPropertyNestedObject(o1, k) && _isPropertyNestedObject(o2, k);
 
         if (nestedO && _depth < MAX_DEPTH) {
-            diffs.push(isEqual(o1[k], o2[k], _depth + 1));
+            diffs.push(isDeepEqual(o1[k], o2[k], _depth + 1));
         } else {
             const r = isObjectEmpty(o1[k]) && isObjectEmpty(o2[k]) || o2.hasOwnProperty(k) && o2[k] === o1[k];
 
@@ -70,6 +70,7 @@ function isObjectEmpty(o) {
 }
 
 /**
+ * @TODO: Support for arrays?
  * This function merges two objects o1 and o2, where o2 properties override existent o1 properties, and
  * if o2 doesn't posses some o1 property the function will fallback to the o1 property.
  * @param  {Object} o1 - object.
@@ -109,7 +110,7 @@ function throwErr(component, msg) {
 }
 
 export default {
-    isEqual,
+    isDeepEqual,
     isObjectEmpty,
     merge,
     throwErr
diff --git a/test/utils.test.js b/test/utils.test.js
@@ -46,7 +46,7 @@ describe('Utils', () => {
         expect(Utils.isObjectEmpty({})).toEqual(true);
     });
 
-    describe('isEqual', () => {
+    describe('isDeepEqual', () => {
         let that = {};
 
         beforeEach(() => {
@@ -90,63 +90,63 @@ describe('Utils', () => {
         });
 
         test('should return true if no modifications are performed', () => {
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(true);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(true);
         });
 
         test('should return false when o2 is an empty object but o1 is not', () => {
-            expect(Utils.isEqual(that.o1, {})).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, {})).toEqual(false);
         });
 
         test('should return false when o1 is an empty object but o2 is not', () => {
-            expect(Utils.isEqual({}, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual({}, that.o2)).toEqual(false);
         });
 
         test('should return true when both objects are empty', () => {
-            expect(Utils.isEqual({}, {})).toEqual(true);
+            expect(Utils.isDeepEqual({}, {})).toEqual(true);
         });
 
         test('should return false when: o2.b.j.k = undefined', () => {
             that.o2.b.j.k = undefined;
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return false when: o2.b.j.m.n.o = "1"', () => {
             that.o2.b.j.m.n.o = '1';
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return false when: o2.b.c.e = "tests"', () => {
             that.o2.b.c.e = 'tests';
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return false when: o1.b.i = false', () => {
             that.o1.b.i = false;
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return false when: o1.a = false', () => {
             that.o1.a = false;
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return false when: o2.b.g = "tests"', () => {
             that.o2.b.g = 'tests';
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
 
         test('should return true when: o1.b.g = o2.b and o2.b.g = o2.b (circular structure)', () => {
-            // isEqual will evaluate until certain depth
+            // isDeepEqual will evaluate until certain depth
             // but since objects still the same we expect the result to be true
             that.o1.b.g = that.o2.b;
             that.o2.b.g = that.o2.b;
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(true);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(true);
         });
 
         test('should return true when: o1.b.g = o2.b and o2.b.g = o2 (circular structure)', () => {
             that.o1.b.g = that.o2.b;
             that.o2.b.g = that.o2;
-            expect(Utils.isEqual(that.o1, that.o2)).toEqual(false);
+            expect(Utils.isDeepEqual(that.o1, that.o2)).toEqual(false);
         });
     });
 });
