Merge pull request #679 from weaveworks/375-oneline

Mitigate one-line-of-nodes layouts

client/app/scripts/charts/__tests__/node-layout-test.js

@@ -1,4 +1,5 @@
 jest.dontMock('../nodes-layout');
+jest.dontMock('../topology-utils');
 jest.dontMock('../../constants/naming'); // edge naming: 'source-target'
 
 import { fromJS, Map } from 'immutable';
@@ -69,6 +70,26 @@ describe('NodesLayout', () => {
       edges: fromJS({
         'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
       })
+    },
+    single3: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'}
+      }),
+      edges: fromJS({})
+    },
+    singlePortrait: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'},
+        n5: {id: 'n5'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
     }
   };
 
@@ -232,4 +253,37 @@ describe('NodesLayout', () => {
     expect(resultCoords.slice(0, 2)).toEqual(coords.slice(0, 2));
     expect(resultCoords.slice(2, 6)).toEqual(coords.slice(4, 8));
   });
+
+  it('renders single nodes in a square', () => {
+    const result = NodesLayout.doLayout(
+      nodeSets.single3.nodes,
+      nodeSets.single3.edges);
+
+    nodes = result.nodes.toJS();
+
+    expect(nodes.n1.x).toEqual(nodes.n3.x);
+    expect(nodes.n1.y).toEqual(nodes.n2.y);
+    expect(nodes.n1.x).toBeLessThan(nodes.n2.x);
+    expect(nodes.n1.y).toBeLessThan(nodes.n3.y);
+  });
+
+  it('renders single nodes next to portrait graph', () => {
+    const result = NodesLayout.doLayout(
+      nodeSets.singlePortrait.nodes,
+      nodeSets.singlePortrait.edges);
+
+    nodes = result.nodes.toJS();
+
+    // first square row on same level as top-most other node
+    expect(nodes.n1.y).toEqual(nodes.n2.y);
+    expect(nodes.n1.y).toEqual(nodes.n3.y);
+    expect(nodes.n4.y).toEqual(nodes.n5.y);
+
+    // all singles right to other nodes
+    expect(nodes.n1.x).toEqual(nodes.n4.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n2.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n3.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n5.x);
+    expect(nodes.n2.x).toEqual(nodes.n5.x);
+  });
 });

client/app/scripts/charts/__tests__/topology-utils-test.js

@@ -0,0 +1,109 @@
+jest.dontMock('../topology-utils');
+jest.dontMock('../../constants/naming'); // edge naming: 'source-target'
+
+import { fromJS } from 'immutable';
+
+describe('TopologyUtils', () => {
+  let TopologyUtils;
+  let nodes;
+
+  const nodeSets = {
+    initial4: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'},
+        'n2-n4': {id: 'n2-n4', source: 'n2', target: 'n4'}
+      })
+    },
+    removeEdge24: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    removeNode2: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    removeNode23: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    single3: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'}
+      }),
+      edges: fromJS({})
+    },
+    singlePortrait: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'},
+        n5: {id: 'n5'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    }
+  };
+
+  beforeEach(() => {
+    TopologyUtils = require('../topology-utils');
+  });
+
+  it('sets node degrees', () => {
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.initial4.nodes,
+      nodeSets.initial4.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(2);
+    expect(nodes.n2.degree).toEqual(1);
+    expect(nodes.n3.degree).toEqual(1);
+    expect(nodes.n4.degree).toEqual(2);
+
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.removeEdge24.nodes,
+      nodeSets.removeEdge24.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(2);
+    expect(nodes.n2.degree).toEqual(0);
+    expect(nodes.n3.degree).toEqual(1);
+    expect(nodes.n4.degree).toEqual(1);
+
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.single3.nodes,
+      nodeSets.single3.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(0);
+    expect(nodes.n2.degree).toEqual(0);
+    expect(nodes.n3.degree).toEqual(0);
+  });
+});

client/app/scripts/charts/nodes-layout.js

@@ -2,10 +2,18 @@ const dagre = require('dagre');
 const debug = require('debug')('scope:nodes-layout');
 const makeMap = require('immutable').Map;
 const ImmSet = require('immutable').Set;
+
 const Naming = require('../constants/naming');
+const TopologyUtils = require('./topology-utils');
 
 const MAX_NODES = 100;
 const topologyCaches = {};
+const DEFAULT_WIDTH = 800;
+const DEFAULT_MARGINS = {top: 0, left: 0};
+const DEFAULT_SCALE = val => val * 2;
+const NODE_SIZE_FACTOR = 1;
+const NODE_SEPARATION_FACTOR = 2.5;
+const RANK_SEPARATION_FACTOR = 2.5;
 let layoutRuns = 0;
 let layoutRunsTrivial = 0;
 
@@ -29,15 +37,16 @@ function runLayoutEngine(graph, imNodes, imEdges, opts) {
   }
 
   const options = opts || {};
