Skip to content

Fix Plotly.animate on graphs with multiple basePlotModules #3860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = [];
Expand Down
30 changes: 17 additions & 13 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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]);
Expand Down Expand Up @@ -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);
}
};

Expand Down
1 change: 1 addition & 0 deletions src/traces/sunburst/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
54 changes: 54 additions & 0 deletions test/jasmine/tests/sunburst_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});