Fix usage of clustering with hierarchical networks (#3132)

* clusters in hierarchical working for sortorder 'hubsize'

* Code cleanup

lib/network/Network.js

@@ -55,13 +55,27 @@ function Network(container, data, options) {
   };
   util.extend(this.options, this.defaultOptions);
 
-  // containers for nodes and edges
+  /**
+   * Containers for nodes and edges.
+   *
+   * 'edges' and 'nodes' contain the full definitions of all the network elements.
+   * 'nodeIndices' and 'edgeIndices' contain the id's of the active elements.
+   *
+   * The distinction is important, because a defined node need not be active, i.e.
+   * visible on the canvas. This happens in particular when clusters are defined, in
+   * that case there will be nodes and edges not displayed.
+   * The bottom line is that all code with actions related to visibility, *must* use
+   * 'nodeIndices' and 'edgeIndices', not 'nodes' and 'edges' directly.
+   */
   this.body = {
     container: container,
+
+    // See comment above for following fields
     nodes: {},
     nodeIndices: [],
     edges: {},
     edgeIndices: [],
+
     emitter: {
       on:   this.on.bind(this),
       off:  this.off.bind(this),

lib/network/modules/LayoutEngine.js

@@ -1194,50 +1194,78 @@ class LayoutEngine {
 
 
   /**
-   * Get the hubsize from all remaining unlevelled nodes.
+   * Return the active (i.e. visible) edges for this node
    *
-   * @returns {number}
+   * @returns {array} Array of edge instances
    * @private
    */
-  _getHubSize() {
-    let hubSize = 0;
-    for (let nodeId in this.body.nodes) {
-      if (this.body.nodes.hasOwnProperty(nodeId)) {
-        let node = this.body.nodes[nodeId];
-        if (this.hierarchical.levels[nodeId] === undefined) {
-          hubSize = node.edges.length < hubSize ? hubSize : node.edges.length;
-        }
+  _getActiveEdges(node) {
+    let result = [];
+
+    for (let j in node.edges) {
+      let edge = node.edges[j];
+      if (this.body.edgeIndices.indexOf(edge.id) !== -1) {
+        result.push(edge);
       }
     }
-    return hubSize;
+
+    return result;
+  }
+
+
+  /**
+   * Get the hubsizes for all active nodes.
+   *
+   * @returns {number}
+   * @private
+   */
+  _getHubSizes() {
+    let hubSizes = {};
+    let nodeIds = this.body.nodeIndices;
+
+    for (let i in nodeIds) {
+      let nodeId = nodeIds[i];
+      let node = this.body.nodes[nodeId];
+      let hubSize = this._getActiveEdges(node).length;
+      hubSizes[hubSize] = true;
+    }
+
+    // Make an array of the size sorted descending
+    let result = [];
+    for (let size in hubSizes) {
+      result.push(Number(size));
+    }
+    result.sort(function(a, b) {
+      return b - a;
+    });
+
+    return result;
   }
 
 
   /**
    * this function allocates nodes in levels based on the recursive branching from the largest hubs.
    *
-   * @param hubsize
    * @private
    */
   _determineLevelsByHubsize() {
-    let hubSize = 1;
-
     let levelDownstream = (nodeA, nodeB) => {
       this.hierarchical.levelDownstream(nodeA, nodeB);
     }
 
-    while (hubSize > 0) {
-      // determine hubs
-      hubSize = this._getHubSize();
-      if (hubSize === 0)
-        break;
+    let hubSizes = this._getHubSizes();
 
-      for (let nodeId in this.body.nodes) {
-        if (this.body.nodes.hasOwnProperty(nodeId)) {
-          let node = this.body.nodes[nodeId];
-          if (node.edges.length === hubSize) {
-            this._crawlNetwork(levelDownstream,nodeId);
-          }
+    for (let i = 0; i < hubSizes.length; ++i ) {
+      let hubSize = hubSizes[i];
+      if (hubSize === 0) break;
+
+      let nodeIds = this.body.nodeIndices;
+      for (let j in nodeIds) {
+        let nodeId = nodeIds[j];
+        let node = this.body.nodes[nodeId];
+
+        if (hubSize === this._getActiveEdges(node).length) {
+          this._crawlNetwork(levelDownstream, nodeId);
         }
       }
     }
@@ -1322,7 +1350,7 @@ class LayoutEngine {
 
   /**
    * Crawl over the entire network and use a callback on each node couple that is connected to each other.
-   * @param callback          | will receive nodeA nodeB and the connecting edge. A and B are unique.
+   * @param callback          | will receive nodeA, nodeB and the connecting edge. A and B are distinct.
    * @param startingNodeId
    * @private
    */
@@ -1340,18 +1368,19 @@ class LayoutEngine {
 
         progress[node.id] = true;
         let childNode;
-        for (let i = 0; i < node.edges.length; i++) {
-          let edges = node.edges[i];
-          if (edges.connected === true) {
-            if (edges.toId === node.id) {
-              childNode = edges.from;
+        let edges = this._getActiveEdges(node);
+        for (let i = 0; i < edges.length; i++) {
+          let edge = edges[i];
+          if (edge.connected === true) {
+            if (edge.toId == node.id) {         // '==' because id's can be string and numeric
+              childNode = edge.from;
             }
             else {
-              childNode = edges.to;
+              childNode = edge.to;
             }
 
-            if (node.id !== childNode.id) {
-              callback(node, childNode, edges);
+            if (node.id != childNode.id) {      // '!=' because id's can be string and numeric
+              callback(node, childNode, edge);
               crawler(childNode, tree);
             }
           }