-  const margins = options.margins || {top: 0, left: 0};
-  const width = options.width || 800;
-  const height = options.height || width / 2;
-  const scale = options.scale || (val => val * 2);
+  const scale = options.scale || DEFAULT_SCALE;
+  const ranksep = scale(RANK_SEPARATION_FACTOR);
+  const nodesep = scale(NODE_SEPARATION_FACTOR);
+  const nodeWidth = scale(NODE_SIZE_FACTOR);
+  const nodeHeight = scale(NODE_SIZE_FACTOR);
 
   // configure node margins
   graph.setGraph({
-    nodesep: scale(2.5),
-    ranksep: scale(2.5)
+    nodesep: nodesep,
+    ranksep: ranksep
   });
 
   // add nodes to the graph if not already there
@@ -45,15 +54,15 @@ function runLayoutEngine(graph, imNodes, imEdges, opts) {
     if (!graph.hasNode(node.get('id'))) {
       graph.setNode(node.get('id'), {
         id: node.get('id'),
-        width: scale(1),
-        height: scale(1)
+        width: nodeWidth,
+        height: nodeHeight
       });
     }
   });
 
-  // remove nodes that are no longer there
+  // remove nodes that are no longer there or are 0-degree nodes
   graph.nodes().forEach(nodeid => {
-    if (!nodes.has(nodeid)) {
+    if (!nodes.has(nodeid) || nodes.get(nodeid).get('degree') === 0) {
       graph.removeNode(nodeid);
     }
   });
@@ -82,33 +91,18 @@ function runLayoutEngine(graph, imNodes, imEdges, opts) {
   dagre.layout(graph);
   const layout = graph.graph();
 
-  // shifting graph coordinates to center
-
-  let offsetX = 0 + margins.left;
-  let offsetY = 0 + margins.top;
-
-  if (layout.width < width) {
-    offsetX = (width - layout.width) / 2 + margins.left;
-  }
-  if (layout.height < height) {
-    offsetY = (height - layout.height) / 2 + margins.top;
-  }
-
   // apply coordinates to nodes and edges
 
   graph.nodes().forEach(id => {
     const graphNode = graph.node(id);
-    nodes = nodes.setIn([id, 'x'], graphNode.x + offsetX);
-    nodes = nodes.setIn([id, 'y'], graphNode.y + offsetY);
+    nodes = nodes.setIn([id, 'x'], graphNode.x);
+    nodes = nodes.setIn([id, 'y'], graphNode.y);
   });
 
   graph.edges().forEach(id => {
     const graphEdge = graph.edge(id);
     const edge = edges.get(graphEdge.id);
-    const points = graphEdge.points.map(point => ({
-      x: point.x + offsetX,
-      y: point.y + offsetY
-    }));
+    const points = graphEdge.points;
 
     // set beginning and end points to node coordinates to ignore node bounding box
     const source = nodes.get(edge.get('source'));
@@ -125,6 +119,126 @@ function runLayoutEngine(graph, imNodes, imEdges, opts) {
   return layout;
 }
 
+/**
+ * Add coordinates to 0-degree nodes using a square layout
+ * Depending on the previous layout run's graph aspect ratio, the square will be
+ * placed on the right side or below the graph.
+ * @param  {Object} layout Layout with nodes and edges
+ * @param  {Object} opts   Options with node distances
+ * @return {Object}        modified layout
+ */
+function layoutSingleNodes(layout, opts) {
+  const options = opts || {};
+  const margins = options.margins || DEFAULT_MARGINS;
+  const scale = options.scale || DEFAULT_SCALE;
+  const ranksep = scale(RANK_SEPARATION_FACTOR) / 2; // dagre splits it in half
+  const nodesep = scale(NODE_SEPARATION_FACTOR);
+  const nodeWidth = scale(NODE_SIZE_FACTOR);
+  const nodeHeight = scale(NODE_SIZE_FACTOR);
+  const aspectRatio = layout.height ? layout.width / layout.height : 1;
+
+  let nodes = layout.nodes;
+
+  // 0-degree nodes
+  const singleNodes = nodes.filter(node => node.get('degree') === 0);
+
+  if (singleNodes.size) {
+    let offsetX;
+    let offsetY;
+    const nonSingleNodes = nodes.filter(node => node.get('degree') !== 0);
+    if (nonSingleNodes.size > 0) {
+      if (aspectRatio < 1) {
+        debug('laying out single nodes to the right', aspectRatio);
+        offsetX = nonSingleNodes.maxBy(node => node.get('x')).get('x');
+        offsetY = nonSingleNodes.minBy(node => node.get('y')).get('y');
+        if (offsetX) {
+          offsetX += nodeWidth + nodesep;
+        }
+      } else {
+        debug('laying out single nodes below', aspectRatio);
+        offsetX = nonSingleNodes.minBy(node => node.get('x')).get('x');
+        offsetY = nonSingleNodes.maxBy(node => node.get('y')).get('y');
+        if (offsetY) {
+          offsetY += nodeHeight + ranksep;
+        }
+      }
+    }
+
+    // default margins
+    offsetX = offsetX || margins.left + nodeWidth / 2;
+    offsetY = offsetY || margins.top + nodeHeight / 2;
+
+    const columns = Math.ceil(Math.sqrt(singleNodes.size));
+    let row = 0;
+    let col = 0;
+    let singleX;
+    let singleY;
+    nodes = nodes.sortBy(node => node.get('rank')).map(node => {
+      if (singleNodes.has(node.get('id'))) {
+        if (col === columns) {
+          col = 0;
+          row++;
+        }
+        singleX = col * (nodesep + nodeWidth) + offsetX;
+        singleY = row * (ranksep + nodeHeight) + offsetY;
+        col++;
+        return node.merge({
+          x: singleX,
+          y: singleY
+        });
+      }
+      return node;
+    });
+
+    // adjust layout dimensions if graph is now bigger
+    layout.width = Math.max(layout.width, singleX + nodeWidth / 2 + nodesep);
+    layout.height = Math.max(layout.height, singleY + nodeHeight / 2 + ranksep);
+    layout.nodes = nodes;
+  }
+
+  return layout;
+}
+
+/**
+ * Shifts all coordinates of node and edge points to make the layout more centered
+ * @param  {Object} layout Layout
+ * @param  {Object} opts   Options with width and margins
+ * @return {Object}        modified layout
+ */
+function shiftLayoutToCenter(layout, opts) {
+  const options = opts || {};
+  const margins = options.margins || DEFAULT_MARGINS;
+  const width = options.width || DEFAULT_WIDTH;
+  const height = options.height || width / 2;
+
+  let offsetX = 0 + margins.left;
+  let offsetY = 0 + margins.top;
+
+  if (layout.width < width) {
+    offsetX = (width - layout.width) / 2 + margins.left;
+  }
+  if (layout.height < height) {
+    offsetY = (height - layout.height) / 2 + margins.top;
+  }
+
+  layout.nodes = layout.nodes.map(node => {
+    return node.merge({
+      x: node.get('x') + offsetX,
+      y: node.get('y') + offsetY
+    });
+  });
+
+  layout.edges = layout.edges.map(edge => {
+    const points = edge.get('points').map(point => ({
+      x: point.x + offsetX,
+      y: point.y + offsetY
+    }));
+    return edge.set('points', points);
+  });
+
+  return layout;
+}
+
 /**
  * Adds `points` array to edge based on location of source and target
  * @param {Map} edge           new edge
@@ -210,12 +324,12 @@ function copyLayoutProperties(layout, nodeCache, edgeCache) {
  * Layout of nodes and edges
  * If a previous layout was given and not too much changed, the previous layout
  * is changed and returned. Otherwise does a new layout engine run.
- * @param  {Map} nodes All nodes
- * @param  {Map} edges All edges
+ * @param  {Map} immNodes All nodes
+ * @param  {Map} immEdges All edges
  * @param  {object} opts  width, height, margins, etc...
  * @return {object} graph object with nodes, edges, dimensions
  */
-export function doLayout(nodes, edges, opts) {
+export function doLayout(immNodes, immEdges, opts) {
   const options = opts || {};
   const topologyId = options.topologyId || 'noId';
 
@@ -235,14 +349,17 @@ export function doLayout(nodes, edges, opts) {
   let layout;
 
   ++layoutRuns;
-  if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(nodes, nodeCache)) {
+  if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(immNodes, nodeCache)) {
     debug('skip layout, trivial adjustment', ++layoutRunsTrivial, layoutRuns);
-    layout = cloneLayout(cachedLayout, nodes, edges);
+    layout = cloneLayout(cachedLayout, immNodes, immEdges);
     // copy old properties, works also if nodes get re-added
     layout = copyLayoutProperties(layout, nodeCache, edgeCache);
   } else {
     const graph = cache.graph;
-    layout = runLayoutEngine(graph, nodes, edges, opts);
+    const nodesWithDegrees = TopologyUtils.updateNodeDegrees(immNodes, immEdges);
+    layout = runLayoutEngine(graph, nodesWithDegrees, immEdges, opts);
+    layout = layoutSingleNodes(layout, opts);
+    layout = shiftLayoutToCenter(layout, opts);
   }
 
   // cache results

client/app/scripts/charts/topology-utils.js

@@ -0,0 +1,10 @@
+
+export function updateNodeDegrees(nodes, edges) {
+  return nodes.map(node => {
+    const nodeId = node.get('id');
+    const degree = edges.count(edge => {
+      return edge.get('source') === nodeId || edge.get('target') === nodeId;
+    });
+    return node.set('degree', degree);
+  });
+}