Merge pull request #3860 from plotly/animate-multiple-basePlotModules

Fix Plotly.animate on graphs with multiple basePlotModules

Author: etpinard

src/plots/cartesian/index.js

@@ -125,10 +125,9 @@ exports.finalizeSubplots = function(layoutIn, layoutOut) {
  * Cartesian.plot
  *
  * @param {DOM div | object} gd
- * @param {array | null} (optional) traces
+ * @param {array (optional)} traces
  *  array of traces indices to plot
  *  if undefined, plots all cartesian traces,
- *  if null, plots no traces
  * @param {object} (optional) transitionOpts
  *  transition option object
  * @param {function} (optional) makeOnCompleteCallback
@@ -140,11 +139,7 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) {
     var calcdata = gd.calcdata;
     var i;
 
-    if(traces === null) {
-        // this means no updates required, must return here
-        // so that plotOne doesn't remove the trace layers
-        return;
-    } else if(!Array.isArray(traces)) {
+    if(!Array.isArray(traces)) {
         // If traces is not provided, then it's a complete replot and missing
         // traces are removed
         traces = [];

src/plots/plots.js

@@ -2309,7 +2309,7 @@ plots.extendLayout = function(destLayout, srcLayout) {
  */
 plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) {
     var opts = {redraw: frameOpts.redraw};
-    var transitionedTraces = [];
+    var transitionedTraces = {};
     var axEdits = [];
 
     opts.prepareFn = function() {
@@ -2319,16 +2319,18 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts)
         for(var i = 0; i < traceIndices.length; i++) {
             var traceIdx = traceIndices[i];
             var trace = gd._fullData[traceIdx];
-            var module = trace._module;
+            var _module = trace._module;
 
             // There's nothing to do if this module is not defined:
-            if(!module) continue;
+            if(!_module) continue;
 
             // Don't register the trace as transitioned if it doesn't know what to do.
             // If it *is* registered, it will receive a callback that it's responsible
             // for calling in order to register the transition as having completed.
-            if(module.animatable) {
-                transitionedTraces.push(traceIdx);
+            if(_module.animatable) {
+                var n = _module.basePlotModule.name;
+                if(!transitionedTraces[n]) transitionedTraces[n] = [];
+                transitionedTraces[n].push(traceIdx);
             }
 
             gd.data[traceIndices[i]] = plots.extendTrace(gd.data[traceIndices[i]], data[i]);
@@ -2427,19 +2429,21 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts)
         if(hasAxisTransition) {
             traceTransitionOpts = Lib.extendFlat({}, transitionOpts);
             traceTransitionOpts.duration = 0;
-            // This means do not transition traces,
+            // This means do not transition cartesian traces,
             // this happens on layout-only (e.g. axis range) animations
-            transitionedTraces = null;
+            delete transitionedTraces.cartesian;
         } else {
             traceTransitionOpts = transitionOpts;
         }
 
-        for(i = 0; i < basePlotModules.length; i++) {
-            // Note that we pass a callback to *create* the callback that must be invoked on completion.
-            // This is since not all traces know about transitions, so it greatly simplifies matters if
-            // the trace is responsible for creating a callback, if needed, and then executing it when
-            // the time is right.
-            basePlotModules[i].plot(gd, transitionedTraces, traceTransitionOpts, makeCallback);
+        // Note that we pass a callback to *create* the callback that must be invoked on completion.
+        // This is since not all traces know about transitions, so it greatly simplifies matters if
+        // the trace is responsible for creating a callback, if needed, and then executing it when
+        // the time is right.
+        for(var n in transitionedTraces) {
+            var traceIndices = transitionedTraces[n];
+            var _module = gd._fullData[traceIndices[0]]._module;
+            _module.basePlotModule.plot(gd, traceIndices, traceTransitionOpts, makeCallback);
         }
     };
 

src/traces/sunburst/attributes.js

@@ -61,6 +61,7 @@ module.exports = {
     level: {
         valType: 'any',
         editType: 'plot',
+        anim: true,
         role: 'info',
         description: [
             'Sets the level from which this sunburst trace hierarchy is rendered.',

test/jasmine/tests/sunburst_test.js

@@ -1160,4 +1160,58 @@ describe('Test sunburst interactions edge cases', function() {
         })
         .then(done);
     });
+
+    it('should transition sunburst traces only', function(done) {
+        var mock = Lib.extendDeep({}, require('@mocks/display-text_zero-number.json'));
+        mock.data[0].visible = false;
+
+        function _assert(msg, exp) {
+            var gd3 = d3.select(gd);
+            expect(gd3.select('.cartesianlayer').selectAll('.trace').size())
+                .toBe(exp.cartesianTraceCnt, '# of cartesian traces');
+            expect(gd3.select('.pielayer').selectAll('.trace').size())
+                .toBe(exp.pieTraceCnt, '# of pie traces');
+            expect(gd3.select('.sunburstlayer').selectAll('.trace').size())
+                .toBe(exp.sunburstTraceCnt, '# of sunburst traces');
+        }
+
+        Plotly.plot(gd, mock)
+        .then(function() {
+            _assert('base', {
+                cartesianTraceCnt: 2,
+                pieTraceCnt: 0,
+                sunburstTraceCnt: 1
+            });
+        })
+        .then(click(gd, 2))
+        .then(delay(constants.CLICK_TRANSITION_TIME + 1))
+        .then(function() {
+            _assert('after sunburst click', {
+                cartesianTraceCnt: 2,
+                pieTraceCnt: 0,
+                sunburstTraceCnt: 1
+            });
+        })
+        .catch(failTest)
+        .then(done);
+    });
+
+    it('should be able to transition sunburst traces via `Plotly.react`', function(done) {
+        var mock = Lib.extendDeep({}, require('@mocks/display-text_zero-number.json'));
+        mock.layout.transition = {duration: 200};
+
+        spyOn(Plots, 'transitionFromReact').and.callThrough();
+
+        Plotly.plot(gd, mock)
+        .then(function() {
+            gd.data[1].level = 'B';
+            return Plotly.react(gd, gd.data, gd.layout);
+        })
+        .then(delay(202))
+        .then(function() {
+            expect(Plots.transitionFromReact).toHaveBeenCalledTimes(1);
+        })
+        .catch(failTest)
+        .then(done);
+    });
 });