From 736770c978a91a5c93a864873e07bc73622cc5aa Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Mon, 11 Sep 2017 18:10:42 +0200 Subject: [PATCH 1/3] Fix edges disappearing. --- client/app/scripts/charts/edge-container.js | 28 ++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/client/app/scripts/charts/edge-container.js b/client/app/scripts/charts/edge-container.js index 981e8b0c4..9e99df19e 100644 --- a/client/app/scripts/charts/edge-container.js +++ b/client/app/scripts/charts/edge-container.js @@ -1,8 +1,8 @@ import React from 'react'; import { Motion, spring } from 'react-motion'; -import { Map as makeMap } from 'immutable'; +import { fromJS, Map as makeMap } from 'immutable'; import { line, curveBasis } from 'd3-shape'; -import { each, times, constant } from 'lodash'; +import { times, constant } from 'lodash'; import { NODES_SPRING_ANIMATION_CONFIG } from '../constants/animation'; import { NODE_BASE_SIZE, EDGE_WAYPOINTS_CAP } from '../constants/styles'; @@ -15,7 +15,7 @@ const spline = line() .y(d => d.y); const transformedEdge = (props, path, thickness) => ( - + ); // Converts a waypoints map of the format { x0: 11, y0: 22, x1: 33, y1: 44 } @@ -23,11 +23,11 @@ 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; }); - return waypointsArray; + return fromJS(waypointsArray); }; // Converts a waypoints array of the input format [{ x: 11, y: 22 }, { x: 33, y: 44 }] @@ -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}`, spring(point.get('x'), NODES_SPRING_ANIMATION_CONFIG)); + waypointsMap = waypointsMap.set(`y${index}`, spring(point.get('y'), NODES_SPRING_ANIMATION_CONFIG)); }); return waypointsMap; }; @@ -53,14 +53,14 @@ export default class EdgeContainer extends React.PureComponent { } componentWillMount() { - if (this.props.isAnimated) { - this.prepareWaypointsForMotion(this.props.waypoints); - } + // if (this.props.isAnimated) { + this.prepareWaypointsForMotion(this.props.waypoints); + // } } componentWillReceiveProps(nextProps) { // immutablejs allows us to `===`! \o/ - if (this.props.isAnimated && nextProps.waypoints !== this.props.waypoints) { + if (nextProps.waypoints !== this.props.waypoints) { this.prepareWaypointsForMotion(nextProps.waypoints); } // Edge thickness will reflect the zoom scale. @@ -73,7 +73,7 @@ export default class EdgeContainer extends React.PureComponent { const { isAnimated, waypoints, scale, ...forwardedProps } = this.props; if (!isAnimated) { - return transformedEdge(forwardedProps, waypoints.toJS(), this.state.thickness); + return transformedEdge(forwardedProps, waypoints, this.state.thickness); } return ( @@ -86,7 +86,7 @@ export default class EdgeContainer extends React.PureComponent { }} > {({ thickness, ...interpolatedWaypoints}) => transformedEdge( - forwardedProps, waypointsMapToArray(interpolatedWaypoints), thickness + forwardedProps, waypointsMapToArray(fromJS(interpolatedWaypoints)), thickness )} ); @@ -103,6 +103,6 @@ export default class EdgeContainer extends React.PureComponent { waypoints = times(waypointsMissing, constant(waypoints[0])).concat(waypoints); } - this.setState({ waypointsMap: waypointsArrayToMap(waypoints) }); + this.setState({ waypointsMap: waypointsArrayToMap(fromJS(waypoints)) }); } } From 75e0d383fed264954ac5eeb639baa0fd6858bbd6 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Mon, 11 Sep 2017 18:50:23 +0200 Subject: [PATCH 2/3] Simplified spring usage. --- client/app/scripts/charts/edge-container.js | 10 +++++----- client/app/scripts/charts/node-container.js | 11 +++-------- client/app/scripts/components/time-travel-timeline.js | 8 ++++---- client/app/scripts/constants/animation.js | 3 --- client/app/scripts/utils/animation-utils.js | 10 ++++++++++ 5 files changed, 22 insertions(+), 20 deletions(-) delete mode 100644 client/app/scripts/constants/animation.js create mode 100644 client/app/scripts/utils/animation-utils.js diff --git a/client/app/scripts/charts/edge-container.js b/client/app/scripts/charts/edge-container.js index 9e99df19e..ce06215fc 100644 --- a/client/app/scripts/charts/edge-container.js +++ b/client/app/scripts/charts/edge-container.js @@ -1,11 +1,11 @@ import React from 'react'; -import { Motion, spring } from 'react-motion'; +import { Motion } from 'react-motion'; import { fromJS, Map as makeMap } from 'immutable'; import { line, curveBasis } from 'd3-shape'; import { times, constant } 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'; @@ -35,8 +35,8 @@ const waypointsMapToArray = (waypointsMap) => { const waypointsArrayToMap = (waypointsArray) => { let waypointsMap = makeMap(); waypointsArray.forEach((point, index) => { - waypointsMap = waypointsMap.set(`x${index}`, spring(point.get('x'), NODES_SPRING_ANIMATION_CONFIG)); - waypointsMap = waypointsMap.set(`y${index}`, spring(point.get('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; }; @@ -81,7 +81,7 @@ export default class EdgeContainer extends React.PureComponent { // { x0: 11, y0: 22, x1: 33, y1: 44 } that we convert to the array format when rendering. diff --git a/client/app/scripts/charts/node-container.js b/client/app/scripts/charts/node-container.js index 0851035b7..b613c680c 100644 --- a/client/app/scripts/charts/node-container.js +++ b/client/app/scripts/charts/node-container.js @@ -1,7 +1,7 @@ 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'; @@ -24,12 +24,7 @@ export default class NodeContainer extends React.PureComponent { return ( // Animate the node if the layout is sufficiently small - + {interpolated => transformedNode(forwardedProps, interpolated)} ); diff --git a/client/app/scripts/components/time-travel-timeline.js b/client/app/scripts/components/time-travel-timeline.js index fc427fbb4..c8abe83a5 100644 --- a/client/app/scripts/components/time-travel-timeline.js +++ b/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 ( {interpolated => this.renderAxis({ focusedTimestamp: moment(interpolated.focusedTimestampValue), diff --git a/client/app/scripts/constants/animation.js b/client/app/scripts/constants/animation.js deleted file mode 100644 index 765fa258d..000000000 --- a/client/app/scripts/constants/animation.js +++ /dev/null @@ -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 }; diff --git a/client/app/scripts/utils/animation-utils.js b/client/app/scripts/utils/animation-utils.js new file mode 100644 index 000000000..24498154c --- /dev/null +++ b/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 }); +} From 687d06193945c99ceb7f33666684a4225b37ed60 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Tue, 12 Sep 2017 12:16:30 +0200 Subject: [PATCH 3/3] Cleanup and revert to optimized waypoints recalculation. --- client/app/scripts/charts/edge-container.js | 43 ++++++++++----------- client/app/scripts/charts/node-container.js | 2 - 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/client/app/scripts/charts/edge-container.js b/client/app/scripts/charts/edge-container.js index ce06215fc..82f3a7a9f 100644 --- a/client/app/scripts/charts/edge-container.js +++ b/client/app/scripts/charts/edge-container.js @@ -1,8 +1,8 @@ import React from 'react'; import { Motion } from 'react-motion'; -import { fromJS, Map as makeMap } from 'immutable'; +import { Repeat, fromJS, Map as makeMap } from 'immutable'; import { line, curveBasis } from 'd3-shape'; -import { times, constant } from 'lodash'; +import { times } from 'lodash'; import { NODE_BASE_SIZE, EDGE_WAYPOINTS_CAP } from '../constants/styles'; import { weakSpring } from '../utils/animation-utils'; @@ -15,7 +15,7 @@ const spline = line() .y(d => d.y); const transformedEdge = (props, path, thickness) => ( - + ); // Converts a waypoints map of the format { x0: 11, y0: 22, x1: 33, y1: 44 } @@ -27,7 +27,7 @@ const waypointsMapToArray = (waypointsMap) => { const [axis, index] = [key[0], key.slice(1)]; waypointsArray[index][axis] = value; }); - return fromJS(waypointsArray); + return waypointsArray; }; // Converts a waypoints array of the input format [{ x: 11, y: 22 }, { x: 33, y: 44 }] @@ -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 (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,38 +71,35 @@ 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, 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. - - {({ thickness, ...interpolatedWaypoints}) => transformedEdge( - forwardedProps, waypointsMapToArray(fromJS(interpolatedWaypoints)), thickness + + {({ interpolatedThickness, ...interpolatedWaypoints}) => transformedEdge( + forwardedProps, waypointsMapToArray(fromJS(interpolatedWaypoints)), interpolatedThickness )} ); } - 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(fromJS(waypoints)) }); + this.setState({ waypointsMap: waypointsArrayToMap(waypoints) }); } } diff --git a/client/app/scripts/charts/node-container.js b/client/app/scripts/charts/node-container.js index b613c680c..2d8198233 100644 --- a/client/app/scripts/charts/node-container.js +++ b/client/app/scripts/charts/node-container.js @@ -6,8 +6,6 @@ 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.