From fc95e1efa0bc42768508678d1f745516f0598473 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 6 Sep 2016 17:00:24 +0200 Subject: [PATCH] Fixes selected layout! (was the removal of :id from the node objects) Only show errors in webpack output! Tidying up More tidying more tidying More fiddling around wip wip wip Fixes forceLayout rm console.log --- client/app/scripts/charts/node-container.js | 2 - client/app/scripts/charts/nodes-chart.js | 84 +++++-------------- .../app/scripts/components/debug-toolbar.js | 14 +++- client/app/scripts/components/nodes.js | 2 - .../app/scripts/selectors/chartSelectors.js | 79 +++++++++++------ client/server.js | 2 +- client/test/actions/90-nodes-select.js | 65 +++++++------- client/test/browser-perf/main.js | 2 +- client/test/run-jankie.sh | 14 +++- 9 files changed, 129 insertions(+), 135 deletions(-) diff --git a/client/app/scripts/charts/node-container.js b/client/app/scripts/charts/node-container.js index 9e6b5fb66..74820303d 100644 --- a/client/app/scripts/charts/node-container.js +++ b/client/app/scripts/charts/node-container.js @@ -13,8 +13,6 @@ class NodeContainer extends React.Component { const scaleFactor = focused ? (1 / zoomScale) : 1; const other = _.omit(this.props, 'dx', 'dy'); - console.log('nodecontainer.render'); - return ( is(v1, v2) ? v1 : v2, a, b); -} - - -function getLayoutNodes(nodes) { - return nodes.map(n => makeMap({ - id: n.get('id'), - adjacency: n.get('adjacency'), - })); -} - - function initEdges(nodes) { let edges = makeMap(); @@ -94,7 +74,6 @@ function initEdges(nodes) { function getNodeScale(nodesCount, width, height) { - console.log(nodesCount, width, height); const expanse = Math.min(height, width); const nodeSize = expanse / 3; // single node should fill a third of the screen const maxNodeSize = Math.min(MAX_NODE_SIZE, expanse / 10); @@ -105,20 +84,13 @@ function getNodeScale(nodesCount, width, height) { } -function updateLayout({width, height, margins, topologyId, topologyOptions, forceRelayout, - nodes }) { +function updateLayout(width, height, nodes, baseOptions) { const nodeScale = getNodeScale(nodes.size, width, height); const edges = initEdges(nodes); - const options = { - width, - height, - margins: margins.toJS(), - forceRelayout, - topologyId, - topologyOptions: (topologyOptions && topologyOptions.toJS()), + const options = Object.assign({}, baseOptions, { scale: nodeScale, - }; + }); const timedLayouter = timely(doLayout); const graph = timedLayouter(nodes, edges, options); @@ -136,7 +108,7 @@ function updateLayout({width, height, margins, topologyId, topologyOptions, forc const layoutEdges = graph.edges .map(edge => edge.set('ppoints', edge.get('points'))); - return { layoutNodes, layoutEdges, width: graph.width, height: graph.height }; + return { layoutNodes, layoutEdges, layoutWidth: graph.width, layoutHeight: graph.height }; } @@ -160,9 +132,6 @@ class NodesChart extends React.Component { height: props.height || 0, width: props.width || 0, zoomCache: {}, - - layoutInput: makeMap(), - layoutNodes: makeMap(), }; } @@ -172,7 +141,6 @@ class NodesChart extends React.Component { } componentWillReceiveProps(nextProps) { - console.log('componentWillReceiveProps', diff(nextProps, this.props), nextProps); // gather state, setState should be called only once here const state = _.assign({}, this.state); @@ -202,7 +170,9 @@ class NodesChart extends React.Component { state.height = nextProps.forceRelayout ? nextProps.height : (state.height || nextProps.height); state.width = nextProps.forceRelayout ? nextProps.width : (state.width || nextProps.width); - _.assign(state, this.updateGraphState(nextProps, state)); + if (nextProps.forceRelayout || nextProps.nodes !== this.props.nodes) { + _.assign(state, this.updateGraphState(nextProps, state)); + } if (this.props.selectedNodeId !== nextProps.selectedNodeId) { _.assign(state, this.restoreLayout(state)); @@ -243,7 +213,6 @@ class NodesChart extends React.Component { const translate = [panTranslateX, panTranslateY]; const transform = `translate(${translate}) scale(${scale})`; const svgClassNames = this.props.isEmpty ? 'hide' : ''; - console.log('nodes-chart.render'); const layoutPrecision = getLayoutPrecision(nodes.size); return ( @@ -277,9 +246,7 @@ class NodesChart extends React.Component { centerSelectedNode(props, state) { let stateNodes = state.nodes; let stateEdges = state.edges; - const selectedLayoutNode = stateNodes.get(props.selectedNodeId); - - if (!selectedLayoutNode) { + if (!stateNodes.has(props.selectedNodeId)) { return {}; } @@ -310,8 +277,8 @@ class NodesChart extends React.Component { const radius = Math.min(state.width, state.height) / density / zoomScale; const offsetAngle = Math.PI / 4; - stateNodes = stateNodes.map((node) => { - const index = adjacentLayoutNodeIds.indexOf(node.get('id')); + stateNodes = stateNodes.map((node, nodeId) => { + const index = adjacentLayoutNodeIds.indexOf(nodeId); if (index > -1) { const angle = offsetAngle + Math.PI * 2 * index / adjacentCount; return node.merge({ @@ -324,8 +291,8 @@ class NodesChart extends React.Component { // fix all edges for circular nodes stateEdges = stateEdges.map(edge => { - if (edge.get('source') === selectedLayoutNode.get('id') - || edge.get('target') === selectedLayoutNode.get('id') + if (edge.get('source') === props.selectedNodeId + || edge.get('target') === props.selectedNodeId || _.includes(adjacentLayoutNodeIds, edge.get('source')) || _.includes(adjacentLayoutNodeIds, edge.get('target'))) { const source = stateNodes.get(edge.get('source')); @@ -376,27 +343,21 @@ class NodesChart extends React.Component { }; } - const layoutInput = identityPresevingMerge(state.layoutInput, { + const options = { width: state.width, height: state.height, - nodes: getLayoutNodes(props.nodes), - margins: fromJS(props.margins), - topologyId: props.topologyId, - topologyOptions: fromJS(props.topologyOptions), + margins: props.margins, forceRelayout: props.forceRelayout, - }); + topologyId: props.topologyId, + topologyOptions: props.topologyOptions, + }; - // layout input hasn't changed. - // TODO: move this out into reselect (relies on `state` a bit right now which makes it tricky) - if (state.layoutInput === layoutInput) { - return {}; - } - - const { layoutNodes, layoutEdges, width, height } = updateLayout(layoutInput.toObject()); + const { layoutNodes, layoutEdges, layoutWidth, layoutHeight } = updateLayout( + state.width, state.height, props.nodes, options); // // adjust layout based on viewport - const xFactor = (state.width - props.margins.left - props.margins.right) / width; - const yFactor = state.height / height; + const xFactor = (state.width - props.margins.left - props.margins.right) / layoutWidth; + const yFactor = state.height / layoutHeight; const zoomFactor = Math.min(xFactor, yFactor); let zoomScale = state.scale; @@ -407,7 +368,6 @@ class NodesChart extends React.Component { } return { - layoutInput, scale: zoomScale, nodes: layoutNodes, edges: layoutEdges, diff --git a/client/app/scripts/components/debug-toolbar.js b/client/app/scripts/components/debug-toolbar.js index ae61fff59..e1574574b 100644 --- a/client/app/scripts/components/debug-toolbar.js +++ b/client/app/scripts/components/debug-toolbar.js @@ -178,7 +178,7 @@ class DebugToolbar extends React.Component { } setLoading(loading) { - this.props.setAppState(state => state.set('topologiesLoaded', !loading)); + this.props.dispatch(setAppState(state => state.set('topologiesLoaded', !loading))); } addNodes(n, prefix = 'zing') { @@ -204,6 +204,14 @@ class DebugToolbar extends React.Component { log('added nodes', n); } + removeNode() { + const ns = this.props.nodes; + const nodeNames = ns.keySeq().toJS(); + this.props.dispatch(receiveNodesDelta({ + remove: [nodeNames[_.random(nodeNames.length - 1)]] + })); + } + render() { const { availableCanvasMetrics } = this.props; @@ -220,6 +228,7 @@ class DebugToolbar extends React.Component { Metric Variants +
@@ -289,6 +298,5 @@ function mapStateToProps(state) { export default connect( - mapStateToProps, - {setAppState} + mapStateToProps )(DebugToolbar); diff --git a/client/app/scripts/components/nodes.js b/client/app/scripts/components/nodes.js index 385d64910..20490c321 100644 --- a/client/app/scripts/components/nodes.js +++ b/client/app/scripts/components/nodes.js @@ -52,8 +52,6 @@ class Nodes extends React.Component { const { topologyEmpty, gridMode, topologiesLoaded, nodesLoaded, topologies, currentTopology } = this.props; - console.log('nodes.render'); - return (
diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 9d730c96a..0d5456ae0 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -4,14 +4,18 @@ import { Map as makeMap, is } from 'immutable'; import { getAdjacentNodes } from '../utils/topology-utils'; -// imm createSelector // -const imCreateSelector = createSelectorCreator( +// "immutable" createSelector +// +const createDeepEqualSelector = createSelectorCreator( defaultMemoize, is ); +const identity = v => v; + + const allNodesSelector = state => state.get('nodes'); @@ -21,36 +25,46 @@ export const nodesSelector = createSelector( ); -export const _nodeAdjacenciesSelector = createSelector( - nodesSelector, - (nodes) => nodes.map(n => makeMap({ - id: n.get('id'), - adjacency: n.get('adjacency'), - })) -); - - -export const nodeAdjacenciesSelector = imCreateSelector( - _nodeAdjacenciesSelector, - (nodes) => nodes -); - - -const _adjacentNodesSelector = createSelector( +// +// This is like an === cache... +// +// - getAdjacentNodes is run on every state change and can generate a new immutable object each +// time: +// - v1 = getAdjacentNodes(a) +// - v2 = getAdjacentNodes(a) +// - v1 !== v2 +// - is(v1, v2) === true +// +// - createDeepEqualSelector will wrap those calls with a: is(v1, v2) ? v1 : v2 +// - Thus you can compare consequtive calls to adjacentNodesSelector(state) with === (which is +// what redux is doing with connect() +// +// Note: this feels like the wrong way to be using reselect... +// +export const adjacentNodesSelector = createDeepEqualSelector( getAdjacentNodes, - (ns) => ns + identity ); -export const adjacentNodesSelector = imCreateSelector( - _adjacentNodesSelector, - (adjacentNodes) => adjacentNodes +// +// You what? What is going on here? +// +// We wrap the result of nodes.map in another equality test which discards the new value +// if it was the same as the old one. Again preserving === +// +export const nodeAdjacenciesSelector = createDeepEqualSelector( + createSelector( + nodesSelector, + (nodes) => nodes.map(n => makeMap({ + id: n.get('id'), + adjacency: n.get('adjacency'), + })) + ), + identity ); -export const layoutNodesSelector = (_, props) => props.layoutNodes; - - export const dataNodesSelector = createSelector( nodesSelector, (nodes) => nodes.map((node, id) => makeMap({ @@ -68,6 +82,10 @@ export const dataNodesSelector = createSelector( ); +// FIXME: this is a bit of a hack... +export const layoutNodesSelector = (_, props) => props.layoutNodes || makeMap(); + + function mergeDeepIfExists(mapA, mapB) { // // Does a deep merge on any key that exists in the first map @@ -76,14 +94,21 @@ function mergeDeepIfExists(mapA, mapB) { } -export const completeNodesSelector = createSelector( +const _completeNodesSelector = createSelector( layoutNodesSelector, dataNodesSelector, (layoutNodes, dataNodes) => { - if (!layoutNodes || layoutNodes.size === 0) { + if (layoutNodes.size === 0 || dataNodes.size === 0) { return makeMap(); } + // dataNodes might get updated before layoutNodes when a node is removed from the topo. return mergeDeepIfExists(dataNodes, layoutNodes); } ); + + +export const completeNodesSelector = createDeepEqualSelector( + _completeNodesSelector, + identity +); diff --git a/client/server.js b/client/server.js index a27ac30c7..3b984ba1f 100644 --- a/client/server.js +++ b/client/server.js @@ -66,7 +66,7 @@ if (process.env.NODE_ENV !== 'production') { hot: true, noInfo: true, historyApiFallback: true, - stats: { colors: true } + stats: 'errors-only', }).listen(4041, '0.0.0.0', function (err, result) { if (err) { console.log(err); diff --git a/client/test/actions/90-nodes-select.js b/client/test/actions/90-nodes-select.js index ed7c28313..5693ae01b 100644 --- a/client/test/actions/90-nodes-select.js +++ b/client/test/actions/90-nodes-select.js @@ -18,85 +18,84 @@ function clickIfVisible(list, index) { }); } + +function selectNode(browser) { + debug('select node'); + return browser.elementByCssSelector('.nodes-chart-nodes .node:nth-child(1) > g', function(err, el) { + return el.click(); + }); +} + + +function deselectNode(browser) { + debug('deselect node'); + return browser.elementByCssSelector('.fa-close', function(err, el) { + return el.click(); + }); +} + + module.exports = function(cfg) { - var startUrl = 'http://' + cfg.host + '/debug.html'; - var selectedUrl = 'http://' + cfg.host + '/debug.html#!/state/{"nodeDetails":[{"id":"zing11","label":"zing11","topologyId":"containers"}],"selectedNodeId":"zing11","topologyId":"containers","topologyOptions":{"processes":{"unconnected":"hide"},"processes-by-name":{"unconnected":"hide"},"containers":{"system":"hide","stopped":"hide"},"containers-by-hostname":{"system":"hide","stopped":"hide"},"containers-by-image":{"system":"hide","stopped":"hide"}}}'; - + var startUrl = 'http://' + cfg.host + '/'; // cfg - The configuration object. args, from the example above. return function(browser) { // browser is created using wd.promiseRemote() // More info about wd at https://github.com/admc/wd - return browser.get('http://' + cfg.host + '/debug.html') + return browser.get('http://' + cfg.host + '/') .then(function() { debug('starting run ' + cfg.run); return browser.sleep(2000); }) + .then(function() { + return browser.execute("localStorage.debugToolbar = 1;"); + }) + .then(function() { + return browser.sleep(5000); + }) .then(function() { return browser.elementByCssSelector('.debug-panel button:nth-child(5)'); // return browser.elementByCssSelector('.debug-panel div:nth-child(2) button:nth-child(9)'); }) .then(function(el) { debug('debug-panel found'); - return el.click(function() { - el.click(function() { - el.click(); - }); - }); + return el.click(); }) .then(function() { return browser.sleep(2000); }) - .then(function() { - return browser.sleep(2000); - }) - .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) - .then(function() { return browser.sleep(2000); }) .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) - .then(function() { return browser.sleep(2000); }) .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) .then(function() { diff --git a/client/test/browser-perf/main.js b/client/test/browser-perf/main.js index 039c6d3ad..8c042556e 100644 --- a/client/test/browser-perf/main.js +++ b/client/test/browser-perf/main.js @@ -3,7 +3,7 @@ var options = { selenium: 'http://local.docker:4444/wd/hub', actions: [require('./custom-action.js')()] } -browserPerf('http://local.docker:4040/debug.html', function(err, res){ +browserPerf('http://local.docker:4040/dev.html', function(err, res){ console.error(err); console.log(res); }, options); diff --git a/client/test/run-jankie.sh b/client/test/run-jankie.sh index 2c3f16b2c..005c8444a 100755 --- a/client/test/run-jankie.sh +++ b/client/test/run-jankie.sh @@ -10,6 +10,12 @@ # # perfjankie --only-update-site --couch-server http://local.docker:5984 --couch-database performance # +# Optional: run from localhost which can be a bit fast than rebuilding... +# - ssh -R 0.0.0.0:4042:localhost:4042 docker@local.docker +# - npm run build +# - BACKEND_HOST=local.docker npm start +# - ./run-jankie.sh 192.168.64.3:4042 +# # Usage: # # ./run-jankie.sh 192.168.64.3:4040 @@ -26,9 +32,9 @@ COMMIT=$(git log --format="%h" -1) echo "Testing $COMMIT on $DATE" -../../scope stop -make SUDO= -C ../.. -../../scope launch -sleep 5 +# ../../scope stop +# make SUDO= -C ../.. +# ../../scope launch +# sleep 5 COMMIT="$COMMIT" DATE=$DATE HOST=$HOST DEBUG=scope* node ./perfjankie/main.js