From a8bc2d64632892efc4ed0f92b1adc14c12abe07b Mon Sep 17 00:00:00 2001
From: David Kaltschmidt <david.kaltschmidt@gmail.com>
Date: Wed, 8 Jun 2016 15:46:36 +0200
Subject: [PATCH] Separate node colors

Node colors are procedurally generated by a node's rank. This can be a
string like `weaveworks/lb-1`. Now if all the nodes share the same
prefix in their rank, e.g. `weaveworks/` they will all get the same
color.

An approach to separate colors a bit more would be to remove the longest
common rank prefix and set that as the color key.

Pros:

- colors are better separation for homogeneous rank values

Cons:

- if 99 nodes share a prefix and one does not, the 99 will be colored
  similarly (which is what happens now)
- if that one node appears and dissappears, the 99 nodes' colors will
  switch based on their new LCP
- Loading details panels from other topologies may yield a different
  color because that node is loaded in isolation (LCP of that
topology's nodes is undetermined)
---
 client/app/scripts/charts/node.js              | 6 +++---
 client/app/scripts/charts/nodes-chart-nodes.js | 1 +
 client/app/scripts/charts/nodes-chart.js       | 1 +
 client/app/scripts/reducers/root.js            | 9 +++++++++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/client/app/scripts/charts/node.js b/client/app/scripts/charts/node.js
index 2ff1b47c1b..e286646d29 100644
--- a/client/app/scripts/charts/node.js
+++ b/client/app/scripts/charts/node.js
@@ -76,13 +76,13 @@ class Node extends React.Component {
   }
 
   render() {
-    const { blurred, focused, highlighted, label, matches = makeMap(), networks,
-      pseudo, rank, subLabel, scaleFactor, transform, zoomScale, exportingGraph,
+    const { blurred, colorKey, focused, highlighted, label, matches = makeMap(), networks,
+      pseudo, subLabel, scaleFactor, transform, zoomScale, exportingGraph,
       showingNetworks, stack } = this.props;
     const { hovered, matched } = this.state;
     const nodeScale = focused ? this.props.selectedNodeScale : this.props.nodeScale;
 
-    const color = getNodeColor(rank, label, pseudo);
+    const color = getNodeColor(colorKey, label, pseudo);
     const truncate = !focused && !hovered;
     const labelTransform = focused ? `scale(${1 / zoomScale})` : '';
     const labelWidth = nodeScale(scaleFactor * 4);
diff --git a/client/app/scripts/charts/nodes-chart-nodes.js b/client/app/scripts/charts/nodes-chart-nodes.js
index d91bb3a5ba..85df5c23a5 100644
--- a/client/app/scripts/charts/nodes-chart-nodes.js
+++ b/client/app/scripts/charts/nodes-chart-nodes.js
@@ -74,6 +74,7 @@ class NodesChartNodes extends React.Component {
           nodeCount={node.get('nodeCount')}
           subLabel={node.get('subLabel')}
           metric={metric(node)}
+          colorKey={node.get('colorKey')}
           rank={node.get('rank')}
           layoutPrecision={layoutPrecision}
           selectedNodeScale={selectedNodeScale}
diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js
index 29bb8eb940..828bf07705 100644
--- a/client/app/scripts/charts/nodes-chart.js
+++ b/client/app/scripts/charts/nodes-chart.js
@@ -171,6 +171,7 @@ class NodesChart extends React.Component {
     topology.forEach((node, id) => {
       nextStateNodes = nextStateNodes.mergeIn([id], makeMap({
         id,
+        colorKey: node.get('colorKey'),
         label: node.get('label'),
         pseudo: node.get('pseudo'),
         subLabel: node.get('label_minor'),
diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js
index 58d9c0ae63..c6a2d06c40 100644
--- a/client/app/scripts/reducers/root.js
+++ b/client/app/scripts/reducers/root.js
@@ -521,6 +521,15 @@ export function rootReducer(state = initialState, action) {
       // apply pinned searches, filters nodes that dont match
       state = applyPinnedSearches(state);
 
+      // optimize color coding for nodes
+      const nodeRankPrefix = longestCommonPrefix(state.get('nodes')
+        .valueSeq()
+        .map(n => n.get('rank')).toJS());
+
+      state = state.update('nodes',
+        nodes => nodes.map(node => node.set('colorKey',
+          nodeRankPrefix ? node.get('rank').substr(nodeRankPrefix.length) : node.get('rank'))));
+
       // TODO move this setting of networks as toplevel node field to backend,
       // to not rely on field IDs here. should be determined by topology implementer
       state = state.update('nodes', nodes => nodes.map(node => {