From 7c22c97382903280f5ffa39f90b29f064187e1f6 Mon Sep 17 00:00:00 2001
From: Filip Barl <filip.barl@gmail.com>
Date: Tue, 11 Apr 2017 09:11:31 +0200
Subject: [PATCH] Fix node highlight for all shapes (#2430)

* Render highlight with strokes & use non-scaled highlight for stacks.

* Reverted part of the changes made to node stack highlighting.
---
 client/app/scripts/charts/node-shape-stack.js | 30 ++++++++-----------
 client/app/scripts/charts/node-shapes.js      |  9 ++++--
 client/app/styles/_base.scss                  | 24 ++++++++-------
 client/app/styles/_contrast-overrides.scss    |  2 +-
 client/app/styles/_variables.scss             |  2 +-
 5 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/client/app/scripts/charts/node-shape-stack.js b/client/app/scripts/charts/node-shape-stack.js
index 48de1d6b9..d150d5d14 100644
--- a/client/app/scripts/charts/node-shape-stack.js
+++ b/client/app/scripts/charts/node-shape-stack.js
@@ -2,27 +2,23 @@ import React from 'react';
 
 import { NODE_BASE_SIZE } from '../constants/styles';
 
-
 export default function NodeShapeStack(props) {
-  const shift = props.contrastMode ? 0.15 : 0.1;
-  const highlightScale = [1, 1 + shift];
-  const dy = NODE_BASE_SIZE * shift;
-
+  const verticalDistance = NODE_BASE_SIZE * (props.contrastMode ? 0.12 : 0.1);
+  const verticalTranslate = t => `translate(0, ${t * verticalDistance})`;
   const Shape = props.shape;
+
+  // Stack three shapes on top of one another pretending they are never highlighted.
+  // Instead, fake the highlight of the whole stack with a vertically stretched shape
+  // drawn in the background. This seems to give a good approximation of the stack
+  // highlight and prevents us from needing to do some render-heavy SVG clipping magic.
   return (
-    <g transform={`translate(0, ${dy * -2.5})`} className="stack">
-      <g transform={`scale(${highlightScale}) translate(0, ${dy})`} className="highlight">
-        <Shape {...props} />
-      </g>
-      <g transform={`translate(0, ${dy * 2})`}>
-        <Shape {...props} />
-      </g>
-      <g transform={`translate(0, ${dy * 1})`}>
-        <Shape {...props} />
-      </g>
-      <g className="only-metrics">
-        <Shape {...props} />
+    <g transform={verticalTranslate(-2.5)} className="stack">
+      <g transform={`${verticalTranslate(1)} scale(1, 1.14)`}>
+        <Shape className="highlight-only" {...props} />
       </g>
+      <g transform={verticalTranslate(2)}><Shape {...props} highlighted={false} /></g>
+      <g transform={verticalTranslate(1)}><Shape {...props} highlighted={false} /></g>
+      <g transform={verticalTranslate(0)}><Shape {...props} highlighted={false} /></g>
     </g>
   );
 }
diff --git a/client/app/scripts/charts/node-shapes.js b/client/app/scripts/charts/node-shapes.js
index 1e39d4f86..3c421ab7c 100644
--- a/client/app/scripts/charts/node-shapes.js
+++ b/client/app/scripts/charts/node-shapes.js
@@ -28,8 +28,13 @@ function NodeShape(shapeType, shapeElement, shapeProps, { id, highlighted, color
   return (
     <g className={className}>
       {highlighted && shapeElement({
-        className: 'highlight',
-        transform: `scale(${NODE_BASE_SIZE * 0.7})`,
+        className: 'highlight-border',
+        transform: `scale(${NODE_BASE_SIZE * 0.5})`,
+        ...shapeProps,
+      })}
+      {highlighted && shapeElement({
+        className: 'highlight-shadow',
+        transform: `scale(${NODE_BASE_SIZE * 0.5})`,
         ...shapeProps,
       })}
       {shapeElement({
diff --git a/client/app/styles/_base.scss b/client/app/styles/_base.scss
index 33e4b9ee2..a8c68c300 100644
--- a/client/app/styles/_base.scss
+++ b/client/app/styles/_base.scss
@@ -425,15 +425,11 @@
     }
   }
 
-  .stack .shape .highlight {
-    display: none;
-  }
-
-  .stack .highlight .shape  {
-    .border { display: none; }
+  .stack .highlight-only {
+    .background { display: none; }
     .shadow { display: none; }
+    .border { display: none; }
     .node { display: none; }
-    .highlight { display: inline; }
   }
 
   .stack .shape .metric-fill {
@@ -444,14 +440,20 @@
     transform: scale(1);
     cursor: pointer;
 
-    .highlight {
-      fill: $weave-blue;
-      fill-opacity: $node-highlight-fill-opacity;
+    .highlight-border {
+      fill: none;
       stroke: $weave-blue;
-      stroke-width: $node-highlight-stroke-width;
+      stroke-width: 0.7 + $node-highlight-stroke-width * 2;
       stroke-opacity: $node-highlight-stroke-opacity;
     }
 
+    .highlight-shadow {
+      fill: none;
+      stroke: white;
+      stroke-width: 0.7;
+      stroke-opacity: $node-highlight-shadow-opacity;
+    }
+
     .background {
       stroke: none;
       fill: $background-lighter-color;
diff --git a/client/app/styles/_contrast-overrides.scss b/client/app/styles/_contrast-overrides.scss
index 4860c5a5f..310f79166 100644
--- a/client/app/styles/_contrast-overrides.scss
+++ b/client/app/styles/_contrast-overrides.scss
@@ -12,7 +12,7 @@ $text-darker-color: darken($text-color, 20%);
 $white: white;
 $edge-color: black;
 
-$node-highlight-fill-opacity: 0.3;
+$node-highlight-shadow-opacity: 0.4;
 $node-highlight-stroke-opacity: 0.5;
 $node-highlight-stroke-width: 0.16;
 $node-border-stroke-width: 0.2;
diff --git a/client/app/styles/_variables.scss b/client/app/styles/_variables.scss
index 2a9f759dc..7abd7c2ee 100644
--- a/client/app/styles/_variables.scss
+++ b/client/app/styles/_variables.scss
@@ -30,7 +30,7 @@ $border-radius: 4px;
 
 $terminal-header-height: 44px;
 
-$node-highlight-fill-opacity: 0.1;
+$node-highlight-shadow-opacity: 0.5;
 $node-highlight-stroke-opacity: 0.4;
 $node-highlight-stroke-width: 0.04;
 $node-border-stroke-width: 0.12;