Merge pull request #2851 from weaveworks/2850-fix-graph-edge-path-undefined

Fix edges disappearing in graph mode

client/app/scripts/charts/edge-container.js

@@ -1,11 +1,11 @@
 import React from 'react';
-import { Motion, spring } from 'react-motion';
-import { Map as makeMap } from 'immutable';
+import { Motion } from 'react-motion';
+import { Repeat, fromJS, Map as makeMap } from 'immutable';
 import { line, curveBasis } from 'd3-shape';
-import { each, times, constant } from 'lodash';
+import { times } from 'lodash';
 
-import { NODES_SPRING_ANIMATION_CONFIG } from '../constants/animation';
 import { NODE_BASE_SIZE, EDGE_WAYPOINTS_CAP } from '../constants/styles';
+import { weakSpring } from '../utils/animation-utils';
 import Edge from './edge';
 
 
@@ -23,7 +23,7 @@ const transformedEdge = (props, path, thickness) => (
 // [{ x: 11, y: 22 }, { x: 33, y: 44 }] that can be used by D3.
 const waypointsMapToArray = (waypointsMap) => {
   const waypointsArray = times(EDGE_WAYPOINTS_CAP, () => ({}));
-  each(waypointsMap, (value, key) => {
+  waypointsMap.forEach((value, key) => {
     const [axis, index] = [key[0], key.slice(1)];
     waypointsArray[index][axis] = value;
   });
@@ -35,8 +35,8 @@ const waypointsMapToArray = (waypointsMap) => {
 const waypointsArrayToMap = (waypointsArray) => {
   let waypointsMap = makeMap();
   waypointsArray.forEach((point, index) => {
-    waypointsMap = waypointsMap.set(`x${index}`, spring(point.x, NODES_SPRING_ANIMATION_CONFIG));
-    waypointsMap = waypointsMap.set(`y${index}`, spring(point.y, NODES_SPRING_ANIMATION_CONFIG));
+    waypointsMap = waypointsMap.set(`x${index}`, weakSpring(point.get('x')));
+    waypointsMap = waypointsMap.set(`y${index}`, weakSpring(point.get('y')));
   });
   return waypointsMap;
 };
@@ -53,15 +53,15 @@ export default class EdgeContainer extends React.PureComponent {
   }
 
   componentWillMount() {
-    if (this.props.isAnimated) {
-      this.prepareWaypointsForMotion(this.props.waypoints);
-    }
+    this.prepareWaypointsForMotion(this.props);
   }
 
   componentWillReceiveProps(nextProps) {
     // immutablejs allows us to `===`! \o/
-    if (this.props.isAnimated && nextProps.waypoints !== this.props.waypoints) {
-      this.prepareWaypointsForMotion(nextProps.waypoints);
+    const waypointsChanged = this.props.waypoints !== nextProps.waypoints;
+    const animationChanged = this.props.isAnimated !== nextProps.isAnimated;
+    if (waypointsChanged || animationChanged) {
+      this.prepareWaypointsForMotion(nextProps);
     }
     // Edge thickness will reflect the zoom scale.
     const baseScale = (nextProps.scale * 0.01) * NODE_BASE_SIZE;
@@ -71,36 +71,33 @@ export default class EdgeContainer extends React.PureComponent {
 
   render() {
     const { isAnimated, waypoints, scale, ...forwardedProps } = this.props;
+    const { thickness, waypointsMap } = this.state;
 
     if (!isAnimated) {
-      return transformedEdge(forwardedProps, waypoints.toJS(), this.state.thickness);
+      return transformedEdge(forwardedProps, waypoints.toJS(), thickness);
     }
 
     return (
       // For the Motion interpolation to work, the waypoints need to be in a map format like
       // { x0: 11, y0: 22, x1: 33, y1: 44 } that we convert to the array format when rendering.
-      <Motion
-        style={{
-          thickness: spring(this.state.thickness, NODES_SPRING_ANIMATION_CONFIG),
-          ...this.state.waypointsMap.toJS(),
-        }}
-      >
-        {({ thickness, ...interpolatedWaypoints}) => transformedEdge(
-          forwardedProps, waypointsMapToArray(interpolatedWaypoints), thickness
+      <Motion style={{ interpolatedThickness: weakSpring(thickness), ...waypointsMap.toJS() }}>
+        {({ interpolatedThickness, ...interpolatedWaypoints}) => transformedEdge(
+          forwardedProps, waypointsMapToArray(fromJS(interpolatedWaypoints)), interpolatedThickness
         )}
       </Motion>
     );
   }
 
-  prepareWaypointsForMotion(waypoints) {
-    waypoints = waypoints.toJS();
+  prepareWaypointsForMotion({ waypoints, isAnimated }) {
+    // Don't update if the edges are not animated.
+    if (!isAnimated) return;
 
     // The Motion library requires the number of waypoints to be constant, so we fill in for
     // the missing ones by reusing the edge source point, which doesn't affect the edge shape
     // because of how the curveBasis interpolation is done.
-    const waypointsMissing = EDGE_WAYPOINTS_CAP - waypoints.length;
+    const waypointsMissing = EDGE_WAYPOINTS_CAP - waypoints.size;
     if (waypointsMissing > 0) {
-      waypoints = times(waypointsMissing, constant(waypoints[0])).concat(waypoints);
+      waypoints = Repeat(waypoints.get(0), waypointsMissing).concat(waypoints);
     }
 
     this.setState({ waypointsMap: waypointsArrayToMap(waypoints) });

client/app/scripts/charts/node-container.js

@@ -1,13 +1,11 @@
 import React from 'react';
-import { Motion, spring } from 'react-motion';
+import { Motion } from 'react-motion';
 
-import { NODES_SPRING_ANIMATION_CONFIG } from '../constants/animation';
+import { weakSpring } from '../utils/animation-utils';
 import Node from './node';
 
 
 const transformedNode = (otherProps, { x, y, k }) => (
-  // NOTE: Controlling blurring and transform from here seems to re-render
-  // faster than adding a CSS class and controlling it from there.
   <g transform={`translate(${x},${y}) scale(${k})`}>
     <Node {...otherProps} />
   </g>
@@ -24,12 +22,7 @@ export default class NodeContainer extends React.PureComponent {
 
     return (
       // Animate the node if the layout is sufficiently small
-      <Motion
-        style={{
-          x: spring(dx, NODES_SPRING_ANIMATION_CONFIG),
-          y: spring(dy, NODES_SPRING_ANIMATION_CONFIG),
-          k: spring(scale, NODES_SPRING_ANIMATION_CONFIG),
-        }}>
+      <Motion style={{ x: weakSpring(dx), y: weakSpring(dy), k: weakSpring(scale) }}>
         {interpolated => transformedNode(forwardedProps, interpolated)}
       </Motion>
     );

client/app/scripts/components/time-travel-timeline.js

@@ -6,9 +6,10 @@ import { connect } from 'react-redux';
 import { drag } from 'd3-drag';
 import { scaleUtc } from 'd3-scale';
 import { event as d3Event, select } from 'd3-selection';
-import { Motion, spring } from 'react-motion';
+import { Motion } from 'react-motion';
 
 import { zoomFactor } from '../utils/zoom-utils';
+import { strongSpring } from '../utils/animation-utils';
 import { linearGradientValue } from '../utils/math-utils';
 import { trackMixpanelEvent } from '../utils/tracking-utils';
 import {
@@ -17,7 +18,6 @@ import {
   scaleDuration,
 } from '../utils/time-utils';
 
-import { NODES_SPRING_FAST_ANIMATION_CONFIG } from '../constants/animation';
 import { TIMELINE_TICK_INTERVAL, ZOOM_TRACK_DEBOUNCE_INTERVAL } from '../constants/timer';
 
 
@@ -359,8 +359,8 @@ class TimeTravelTimeline extends React.Component {
     return (
       <Motion
         style={{
-          focusedTimestampValue: spring(focusedTimestampValue, NODES_SPRING_FAST_ANIMATION_CONFIG),
-          durationPerPixelValue: spring(durationPerPixelValue, NODES_SPRING_FAST_ANIMATION_CONFIG),
+          focusedTimestampValue: strongSpring(focusedTimestampValue),
+          durationPerPixelValue: strongSpring(durationPerPixelValue),
         }}>
         {interpolated => this.renderAxis({
           focusedTimestamp: moment(interpolated.focusedTimestampValue),

client/app/scripts/constants/animation.js

@@ -1,3 +0,0 @@
-
-export const NODES_SPRING_ANIMATION_CONFIG = { stiffness: 100, damping: 18, precision: 1 };
-export const NODES_SPRING_FAST_ANIMATION_CONFIG = { stiffness: 800, damping: 50, precision: 1 };

client/app/scripts/utils/animation-utils.js

@@ -0,0 +1,10 @@
+import { spring } from 'react-motion';
+
+
+export function weakSpring(value) {
+  return spring(value, { stiffness: 100, damping: 18, precision: 1 });
+}
+
+export function strongSpring(value) {
+  return spring(value, { stiffness: 800, damping: 50, precision: 1 });
+}