From 3d08b154303e8f611ab3c4817e604873458e1b0c Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 27 Oct 2015 11:12:19 +0000 Subject: [PATCH 01/11] JS test framework for node layout --- .../charts/__tests__/node-layout-test.js | 50 +++++++++++++++++++ client/package.json | 1 + 2 files changed, 51 insertions(+) create mode 100644 client/app/scripts/charts/__tests__/node-layout-test.js diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js new file mode 100644 index 000000000..a6d8d3cf2 --- /dev/null +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -0,0 +1,50 @@ +jest.dontMock('../nodes-layout'); +jest.dontMock('../../constants/naming'); // edge naming: 'source-target' + +describe('NodesLayout', () => { + const NodesLayout = require('../nodes-layout'); + + function scale(val) { + return val * 3; + } + const topologyId = 'tid'; + const width = 80; + const height = 80; + const margins = { + left: 0, + top: 0 + }; + + const nodeSets = { + initial4: { + nodes: { + n1: {id: 'n1'}, + n2: {id: 'n2'}, + n3: {id: 'n3'}, + n4: {id: 'n4'} + }, + edges: { + 'n1-n3': {id: 'n1-n3', source: {id: 'n1'}, target: {id: 'n3'}}, + 'n1-n4': {id: 'n1-n4', source: {id: 'n1'}, target: {id: 'n4'}}, + 'n2-n4': {id: 'n2-n4', source: {id: 'n2'}, target: {id: 'n4'}} + } + } + }; + + it('lays out initial nodeset', () => { + const nodes = nodeSets.initial4.nodes; + const edges = nodeSets.initial4.edges; + NodesLayout.doLayout(nodes, edges, width, height, scale, margins, topologyId); + expect(nodes.n1.x).toBeLessThan(nodes.n2.x); + expect(nodes.n1.y).toEqual(nodes.n2.y); + + expect(nodes.n1.x).toEqual(nodes.n3.x); + expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + + expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + expect(nodes.n3.y).toEqual(nodes.n4.y); + + console.log(nodes, nodeSets.initial4.nodes); + }); + +}); diff --git a/client/package.json b/client/package.json index 0fd48ea14..6d4fcbdeb 100644 --- a/client/package.json +++ b/client/package.json @@ -78,6 +78,7 @@ "react", "immutable", "d3", + "dagre", "keymirror", "object-assign", "lodash", From e4da515fa1c121583910e5998f455ec5845995d4 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 27 Oct 2015 11:27:52 +0000 Subject: [PATCH 02/11] Refactored doLayout signature --- .../charts/__tests__/node-layout-test.js | 2 +- client/app/scripts/charts/nodes-chart.js | 17 ++++++++--------- client/app/scripts/charts/nodes-layout.js | 15 +++++++++------ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index a6d8d3cf2..a8799bd6f 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -34,7 +34,7 @@ describe('NodesLayout', () => { it('lays out initial nodeset', () => { const nodes = nodeSets.initial4.nodes; const edges = nodeSets.initial4.edges; - NodesLayout.doLayout(nodes, edges, width, height, scale, margins, topologyId); + NodesLayout.doLayout(nodes, edges); expect(nodes.n1.x).toBeLessThan(nodes.n2.x); expect(nodes.n1.y).toEqual(nodes.n2.y); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 7fb918129..945554a94 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -428,17 +428,16 @@ const NodesChart = React.createClass({ const nodeSize = expanse / 3; // single node should fill a third of the screen const normalizedNodeSize = nodeSize / Math.sqrt(n); // assuming rectangular layout const nodeScale = this.state.nodeScale.range([0, normalizedNodeSize]); + const options = { + width: props.width, + height: props.height, + scale: nodeScale, + margins: MARGINS, + topologyId: this.props.topologyId + }; const timedLayouter = timely(NodesLayout.doLayout); - const graph = timedLayouter( - nodes, - edges, - props.width, - props.height, - nodeScale, - MARGINS, - this.props.topologyId - ); + const graph = timedLayouter(nodes, edges, options); debug('graph layout took ' + timedLayouter.time + 'ms'); diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 5ee720f5b..a86247991 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -6,7 +6,14 @@ const _ = require('lodash'); const MAX_NODES = 100; const topologyGraphs = {}; -const doLayout = function(nodes, edges, width, height, scale, margins, topologyId) { +export function doLayout(nodes, edges, 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 topologyId = options.topologyId || 'noId'; + let offsetX = 0 + margins.left; let offsetY = 0 + margins.top; let graph; @@ -101,8 +108,4 @@ const doLayout = function(nodes, edges, width, height, scale, margins, topologyI // return object with the width and height of layout return layout; -}; - -module.exports = { - doLayout: doLayout -}; +} From 7306e8fb5f1c6f5e4252a5fcf1cc41e6d6fb8a5c Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 27 Oct 2015 20:24:28 +0000 Subject: [PATCH 03/11] Rewrote nodes-layout to use immutablejs --- .../charts/__tests__/node-layout-test.js | 56 +++- client/app/scripts/charts/nodes-chart.js | 268 ++++++++++-------- client/app/scripts/charts/nodes-layout.js | 102 ++++--- 3 files changed, 253 insertions(+), 173 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index a8799bd6f..04a308bb3 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -1,6 +1,8 @@ jest.dontMock('../nodes-layout'); jest.dontMock('../../constants/naming'); // edge naming: 'source-target' +import { fromJS } from 'immutable'; + describe('NodesLayout', () => { const NodesLayout = require('../nodes-layout'); @@ -14,6 +16,8 @@ describe('NodesLayout', () => { left: 0, top: 0 }; + let history; + let nodes; const nodeSets = { initial4: { @@ -24,27 +28,57 @@ describe('NodesLayout', () => { n4: {id: 'n4'} }, edges: { - 'n1-n3': {id: 'n1-n3', source: {id: 'n1'}, target: {id: 'n3'}}, - 'n1-n4': {id: 'n1-n4', source: {id: 'n1'}, target: {id: 'n4'}}, - 'n2-n4': {id: 'n2-n4', source: {id: 'n2'}, target: {id: 'n4'}} + '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: { + n1: {id: 'n1'}, + n2: {id: 'n2'}, + n3: {id: 'n3'}, + n4: {id: 'n4'} + }, + edges: { + 'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'}, + 'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'} } } }; - it('lays out initial nodeset', () => { - const nodes = nodeSets.initial4.nodes; - const edges = nodeSets.initial4.edges; - NodesLayout.doLayout(nodes, edges); + it('lays out initial nodeset in a rectangle', () => { + const result = NodesLayout.doLayout( + fromJS(nodeSets.initial4.nodes), + fromJS(nodeSets.initial4.edges)); + // console.log('initial', result.get('nodes')); + nodes = result.nodes.toJS(); + expect(nodes.n1.x).toBeLessThan(nodes.n2.x); expect(nodes.n1.y).toEqual(nodes.n2.y); - expect(nodes.n1.x).toEqual(nodes.n3.x); expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - expect(nodes.n3.x).toBeLessThan(nodes.n4.x); expect(nodes.n3.y).toEqual(nodes.n4.y); - - console.log(nodes, nodeSets.initial4.nodes); }); + // it('keeps nodes in rectangle after removing one edge', () => { + // history = [{ + // nodes: nodeSets.initial4.nodes, + // edges: nodeSets.initial4.edges + // }]; + // nodes = nodeSets.removeEdge24.nodes; + // edges = nodeSets.removeEdge24.edges; + // NodesLayout.doLayout(nodes, edges, {history}); + // console.log('remove 1 edge', nodes); + // + // expect(nodes.n1.x).toBeLessThan(nodes.n2.x); + // expect(nodes.n1.y).toEqual(nodes.n2.y); + // expect(nodes.n1.x).toEqual(nodes.n3.x); + // expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + // expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + // expect(nodes.n3.y).toEqual(nodes.n4.y); + // + // }); + }); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 945554a94..a0c98336f 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -2,6 +2,7 @@ const _ = require('lodash'); const d3 = require('d3'); const debug = require('debug')('scope:nodes-chart'); const React = require('react'); +const makeMap = require('immutable').Map; const timely = require('timely'); const Spring = require('react-motion').Spring; @@ -28,8 +29,8 @@ const NodesChart = React.createClass({ getInitialState: function() { return { - nodes: {}, - edges: {}, + nodes: makeMap(), + edges: makeMap(), nodeScale: d3.scale.linear(), shiftTranslate: [0, 0], panTranslate: [0, 0], @@ -62,8 +63,8 @@ const NodesChart = React.createClass({ if (nextProps.topologyId !== this.props.topologyId) { _.assign(state, { autoShifted: false, - nodes: {}, - edges: {} + nodes: makeMap(), + edges: makeMap() }); } // FIXME add PureRenderMixin, Immutables, and move the following functions to render() @@ -96,60 +97,81 @@ const NodesChart = React.createClass({ const adjacency = hasSelectedNode ? AppStore.getAdjacentNodes(this.props.selectedNodeId) : null; const onNodeClick = this.props.onNodeClick; - _.each(nodes, function(node) { - node.highlighted = _.includes(this.props.highlightedNodeIds, node.id) - || this.props.selectedNodeId === node.id; - node.focused = hasSelectedNode - && (this.props.selectedNodeId === node.id || adjacency.includes(node.id)); - node.blurred = hasSelectedNode && !node.focused; - }, this); + // highlighter functions + const setHighlighted = node => { + const highlighted = _.includes(this.props.highlightedNodeIds, node.get('id')) + || this.props.selectedNodeId === node.get('id'); + return node.set('highlighted', highlighted); + }; + const setFocused = node => { + const focused = hasSelectedNode + && (this.props.selectedNodeId === node.get('id') || adjacency.includes(node.get('id'))); + return node.set('focused', focused); + }; + const setBlurred = node => { + return node.set('blurred', hasSelectedNode && !node.get('focused')); + }; - return _.chain(nodes) - .sortBy(function(node) { - if (node.blurred) { - return 0; - } - if (node.highlighted) { - return 2; - } - return 1; - }) - .map(function(node) { - return ( - { + if (node.get('blurred')) { + return 0; + } + if (node.get('highlighted')) { + return 2; + } + return 1; + }; + + return nodes + .toIndexedSeq() + .map(setHighlighted) + .map(setFocused) + .map(setBlurred) + .sortBy(sortNodes) + .map(node => { + return ( ); - }) - .value(); + }); }, renderGraphEdges: function(edges) { const selectedNodeId = this.props.selectedNodeId; const hasSelectedNode = selectedNodeId && this.props.nodes.has(selectedNodeId); - return _.map(edges, function(edge) { - const highlighted = _.includes(this.props.highlightedEdgeIds, edge.id); - const blurred = hasSelectedNode - && edge.source.id !== selectedNodeId - && edge.target.id !== selectedNodeId; - return ( - - ); - }, this); + const setHighlighted = edge => { + return edge.set('highlighted', _.includes(this.props.highlightedEdgeIds, edge.get('id'))); + }; + const setBlurred = edge => { + return (edge.set('blurred', hasSelectedNode + && edge.get('source') !== selectedNodeId + && edge.get('target') !== selectedNodeId)); + }; + + return edges + .toIndexedSeq() + .map(setHighlighted) + .map(setBlurred) + .map(edge => { + return ( + + ); + }); }, renderMaxNodesError: function(show) { @@ -187,7 +209,7 @@ const NodesChart = React.createClass({ translate = shiftTranslate; wasShifted = true; } - const svgClassNames = this.state.maxNodesExceeded || _.size(nodeElements) === 0 ? 'hide' : ''; + const svgClassNames = this.state.maxNodesExceeded || nodeElements.size === 0 ? 'hide' : ''; const errorEmpty = this.renderEmptyTopologyError(AppStore.isTopologyEmpty()); const errorMaxNodesExceeded = this.renderMaxNodesError(this.state.maxNodesExceeded); @@ -219,34 +241,22 @@ const NodesChart = React.createClass({ }, initNodes: function(topology) { - const centerX = this.props.width / 2; - const centerY = this.props.height / 2; - const nodes = {}; - - topology.forEach(function(node, id) { - nodes[id] = {}; - - // use cached positions if available - _.defaults(nodes[id], { - x: centerX, - y: centerY - }); - + return topology.map((node, id) => { // copy relevant fields to state nodes - _.assign(nodes[id], { + return makeMap({ id: id, label: node.get('label_major'), pseudo: node.get('pseudo'), subLabel: node.get('label_minor'), - rank: node.get('rank') + rank: node.get('rank'), + x: 0, + y: 0 }); }); - - return nodes; }, - initEdges: function(topology, nodes) { - const edges = {}; + initEdges: function(topology, stateNodes) { + let edges = makeMap(); topology.forEach(function(node, nodeId) { const adjacency = node.get('adjacency'); @@ -255,20 +265,20 @@ const NodesChart = React.createClass({ const edge = [nodeId, adjacent]; const edgeId = edge.join(Naming.EDGE_ID_SEPARATOR); - if (!edges[edgeId]) { - const source = nodes[edge[0]]; - const target = nodes[edge[1]]; + if (!edges.has(edgeId)) { + const source = edge[0]; + const target = edge[1]; - if (!source || !target) { - debug('Missing edge node', edge[0], source, edge[1], target); + if (!stateNodes.has(source) || !stateNodes.has(target)) { + debug('Missing edge node', edge[0], edge[1]); } - edges[edgeId] = { + edges = edges.set(edgeId, makeMap({ id: edgeId, value: 1, source: source, target: target - }; + })); } }); } @@ -278,55 +288,65 @@ const NodesChart = React.createClass({ }, centerSelectedNode: function(props, state) { - const layoutNodes = state.nodes; - const layoutEdges = state.edges; - const selectedLayoutNode = layoutNodes[props.selectedNodeId]; + let stateNodes = state.nodes; + let stateEdges = state.edges; + let selectedLayoutNode = stateNodes.get(props.selectedNodeId); if (!selectedLayoutNode) { return {}; } const adjacency = AppStore.getAdjacentNodes(props.selectedNodeId); - const adjacentLayoutNodes = []; + let adjacentLayoutNodeIds = []; adjacency.forEach(function(adjacentId) { // filter loopback if (adjacentId !== props.selectedNodeId) { - adjacentLayoutNodes.push(layoutNodes[adjacentId]); + adjacentLayoutNodeIds.push(adjacentId); } }); // shift center node a bit const nodeScale = state.nodeScale; - selectedLayoutNode.x = selectedLayoutNode.px + nodeScale(1); - selectedLayoutNode.y = selectedLayoutNode.py + nodeScale(1); + const centerX = selectedLayoutNode.get('px') + nodeScale(1); + const centerY = selectedLayoutNode.get('py') + nodeScale(1); + stateNodes = stateNodes.mergeIn([props.selectedNodeId], { + x: centerX, + y: centerY + }); // circle layout for adjacent nodes - const centerX = selectedLayoutNode.x; - const centerY = selectedLayoutNode.y; - const adjacentCount = adjacentLayoutNodes.length; + const adjacentCount = adjacentLayoutNodeIds.length; const density = radiusDensity(adjacentCount); const radius = Math.min(props.width, props.height) / density; const offsetAngle = Math.PI / 4; - _.each(adjacentLayoutNodes, function(node, i) { - const angle = offsetAngle + Math.PI * 2 * i / adjacentCount; - node.x = centerX + radius * Math.sin(angle); - node.y = centerY + radius * Math.cos(angle); + stateNodes = stateNodes.map((node) => { + const index = adjacentLayoutNodeIds.indexOf(node.get('id')); + if (index > -1) { + const angle = offsetAngle + Math.PI * 2 * index / adjacentCount; + return node.merge({ + x: centerX + radius * Math.sin(angle), + y: centerY + radius * Math.cos(angle) + }); + } + return node; }); // fix all edges for circular nodes - - _.each(layoutEdges, function(edge) { - if (edge.source === selectedLayoutNode - || edge.target === selectedLayoutNode - || _.includes(adjacentLayoutNodes, edge.source) - || _.includes(adjacentLayoutNodes, edge.target)) { - edge.points = [ - {x: edge.source.x, y: edge.source.y}, - {x: edge.target.x, y: edge.target.y} - ]; + stateEdges = stateEdges.map(edge => { + if (edge.get('source') === selectedLayoutNode.get('id') + || edge.get('target') === selectedLayoutNode.get('id') + || _.includes(adjacentLayoutNodeIds, edge.get('source')) + || _.includes(adjacentLayoutNodeIds, edge.get('target'))) { + const source = stateNodes.get(edge.get('source')); + const target = stateNodes.get(edge.get('target')); + return edge.set('points', [ + {x: source.get('x'), y: source.get('y')}, + {x: target.get('x'), y: target.get('y')} + ]); } + return edge; }); // shift canvas selected node out of view if it has not been shifted already @@ -373,8 +393,8 @@ const NodesChart = React.createClass({ return { autoShifted: autoShifted, - edges: layoutEdges, - nodes: layoutNodes, + edges: stateEdges, + nodes: stateNodes, shiftTranslate: shiftTranslate }; }, @@ -394,21 +414,21 @@ const NodesChart = React.createClass({ }, restoreLayout: function(state) { - const edges = state.edges; - const nodes = state.nodes; - - _.each(nodes, function(node) { - node.x = node.px; - node.y = node.py; + const nodes = state.nodes.map(node => { + return node.merge({ + x: node.get('px'), + y: node.get('py') + }); }); - _.each(edges, function(edge) { - if (edge.ppoints) { - edge.points = edge.ppoints; + const edges = state.edges.map(edge => { + if (edge.has('ppoints')) { + return edge.set('points', edge.get('ppoints')); } + return edge; }); - return {edges: edges, nodes: nodes}; + return {edges, nodes}; }, updateGraphState: function(props, state) { @@ -416,13 +436,13 @@ const NodesChart = React.createClass({ if (n === 0) { return { - nodes: {}, - edges: {} + nodes: makeMap(), + edges: makeMap() }; } - const nodes = this.initNodes(props.nodes, state.nodes); - const edges = this.initEdges(props.nodes, nodes); + let stateNodes = this.initNodes(props.nodes, state.nodes); + let stateEdges = this.initEdges(props.nodes, stateNodes); const expanse = Math.min(props.height, props.width); const nodeSize = expanse / 3; // single node should fill a third of the screen @@ -437,7 +457,7 @@ const NodesChart = React.createClass({ }; const timedLayouter = timely(NodesLayout.doLayout); - const graph = timedLayouter(nodes, edges, options); + const graph = timedLayouter(stateNodes, stateEdges, options); debug('graph layout took ' + timedLayouter.time + 'ms'); @@ -445,14 +465,18 @@ const NodesChart = React.createClass({ if (!graph) { return {maxNodesExceeded: true}; } + stateNodes = graph.nodes; + stateEdges = graph.edges; // save coordinates for restore - _.each(nodes, function(node) { - node.px = node.x; - node.py = node.y; + stateNodes = stateNodes.map(node => { + return node.merge({ + px: node.get('x'), + py: node.get('y') + }); }); - _.each(edges, function(edge) { - edge.ppoints = edge.points; + stateEdges = stateEdges.map(edge => { + return edge.set('ppoints', edge.get('points')); }); // adjust layout based on viewport @@ -468,8 +492,8 @@ const NodesChart = React.createClass({ } return { - nodes: nodes, - edges: edges, + nodes: stateNodes, + edges: stateEdges, nodeScale: nodeScale, scale: zoomScale, maxNodesExceeded: false diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index a86247991..91ee6061c 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -1,12 +1,19 @@ const dagre = require('dagre'); const debug = require('debug')('scope:nodes-layout'); const Naming = require('../constants/naming'); -const _ = require('lodash'); const MAX_NODES = 100; const topologyGraphs = {}; -export function doLayout(nodes, edges, opts) { +function runLayoutEngine(imNodes, imEdges, opts) { + let nodes = imNodes; + let edges = imEdges; + + if (nodes.size > MAX_NODES) { + debug('Too many nodes for graph layout engine. Limit: ' + MAX_NODES); + return null; + } + const options = opts || {}; const margins = options.margins || {top: 0, left: 0}; const width = options.width || 800; @@ -14,20 +21,11 @@ export function doLayout(nodes, edges, opts) { const scale = options.scale || (val => val * 2); const topologyId = options.topologyId || 'noId'; - let offsetX = 0 + margins.left; - let offsetY = 0 + margins.top; - let graph; - - if (_.size(nodes) > MAX_NODES) { - debug('Too many nodes for graph layout engine. Limit: ' + MAX_NODES); - return null; - } - // one engine per topology, to keep renderings similar if (!topologyGraphs[topologyId]) { topologyGraphs[topologyId] = new dagre.graphlib.Graph({}); } - graph = topologyGraphs[topologyId]; + const graph = topologyGraphs[topologyId]; // configure node margins graph.setGraph({ @@ -36,10 +34,10 @@ export function doLayout(nodes, edges, opts) { }); // add nodes to the graph if not already there - _.each(nodes, function(node) { - if (!graph.hasNode(node.id)) { - graph.setNode(node.id, { - id: node.id, + nodes.forEach(node => { + if (!graph.hasNode(node.get('id'))) { + graph.setNode(node.get('id'), { + id: node.get('id'), width: scale(1), height: scale(1) }); @@ -47,35 +45,41 @@ export function doLayout(nodes, edges, opts) { }); // remove nodes that are no longer there - _.each(graph.nodes(), function(nodeid) { - if (!_.has(nodes, nodeid)) { + graph.nodes().forEach(nodeid => { + if (!nodes.has(nodeid)) { graph.removeNode(nodeid); } }); // add edges to the graph if not already there - _.each(edges, function(edge) { - if (!graph.hasEdge(edge.source.id, edge.target.id)) { - const virtualNodes = edge.source.id === edge.target.id ? 1 : 0; - graph.setEdge(edge.source.id, edge.target.id, {id: edge.id, minlen: virtualNodes}); + edges.forEach(edge => { + if (!graph.hasEdge(edge.get('source'), edge.get('target'))) { + const virtualNodes = edge.get('source') === edge.get('target') ? 1 : 0; + graph.setEdge( + edge.get('source'), + edge.get('target'), + {id: edge.get('id'), minlen: virtualNodes} + ); } }); - // remoed egdes that are no longer there - _.each(graph.edges(), function(edgeObj) { + // remove edges that are no longer there + graph.edges().forEach(edgeObj => { const edge = [edgeObj.v, edgeObj.w]; const edgeId = edge.join(Naming.EDGE_ID_SEPARATOR); - if (!_.has(edges, edgeId)) { + if (!edges.has(edgeId)) { graph.removeEdge(edgeObj.v, edgeObj.w); } }); 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; } @@ -85,27 +89,45 @@ export function doLayout(nodes, edges, opts) { // apply coordinates to nodes and edges - graph.nodes().forEach(function(id) { - const node = nodes[id]; + graph.nodes().forEach(id => { const graphNode = graph.node(id); - node.x = graphNode.x + offsetX; - node.y = graphNode.y + offsetY; + nodes = nodes.setIn([id, 'x'], graphNode.x + offsetX); + nodes = nodes.setIn([id, 'y'], graphNode.y + offsetY); }); - graph.edges().forEach(function(id) { + graph.edges().forEach(id => { const graphEdge = graph.edge(id); - const edge = edges[graphEdge.id]; - _.each(graphEdge.points, function(point) { - point.x += offsetX; - point.y += offsetY; - }); - edge.points = graphEdge.points; + const edge = edges.get(graphEdge.id); + const points = graphEdge.points.map(point => ({ + x: point.x + offsetX, + y: point.y + offsetY + })); + // set beginning and end points to node coordinates to ignore node bounding box - edge.points[0] = {x: edge.source.x, y: edge.source.y}; - edge.points[edge.points.length - 1] = {x: edge.target.x, y: edge.target.y}; + const source = nodes.get(edge.get('source')); + const target = nodes.get(edge.get('target')); + points[0] = {x: source.get('x'), y: source.get('y')}; + points[points.length - 1] = {x: target.get('x'), y: target.get('y')}; + + edges = edges.setIn([graphEdge.id, 'points'], points); }); // return object with the width and height of layout - + layout.nodes = nodes; + layout.edges = edges; return layout; } + +/** + * Layout of nodes and edges + * @param {Map} nodes All nodes + * @param {Map} edges All edges + * @param {object} opts width, height, margins, etc... + * @return {object} graph object with nodes, edges, dimensions + */ +export function doLayout(nodes, edges, opts) { + // const options = opts || {}; + // const history = options.history || []; + + return runLayoutEngine(nodes, edges, opts); +} From 47ba0ff2a4360f14d678942862b6205cba0f60be Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 28 Oct 2015 14:05:19 +0000 Subject: [PATCH 04/11] Skip layout when only edges changed --- .../charts/__tests__/node-layout-test.js | 101 +++++++++++++----- client/app/scripts/charts/nodes-chart.js | 5 +- client/app/scripts/charts/nodes-layout.js | 53 ++++++++- 3 files changed, 126 insertions(+), 33 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index 04a308bb3..f76f7ce00 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -1,7 +1,7 @@ jest.dontMock('../nodes-layout'); jest.dontMock('../../constants/naming'); // edge naming: 'source-target' -import { fromJS } from 'immutable'; +import { fromJS, is } from 'immutable'; describe('NodesLayout', () => { const NodesLayout = require('../nodes-layout'); @@ -21,36 +21,36 @@ describe('NodesLayout', () => { const nodeSets = { initial4: { - nodes: { + nodes: fromJS({ n1: {id: 'n1'}, n2: {id: 'n2'}, n3: {id: 'n3'}, n4: {id: 'n4'} - }, - edges: { + }), + 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: { + nodes: fromJS({ n1: {id: 'n1'}, n2: {id: 'n2'}, n3: {id: 'n3'}, n4: {id: 'n4'} - }, - edges: { + }), + edges: fromJS({ 'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'}, 'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'} - } + }) } }; it('lays out initial nodeset in a rectangle', () => { const result = NodesLayout.doLayout( - fromJS(nodeSets.initial4.nodes), - fromJS(nodeSets.initial4.edges)); + nodeSets.initial4.nodes, + nodeSets.initial4.edges); // console.log('initial', result.get('nodes')); nodes = result.nodes.toJS(); @@ -62,23 +62,66 @@ describe('NodesLayout', () => { expect(nodes.n3.y).toEqual(nodes.n4.y); }); - // it('keeps nodes in rectangle after removing one edge', () => { - // history = [{ - // nodes: nodeSets.initial4.nodes, - // edges: nodeSets.initial4.edges - // }]; - // nodes = nodeSets.removeEdge24.nodes; - // edges = nodeSets.removeEdge24.edges; - // NodesLayout.doLayout(nodes, edges, {history}); - // console.log('remove 1 edge', nodes); - // - // expect(nodes.n1.x).toBeLessThan(nodes.n2.x); - // expect(nodes.n1.y).toEqual(nodes.n2.y); - // expect(nodes.n1.x).toEqual(nodes.n3.x); - // expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - // expect(nodes.n3.x).toBeLessThan(nodes.n4.x); - // expect(nodes.n3.y).toEqual(nodes.n4.y); - // - // }); + it('keeps nodes in rectangle after removing one edge', () => { + let result = NodesLayout.doLayout( + nodeSets.initial4.nodes, + nodeSets.initial4.edges); + history = [{ + nodes: result.nodes, + edges: result.edges + }]; + result = NodesLayout.doLayout( + nodeSets.removeEdge24.nodes, + nodeSets.removeEdge24.edges, + {history} + ); + nodes = result.nodes.toJS(); + // console.log('remove 1 edge', nodes, result); + + expect(nodes.n1.x).toBeLessThan(nodes.n2.x); + expect(nodes.n1.y).toEqual(nodes.n2.y); + expect(nodes.n1.x).toEqual(nodes.n3.x); + expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + expect(nodes.n3.y).toEqual(nodes.n4.y); + }); + + it('keeps nodes in rectangle after removed edge reappears', () => { + let result = NodesLayout.doLayout( + nodeSets.initial4.nodes, + nodeSets.initial4.edges); + + history = [{ + nodes: result.nodes, + edges: result.edges + }]; + result = NodesLayout.doLayout( + nodeSets.removeEdge24.nodes, + nodeSets.removeEdge24.edges, + {history} + ); + + history = [{ + nodes: result.nodes, + edges: result.edges + }]; + result = NodesLayout.doLayout( + nodeSets.initial4.nodes, + nodeSets.initial4.edges, + {history} + ); + + nodes = result.nodes.toJS(); + // console.log('re-add 1 edge', nodes, result); + + expect(nodes.n1.x).toBeLessThan(nodes.n2.x); + expect(nodes.n1.y).toEqual(nodes.n2.y); + expect(nodes.n1.x).toEqual(nodes.n3.x); + expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + expect(nodes.n3.y).toEqual(nodes.n4.y); + }); + + }); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index a0c98336f..7bc093227 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -31,6 +31,7 @@ const NodesChart = React.createClass({ return { nodes: makeMap(), edges: makeMap(), + history: [], nodeScale: d3.scale.linear(), shiftTranslate: [0, 0], panTranslate: [0, 0], @@ -453,7 +454,8 @@ const NodesChart = React.createClass({ height: props.height, scale: nodeScale, margins: MARGINS, - topologyId: this.props.topologyId + topologyId: this.props.topologyId, + history: state.history }; const timedLayouter = timely(NodesLayout.doLayout); @@ -492,6 +494,7 @@ const NodesChart = React.createClass({ } return { + history: [graph], nodes: stateNodes, edges: stateEdges, nodeScale: nodeScale, diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 91ee6061c..686973b4d 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -1,5 +1,6 @@ const dagre = require('dagre'); const debug = require('debug')('scope:nodes-layout'); +const ImmSet = require('immutable').Set; const Naming = require('../constants/naming'); const MAX_NODES = 100; @@ -118,6 +119,38 @@ function runLayoutEngine(imNodes, imEdges, opts) { return layout; } +function doLayoutEdges(nodes, edges, previousLayout) { + const previousEdges = previousLayout.edges; + + // remove old edges + let layoutEdges = previousEdges.filter(edge => { + return edges.has(edge.get('id')); + }); + + // add new edges with points from source and target + let source; + let target; + let layoutEdge; + edges.forEach(edge => { + if (!layoutEdges.has(edge.get('id'))) { + source = nodes.get(edge.get('source')); + target = nodes.get(edge.get('target')); + layoutEdge = edge.set('points', [ + {x: source.get('x'), y: source.get('y')}, + {x: target.get('x'), y: target.get('y')} + ]); + layoutEdges = layoutEdges.set(layoutEdge.get('id'), layoutEdge); + } + }); + + previousLayout.edges = layoutEdges; + return previousLayout; +} + +function hasSameNodes(nodes, prevNodes) { + return ImmSet.fromKeys(nodes).equals(ImmSet.fromKeys(prevNodes)); +} + /** * Layout of nodes and edges * @param {Map} nodes All nodes @@ -126,8 +159,22 @@ function runLayoutEngine(imNodes, imEdges, opts) { * @return {object} graph object with nodes, edges, dimensions */ export function doLayout(nodes, edges, opts) { - // const options = opts || {}; - // const history = options.history || []; + const options = opts || {}; + const history = options.history || []; + const previous = history.pop(); + let layout; - return runLayoutEngine(nodes, edges, opts); + if (previous) { + // add/remove edges if nodes are the same + if (hasSameNodes(previous.nodes, nodes)) { + debug('skip layout, only edges changed', edges.size, previous.edges.size); + layout = doLayoutEdges(nodes, edges, previous); + } + } + + if (layout === undefined) { + layout = runLayoutEngine(nodes, edges, opts); + } + + return layout; } From 83d7a87d4382827aeb165bb68e92cc767202a9df Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 28 Oct 2015 17:02:09 +0000 Subject: [PATCH 05/11] dont relayout when node dissappears --- .../charts/__tests__/node-layout-test.js | 54 ++++++++++++--- client/app/scripts/charts/nodes-chart.js | 10 ++- client/app/scripts/charts/nodes-layout.js | 67 +++++++++++++++++-- 3 files changed, 114 insertions(+), 17 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index f76f7ce00..0ad74e454 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -1,7 +1,7 @@ jest.dontMock('../nodes-layout'); jest.dontMock('../../constants/naming'); // edge naming: 'source-target' -import { fromJS, is } from 'immutable'; +import { fromJS, List } from 'immutable'; describe('NodesLayout', () => { const NodesLayout = require('../nodes-layout'); @@ -16,7 +16,7 @@ describe('NodesLayout', () => { left: 0, top: 0 }; - let history; + let history = List(); let nodes; const nodeSets = { @@ -44,9 +44,24 @@ describe('NodesLayout', () => { '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'} + }) + }, }; + beforeEach(() => { + history = history.clear(); + }) + it('lays out initial nodeset in a rectangle', () => { const result = NodesLayout.doLayout( nodeSets.initial4.nodes, @@ -66,10 +81,10 @@ describe('NodesLayout', () => { let result = NodesLayout.doLayout( nodeSets.initial4.nodes, nodeSets.initial4.edges); - history = [{ + history = history.unshift({ nodes: result.nodes, edges: result.edges - }]; + }); result = NodesLayout.doLayout( nodeSets.removeEdge24.nodes, nodeSets.removeEdge24.edges, @@ -91,20 +106,20 @@ describe('NodesLayout', () => { nodeSets.initial4.nodes, nodeSets.initial4.edges); - history = [{ + history = history.unshift({ nodes: result.nodes, edges: result.edges - }]; + }); result = NodesLayout.doLayout( nodeSets.removeEdge24.nodes, nodeSets.removeEdge24.edges, {history} ); - history = [{ + history = history.unshift({ nodes: result.nodes, edges: result.edges - }]; + }); result = NodesLayout.doLayout( nodeSets.initial4.nodes, nodeSets.initial4.edges, @@ -122,6 +137,27 @@ describe('NodesLayout', () => { expect(nodes.n3.y).toEqual(nodes.n4.y); }); + it('keeps nodes in rectangle after node dissappears', () => { + let result = NodesLayout.doLayout( + nodeSets.initial4.nodes, + nodeSets.initial4.edges); + history = history.unshift({ + nodes: result.nodes, + edges: result.edges + }); + result = NodesLayout.doLayout( + nodeSets.removeNode2.nodes, + nodeSets.removeNode2.edges, + {history} + ); + + nodes = result.nodes.toJS(); + + expect(nodes.n1.x).toEqual(nodes.n3.x); + expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + expect(nodes.n3.y).toEqual(nodes.n4.y); + }); }); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 7bc093227..aa33776b5 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -2,6 +2,7 @@ const _ = require('lodash'); const d3 = require('d3'); const debug = require('debug')('scope:nodes-chart'); const React = require('react'); +const makeList = require('immutable').List; const makeMap = require('immutable').Map; const timely = require('timely'); const Spring = require('react-motion').Spring; @@ -21,6 +22,8 @@ const MARGINS = { bottom: 0 }; +const MAX_HISTORY = 3; + // make sure circular layouts a bit denser with 3-6 nodes const radiusDensity = d3.scale.threshold() .domain([3, 6]).range([2.5, 3.5, 3]); @@ -31,7 +34,7 @@ const NodesChart = React.createClass({ return { nodes: makeMap(), edges: makeMap(), - history: [], + history: makeList(), nodeScale: d3.scale.linear(), shiftTranslate: [0, 0], panTranslate: [0, 0], @@ -493,8 +496,11 @@ const NodesChart = React.createClass({ this.zoom.scale(zoomFactor); } + // throw away old layouts and save this layout result, first item is recent + const history = state.history.setSize(MAX_HISTORY - 1).unshift(graph); + return { - history: [graph], + history: history, nodes: stateNodes, edges: stateEdges, nodeScale: nodeScale, diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 686973b4d..5d5cfb93e 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -6,6 +6,16 @@ const Naming = require('../constants/naming'); const MAX_NODES = 100; const topologyGraphs = {}; +/** + * Wrapper around layout engine + * After the layout engine run nodes and edges have x-y-coordinates. Creates and + * reuses one engine per topology. Engine is not run if the number of nodes is + * bigger than `MAX_NODES`. + * @param {Map} imNodes new node set + * @param {Map} imEdges new edge set + * @param {Object} opts dimensions, scales, etc. + * @return {Object} Layout with nodes, edges, dimensions + */ function runLayoutEngine(imNodes, imEdges, opts) { let nodes = imNodes; let edges = imEdges; @@ -119,7 +129,14 @@ function runLayoutEngine(imNodes, imEdges, opts) { return layout; } -function doLayoutEdges(nodes, edges, previousLayout) { +/** + * Modifies add/remove edges to a previous layout based on what is present in + * the new edge set + * @param {Map} nodes new node set + * @param {Map} edges new edges + * @param {Object} previousLayout modified layout + */ +function addRemoveLayoutEdges(nodes, edges, previousLayout) { const previousEdges = previousLayout.edges; // remove old edges @@ -147,12 +164,48 @@ function doLayoutEdges(nodes, edges, previousLayout) { return previousLayout; } +/** + * Removes nodes from `previousLayout.nodes` that are not in `nodes` and returns + * the modified `previousLayout`. + * @param {Map} nodes new set of nodes + * @param {Map} edges new set of edges + * @param {object} previousLayout old layout + * @return {Object} Layout with nodes and and edges + */ +function removeOldLayoutNodes(nodes, edges, previousLayout) { + const previousNodes = previousLayout.nodes; + let layoutNodes = previousNodes.filter(node => { + return nodes.has(node.get('id')); + }); + previousLayout.nodes = layoutNodes; + return previousLayout; +} + +/** + * Determine if two node sets have the same nodes + * @param {Map} nodes new node set + * @param {Map} prevNodes old node set + * @return {Boolean} True if node ids of both sets are the same + */ function hasSameNodes(nodes, prevNodes) { return ImmSet.fromKeys(nodes).equals(ImmSet.fromKeys(prevNodes)); } +/** + * Determine if nodes were removed between node sets + * @param {Map} nodes new Map of nodes + * @param {Map} prevNodes old Map of nodes + * @return {Boolean} True if nodes had no new node ids + */ +function wereNodesOnlyRemoved(nodes, prevNodes) { + return (nodes.size < prevNodes.size + && ImmSet.fromKeys(nodes).isSubset(ImmSet.fromKeys(prevNodes))); +} + /** * 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 {object} opts width, height, margins, etc... @@ -160,15 +213,17 @@ function hasSameNodes(nodes, prevNodes) { */ export function doLayout(nodes, edges, opts) { const options = opts || {}; - const history = options.history || []; - const previous = history.pop(); + const previous = options.history && options.history.first(); let layout; if (previous) { - // add/remove edges if nodes are the same - if (hasSameNodes(previous.nodes, nodes)) { + if (hasSameNodes(nodes, previous.nodes)) { debug('skip layout, only edges changed', edges.size, previous.edges.size); - layout = doLayoutEdges(nodes, edges, previous); + layout = addRemoveLayoutEdges(nodes, edges, previous); + } else if (wereNodesOnlyRemoved(nodes, previous.nodes)) { + debug('skip layout, only nodes removed', nodes.size, previous.nodes.size); + layout = removeOldLayoutNodes(nodes, edges, previous); + layout = addRemoveLayoutEdges(nodes, edges, layout); } } From 1fe036ab6a35059fb2c0102e970593502d1661f6 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 29 Oct 2015 14:01:28 +0000 Subject: [PATCH 06/11] Simplified dance heuristics --- .../charts/__tests__/node-layout-test.js | 98 +++++++--- client/app/scripts/charts/nodes-chart.js | 6 - client/app/scripts/charts/nodes-layout.js | 174 +++++++++--------- 3 files changed, 158 insertions(+), 120 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index 0ad74e454..078a16958 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -1,7 +1,7 @@ jest.dontMock('../nodes-layout'); jest.dontMock('../../constants/naming'); // edge naming: 'source-target' -import { fromJS, List } from 'immutable'; +import { fromJS, Map } from 'immutable'; describe('NodesLayout', () => { const NodesLayout = require('../nodes-layout'); @@ -16,7 +16,7 @@ describe('NodesLayout', () => { left: 0, top: 0 }; - let history = List(); + let options; let nodes; const nodeSets = { @@ -56,11 +56,23 @@ describe('NodesLayout', () => { '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'} + }) + } }; beforeEach(() => { - history = history.clear(); - }) + options = { + nodeCache: Map(), + edgeCache: Map() + }; + }); it('lays out initial nodeset in a rectangle', () => { const result = NodesLayout.doLayout( @@ -81,14 +93,15 @@ describe('NodesLayout', () => { let result = NodesLayout.doLayout( nodeSets.initial4.nodes, nodeSets.initial4.edges); - history = history.unshift({ - nodes: result.nodes, - edges: result.edges - }); + + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); + result = NodesLayout.doLayout( nodeSets.removeEdge24.nodes, nodeSets.removeEdge24.edges, - {history} + options ); nodes = result.nodes.toJS(); // console.log('remove 1 edge', nodes, result); @@ -106,24 +119,22 @@ describe('NodesLayout', () => { nodeSets.initial4.nodes, nodeSets.initial4.edges); - history = history.unshift({ - nodes: result.nodes, - edges: result.edges - }); + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); result = NodesLayout.doLayout( nodeSets.removeEdge24.nodes, nodeSets.removeEdge24.edges, - {history} + options ); - history = history.unshift({ - nodes: result.nodes, - edges: result.edges - }); + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); result = NodesLayout.doLayout( nodeSets.initial4.nodes, nodeSets.initial4.edges, - {history} + options ); nodes = result.nodes.toJS(); @@ -142,14 +153,13 @@ describe('NodesLayout', () => { nodeSets.initial4.nodes, nodeSets.initial4.edges); - history = history.unshift({ - nodes: result.nodes, - edges: result.edges - }); + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); result = NodesLayout.doLayout( nodeSets.removeNode2.nodes, nodeSets.removeNode2.edges, - {history} + options ); nodes = result.nodes.toJS(); @@ -160,4 +170,44 @@ describe('NodesLayout', () => { expect(nodes.n3.y).toEqual(nodes.n4.y); }); + it('keeps nodes in rectangle after removed node reappears', () => { + let result = NodesLayout.doLayout( + nodeSets.initial4.nodes, + nodeSets.initial4.edges); + + nodes = result.nodes.toJS(); + + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); + + result = NodesLayout.doLayout( + nodeSets.removeNode23.nodes, + nodeSets.removeNode23.edges, + options + ); + + nodes = result.nodes.toJS(); + + expect(nodes.n1.x).toBeLessThan(nodes.n4.x); + expect(nodes.n1.y).toBeLessThan(nodes.n4.y); + + options.cachedLayout = result; + options.nodeCache = options.nodeCache.merge(result.nodes); + options.edgeCache = options.edgeCache.merge(result.edge); + result = NodesLayout.doLayout( + nodeSets.removeNode2.nodes, + nodeSets.removeNode2.edges, + options + ); + + nodes = result.nodes.toJS(); + // console.log('re-add 1 node', nodes); + + expect(nodes.n1.x).toEqual(nodes.n3.x); + expect(nodes.n1.y).toBeLessThan(nodes.n3.y); + expect(nodes.n3.x).toBeLessThan(nodes.n4.x); + expect(nodes.n3.y).toEqual(nodes.n4.y); + }); + }); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index aa33776b5..7b3ad7f34 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -22,8 +22,6 @@ const MARGINS = { bottom: 0 }; -const MAX_HISTORY = 3; - // make sure circular layouts a bit denser with 3-6 nodes const radiusDensity = d3.scale.threshold() .domain([3, 6]).range([2.5, 3.5, 3]); @@ -496,11 +494,7 @@ const NodesChart = React.createClass({ this.zoom.scale(zoomFactor); } - // throw away old layouts and save this layout result, first item is recent - const history = state.history.setSize(MAX_HISTORY - 1).unshift(graph); - return { - history: history, nodes: stateNodes, edges: stateEdges, nodeScale: nodeScale, diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 5d5cfb93e..0dfae60ae 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -1,22 +1,23 @@ 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 MAX_NODES = 100; -const topologyGraphs = {}; +const topologyCaches = {}; /** - * Wrapper around layout engine - * After the layout engine run nodes and edges have x-y-coordinates. Creates and - * reuses one engine per topology. Engine is not run if the number of nodes is - * bigger than `MAX_NODES`. + * Layout engine runner + * After the layout engine run nodes and edges have x-y-coordinates. Engine is + * not run if the number of nodes is bigger than `MAX_NODES`. + * @param {Object} graph dagre graph instance * @param {Map} imNodes new node set * @param {Map} imEdges new edge set * @param {Object} opts dimensions, scales, etc. * @return {Object} Layout with nodes, edges, dimensions */ -function runLayoutEngine(imNodes, imEdges, opts) { +function runLayoutEngine(graph, imNodes, imEdges, opts) { let nodes = imNodes; let edges = imEdges; @@ -30,13 +31,6 @@ function runLayoutEngine(imNodes, imEdges, opts) { const width = options.width || 800; const height = options.height || width / 2; const scale = options.scale || (val => val * 2); - const topologyId = options.topologyId || 'noId'; - - // one engine per topology, to keep renderings similar - if (!topologyGraphs[topologyId]) { - topologyGraphs[topologyId] = new dagre.graphlib.Graph({}); - } - const graph = topologyGraphs[topologyId]; // configure node margins graph.setGraph({ @@ -130,76 +124,62 @@ function runLayoutEngine(imNodes, imEdges, opts) { } /** - * Modifies add/remove edges to a previous layout based on what is present in - * the new edge set - * @param {Map} nodes new node set - * @param {Map} edges new edges - * @param {Object} previousLayout modified layout + * Adds `points` array to edge based on location of source and target + * @param {Map} edge new edge + * @param {Map} nodeCache all nodes + * @returns {Map} modified edge */ -function addRemoveLayoutEdges(nodes, edges, previousLayout) { - const previousEdges = previousLayout.edges; - - // remove old edges - let layoutEdges = previousEdges.filter(edge => { - return edges.has(edge.get('id')); - }); - - // add new edges with points from source and target - let source; - let target; - let layoutEdge; - edges.forEach(edge => { - if (!layoutEdges.has(edge.get('id'))) { - source = nodes.get(edge.get('source')); - target = nodes.get(edge.get('target')); - layoutEdge = edge.set('points', [ - {x: source.get('x'), y: source.get('y')}, - {x: target.get('x'), y: target.get('y')} - ]); - layoutEdges = layoutEdges.set(layoutEdge.get('id'), layoutEdge); - } - }); - - previousLayout.edges = layoutEdges; - return previousLayout; +function setSimpleEdgePoints(edge, nodeCache) { + const source = nodeCache.get(edge.get('source')); + const target = nodeCache.get(edge.get('target')); + return edge.set('points', [ + {x: source.get('x'), y: source.get('y')}, + {x: target.get('x'), y: target.get('y')} + ]); } /** - * Removes nodes from `previousLayout.nodes` that are not in `nodes` and returns - * the modified `previousLayout`. - * @param {Map} nodes new set of nodes - * @param {Map} edges new set of edges - * @param {object} previousLayout old layout - * @return {Object} Layout with nodes and and edges - */ -function removeOldLayoutNodes(nodes, edges, previousLayout) { - const previousNodes = previousLayout.nodes; - let layoutNodes = previousNodes.filter(node => { - return nodes.has(node.get('id')); - }); - previousLayout.nodes = layoutNodes; - return previousLayout; -} - -/** - * Determine if two node sets have the same nodes - * @param {Map} nodes new node set - * @param {Map} prevNodes old node set - * @return {Boolean} True if node ids of both sets are the same - */ -function hasSameNodes(nodes, prevNodes) { - return ImmSet.fromKeys(nodes).equals(ImmSet.fromKeys(prevNodes)); -} - -/** - * Determine if nodes were removed between node sets + * Determine if nodes were added between node sets * @param {Map} nodes new Map of nodes - * @param {Map} prevNodes old Map of nodes - * @return {Boolean} True if nodes had no new node ids + * @param {Map} cache old Map of nodes + * @return {Boolean} True if nodes had node ids that are not in cache */ -function wereNodesOnlyRemoved(nodes, prevNodes) { - return (nodes.size < prevNodes.size - && ImmSet.fromKeys(nodes).isSubset(ImmSet.fromKeys(prevNodes))); +function hasUnseenNodes(nodes, cache) { + return (nodes.size > cache.size + || !ImmSet.fromKeys(nodes).isSubset(ImmSet.fromKeys(nodes))); +} + +/** + * Clones a previous layout + * @param {Object} layout Layout object + * @param {Map} nodes new nodes + * @param {Map} edges new edges + * @return {Object} layout clone + */ +function cloneLayout(layout, nodes, edges) { + const clone = {...layout, nodes, edges}; + return clone; +} + +/** + * Copies node properties from previous layout runs to new nodes. + * This assumes the cache has data for all new nodes. + * @param {Object} layout Layout + * @param {Object} nodeCache cache of all old nodes + * @param {Object} edgeCache cache of all old edges + * @return {Object} modified layout + */ +function copyLayoutProperties(layout, nodeCache, edgeCache) { + layout.nodes = layout.nodes.map(node => { + return node.merge(nodeCache.get(node.get('id'))); + }); + layout.edges = layout.edges.map(edge => { + if (edgeCache.has(edge.get('id'))) { + return edge.merge(edgeCache.get(edge.get('id'))); + } + return setSimpleEdgePoints(edge, nodeCache); + }); + return layout; } /** @@ -213,23 +193,37 @@ function wereNodesOnlyRemoved(nodes, prevNodes) { */ export function doLayout(nodes, edges, opts) { const options = opts || {}; - const previous = options.history && options.history.first(); + const topologyId = options.topologyId || 'noId'; + + // one engine and node and edge caches per topology, to keep renderings similar + if (!topologyCaches[topologyId]) { + topologyCaches[topologyId] = { + nodeCache: makeMap(), + edgeCache: makeMap(), + graph: new dagre.graphlib.Graph({}) + }; + } + + const cache = topologyCaches[topologyId]; + const cachedLayout = options.cachedLayout || cache.cachedLayout; + const nodeCache = options.nodeCache || cache.nodeCache; + const edgeCache = options.edgeCache || cache.edgeCache; let layout; - if (previous) { - if (hasSameNodes(nodes, previous.nodes)) { - debug('skip layout, only edges changed', edges.size, previous.edges.size); - layout = addRemoveLayoutEdges(nodes, edges, previous); - } else if (wereNodesOnlyRemoved(nodes, previous.nodes)) { - debug('skip layout, only nodes removed', nodes.size, previous.nodes.size); - layout = removeOldLayoutNodes(nodes, edges, previous); - layout = addRemoveLayoutEdges(nodes, edges, layout); - } + if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(nodes, nodeCache)) { + debug('skip layout, trivial adjustment'); + layout = cloneLayout(cachedLayout, nodes, edges); + // 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); } - if (layout === undefined) { - layout = runLayoutEngine(nodes, edges, opts); - } + // cache results + cache.cachedLayout = layout; + cache.nodeCache = cache.nodeCache.merge(layout.nodes); + cache.edgeCache = cache.edgeCache.merge(layout.edges); return layout; } From 413220db6b888bde7a7d1bd5d5cf87bad590c191 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 29 Oct 2015 14:43:15 +0000 Subject: [PATCH 07/11] Fixed bug in unseen nodes detection --- .../charts/__tests__/node-layout-test.js | 25 +++++++++++++++++++ client/app/scripts/charts/nodes-layout.js | 10 +++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index 078a16958..69aba50a1 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -74,6 +74,31 @@ describe('NodesLayout', () => { }; }); + it('detects unseen nodes', () => { + const set1 = fromJS({ + n1: {id: 'n1'} + }); + const set12 = fromJS({ + n1: {id: 'n1'}, + n2: {id: 'n2'} + }); + const set13 = fromJS({ + n1: {id: 'n1'}, + n3: {id: 'n3'} + }); + let hasUnseen; + hasUnseen = NodesLayout.hasUnseenNodes(set12, set1); + expect(hasUnseen).toBeTruthy(); + hasUnseen = NodesLayout.hasUnseenNodes(set13, set1); + expect(hasUnseen).toBeTruthy(); + hasUnseen = NodesLayout.hasUnseenNodes(set1, set12); + expect(hasUnseen).toBeFalsy(); + hasUnseen = NodesLayout.hasUnseenNodes(set1, set13); + expect(hasUnseen).toBeFalsy(); + hasUnseen = NodesLayout.hasUnseenNodes(set12, set13); + expect(hasUnseen).toBeTruthy(); + }); + it('lays out initial nodeset in a rectangle', () => { const result = NodesLayout.doLayout( nodeSets.initial4.nodes, diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 0dfae60ae..635bfd64f 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -144,9 +144,13 @@ function setSimpleEdgePoints(edge, nodeCache) { * @param {Map} cache old Map of nodes * @return {Boolean} True if nodes had node ids that are not in cache */ -function hasUnseenNodes(nodes, cache) { - return (nodes.size > cache.size - || !ImmSet.fromKeys(nodes).isSubset(ImmSet.fromKeys(nodes))); +export function hasUnseenNodes(nodes, cache) { + const hasUnseen = nodes.size > cache.size + || !ImmSet.fromKeys(nodes).isSubset(ImmSet.fromKeys(cache)); + if (hasUnseen) { + debug('unseen nodes:', ...ImmSet.fromKeys(nodes).subtract(ImmSet.fromKeys(cache)).toJS()); + } + return hasUnseen; } /** From 1831e07224a12712250f11a0fde08bb39053fb87 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 29 Oct 2015 16:05:04 +0000 Subject: [PATCH 08/11] Track trivial layout runs --- client/app/scripts/charts/nodes-layout.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 635bfd64f..312e560e7 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -6,6 +6,8 @@ const Naming = require('../constants/naming'); const MAX_NODES = 100; const topologyCaches = {}; +let layoutRuns = 0; +let layoutRunsTrivial = 0; /** * Layout engine runner @@ -214,8 +216,9 @@ export function doLayout(nodes, edges, opts) { const edgeCache = options.edgeCache || cache.edgeCache; let layout; + ++layoutRuns; if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(nodes, nodeCache)) { - debug('skip layout, trivial adjustment'); + debug('skip layout, trivial adjustment', ++layoutRunsTrivial, layoutRuns); layout = cloneLayout(cachedLayout, nodes, edges); // copy old properties, works also if nodes get re-added layout = copyLayoutProperties(layout, nodeCache, edgeCache); From 6cf93f8e1786d59dc725fe3cc573a9c2b514d30a Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 29 Oct 2015 17:02:45 +0000 Subject: [PATCH 09/11] Prevent dangling reused edge --- client/app/scripts/charts/nodes-layout.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 312e560e7..2039683c0 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -155,6 +155,24 @@ export function hasUnseenNodes(nodes, cache) { return hasUnseen; } +/** + * Determine if edge has same endpoints in new nodes as well as in the nodeCache + * @param {Map} edge Edge with source and target + * @param {Map} nodes new node set + * @param {Map} nodeCache set of previous nodes + * @return {Boolean} True if old and new endpoints have same coordinates + */ +function hasSameEndpoints(edge, nodes, nodeCache) { + const oldSource = nodeCache.get(edge.get('source')); + const oldTarget = nodeCache.get(edge.get('target')); + const newSource = nodes.get(edge.get('source')); + const newTarget = nodes.get(edge.get('target')); + return (oldSource.get('x') === newSource.get('x') + && oldSource.get('y') === newSource.get('y') + && oldTarget.get('x') === newTarget.get('x') + && oldTarget.get('y') === newTarget.get('y')); +} + /** * Clones a previous layout * @param {Object} layout Layout object @@ -180,7 +198,7 @@ function copyLayoutProperties(layout, nodeCache, edgeCache) { return node.merge(nodeCache.get(node.get('id'))); }); layout.edges = layout.edges.map(edge => { - if (edgeCache.has(edge.get('id'))) { + if (edgeCache.has(edge.get('id')) && hasSameEndpoints(edge, layout.nodes, nodeCache)) { return edge.merge(edgeCache.get(edge.get('id'))); } return setSimpleEdgePoints(edge, nodeCache); From f7aad21016d1691f1cc582ae808573b7432f7253 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 29 Oct 2015 17:54:25 +0000 Subject: [PATCH 10/11] Use edge points to determine changed endpoints --- client/app/scripts/charts/nodes-layout.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 2039683c0..c911c744c 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -159,18 +159,18 @@ export function hasUnseenNodes(nodes, cache) { * Determine if edge has same endpoints in new nodes as well as in the nodeCache * @param {Map} edge Edge with source and target * @param {Map} nodes new node set - * @param {Map} nodeCache set of previous nodes * @return {Boolean} True if old and new endpoints have same coordinates */ -function hasSameEndpoints(edge, nodes, nodeCache) { - const oldSource = nodeCache.get(edge.get('source')); - const oldTarget = nodeCache.get(edge.get('target')); - const newSource = nodes.get(edge.get('source')); - const newTarget = nodes.get(edge.get('target')); - return (oldSource.get('x') === newSource.get('x') - && oldSource.get('y') === newSource.get('y') - && oldTarget.get('x') === newTarget.get('x') - && oldTarget.get('y') === newTarget.get('y')); +function hasSameEndpoints(cachedEdge, nodes) { + const oldPoints = cachedEdge.get('points'); + const oldSourcePoint = oldPoints[0]; + const oldTargetPoint = oldPoints[oldPoints.length - 1]; + const newSource = nodes.get(cachedEdge.get('source')); + const newTarget = nodes.get(cachedEdge.get('target')); + return (oldSourcePoint.x === newSource.get('x') + && oldSourcePoint.y === newSource.get('y') + && oldTargetPoint.x === newTarget.get('x') + && oldTargetPoint.y === newTarget.get('y')); } /** @@ -198,7 +198,7 @@ function copyLayoutProperties(layout, nodeCache, edgeCache) { return node.merge(nodeCache.get(node.get('id'))); }); layout.edges = layout.edges.map(edge => { - if (edgeCache.has(edge.get('id')) && hasSameEndpoints(edge, layout.nodes, nodeCache)) { + if (edgeCache.has(edge.get('id')) && hasSameEndpoints(edgeCache.get(edge.get('id')), layout.nodes)) { return edge.merge(edgeCache.get(edge.get('id'))); } return setSimpleEdgePoints(edge, nodeCache); From af175b62bc7ce203e7bdeadc253f3fedee0c4e7f Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 4 Nov 2015 17:20:37 +0100 Subject: [PATCH 11/11] Review feedback --- .../charts/__tests__/node-layout-test.js | 54 +++++++++---------- client/app/scripts/charts/nodes-chart.js | 8 ++- client/app/scripts/charts/nodes-layout.js | 8 +-- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index 69aba50a1..d0a92c974 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -6,18 +6,21 @@ import { fromJS, Map } from 'immutable'; describe('NodesLayout', () => { const NodesLayout = require('../nodes-layout'); - function scale(val) { - return val * 3; + function getNodeCoordinates(nodes) { + const coords = []; + nodes + .sortBy(node => node.get('id')) + .forEach(node => { + coords.push(node.get('x')); + coords.push(node.get('y')); + }); + return coords; } - const topologyId = 'tid'; - const width = 80; - const height = 80; - const margins = { - left: 0, - top: 0 - }; + let options; let nodes; + let coords; + let resultCoords; const nodeSets = { initial4: { @@ -122,6 +125,7 @@ describe('NodesLayout', () => { options.cachedLayout = result; options.nodeCache = options.nodeCache.merge(result.nodes); options.edgeCache = options.edgeCache.merge(result.edge); + coords = getNodeCoordinates(result.nodes); result = NodesLayout.doLayout( nodeSets.removeEdge24.nodes, @@ -131,12 +135,8 @@ describe('NodesLayout', () => { nodes = result.nodes.toJS(); // console.log('remove 1 edge', nodes, result); - expect(nodes.n1.x).toBeLessThan(nodes.n2.x); - expect(nodes.n1.y).toEqual(nodes.n2.y); - expect(nodes.n1.x).toEqual(nodes.n3.x); - expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - expect(nodes.n3.x).toBeLessThan(nodes.n4.x); - expect(nodes.n3.y).toEqual(nodes.n4.y); + resultCoords = getNodeCoordinates(result.nodes); + expect(resultCoords).toEqual(coords); }); it('keeps nodes in rectangle after removed edge reappears', () => { @@ -144,6 +144,7 @@ describe('NodesLayout', () => { nodeSets.initial4.nodes, nodeSets.initial4.edges); + coords = getNodeCoordinates(result.nodes); options.cachedLayout = result; options.nodeCache = options.nodeCache.merge(result.nodes); options.edgeCache = options.edgeCache.merge(result.edge); @@ -165,12 +166,8 @@ describe('NodesLayout', () => { nodes = result.nodes.toJS(); // console.log('re-add 1 edge', nodes, result); - expect(nodes.n1.x).toBeLessThan(nodes.n2.x); - expect(nodes.n1.y).toEqual(nodes.n2.y); - expect(nodes.n1.x).toEqual(nodes.n3.x); - expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - expect(nodes.n3.x).toBeLessThan(nodes.n4.x); - expect(nodes.n3.y).toEqual(nodes.n4.y); + resultCoords = getNodeCoordinates(result.nodes); + expect(resultCoords).toEqual(coords); }); it('keeps nodes in rectangle after node dissappears', () => { @@ -189,10 +186,9 @@ describe('NodesLayout', () => { nodes = result.nodes.toJS(); - expect(nodes.n1.x).toEqual(nodes.n3.x); - expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - expect(nodes.n3.x).toBeLessThan(nodes.n4.x); - expect(nodes.n3.y).toEqual(nodes.n4.y); + resultCoords = getNodeCoordinates(result.nodes); + expect(resultCoords.slice(0,2)).toEqual(coords.slice(0,2)); + expect(resultCoords.slice(2,6)).toEqual(coords.slice(4,8)); }); it('keeps nodes in rectangle after removed node reappears', () => { @@ -202,6 +198,7 @@ describe('NodesLayout', () => { nodes = result.nodes.toJS(); + coords = getNodeCoordinates(result.nodes); options.cachedLayout = result; options.nodeCache = options.nodeCache.merge(result.nodes); options.edgeCache = options.edgeCache.merge(result.edge); @@ -229,10 +226,9 @@ describe('NodesLayout', () => { nodes = result.nodes.toJS(); // console.log('re-add 1 node', nodes); - expect(nodes.n1.x).toEqual(nodes.n3.x); - expect(nodes.n1.y).toBeLessThan(nodes.n3.y); - expect(nodes.n3.x).toBeLessThan(nodes.n4.x); - expect(nodes.n3.y).toEqual(nodes.n4.y); + resultCoords = getNodeCoordinates(result.nodes); + expect(resultCoords.slice(0,2)).toEqual(coords.slice(0,2)); + expect(resultCoords.slice(2,6)).toEqual(coords.slice(4,8)); }); }); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 7b3ad7f34..1d0245e28 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -2,7 +2,6 @@ const _ = require('lodash'); const d3 = require('d3'); const debug = require('debug')('scope:nodes-chart'); const React = require('react'); -const makeList = require('immutable').List; const makeMap = require('immutable').Map; const timely = require('timely'); const Spring = require('react-motion').Spring; @@ -32,7 +31,6 @@ const NodesChart = React.createClass({ return { nodes: makeMap(), edges: makeMap(), - history: makeList(), nodeScale: d3.scale.linear(), shiftTranslate: [0, 0], panTranslate: [0, 0], @@ -177,9 +175,10 @@ const NodesChart = React.createClass({ }, renderMaxNodesError: function(show) { + const errorHint = 'We\u0027re working on it, but for now, try a different view?'; return ( ); }, @@ -455,8 +454,7 @@ const NodesChart = React.createClass({ height: props.height, scale: nodeScale, margins: MARGINS, - topologyId: this.props.topologyId, - history: state.history + topologyId: this.props.topologyId }; const timedLayouter = timely(NodesLayout.doLayout); diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index c911c744c..310192f02 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -246,9 +246,11 @@ export function doLayout(nodes, edges, opts) { } // cache results - cache.cachedLayout = layout; - cache.nodeCache = cache.nodeCache.merge(layout.nodes); - cache.edgeCache = cache.edgeCache.merge(layout.edges); + if (layout) { + cache.cachedLayout = layout; + cache.nodeCache = cache.nodeCache.merge(layout.nodes); + cache.edgeCache = cache.edgeCache.merge(layout.edges); + } return layout; }