diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 5fe4d70fe3d..8e6d5440b40 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2214,6 +2214,20 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { } } + // Execute a callback after the wrapper function has been called n times. + // This is used to defer the resolution until a transition has resovled *and* + // the frame has completed. If it's not done this way, then we get a race + // condition in which the animation might resolve before a transition is complete + // or vice versa. + function callbackOnNthTime(cb, n) { + var cnt = 0; + return function() { + if(cb && ++cnt === n) { + return cb(); + } + }; + } + return new Promise(function(resolve, reject) { function discardExistingFrames() { if(trans._frameQueue.length === 0) { @@ -2264,7 +2278,7 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { // loop (which may exist as a result of a *different* .animate call) // still resolves or rejecdts this .animate call's promise. once it's // complete. - nextFrame.onComplete = resolve; + nextFrame.onComplete = callbackOnNthTime(resolve, 2); nextFrame.onInterrupt = reject; } @@ -2302,7 +2316,6 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { // Execute the callback and unset it to ensure it doesn't // accidentally get called twice trans._currentFrame.onComplete(); - trans._currentFrame.onComplete = null; } var newFrame = trans._currentFrame = trans._frameQueue.shift(); @@ -2322,7 +2335,12 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { helpers.coerceTraceIndices(gd, newFrame.frame.traces), newFrame.frameOpts, newFrame.transitionOpts - ); + ).then(function() { + if(newFrame.onComplete) { + newFrame.onComplete(); + } + + }); gd.emit('plotly_animatingframe', { name: newFrame.name, diff --git a/src/plots/plots.js b/src/plots/plots.js index 157489c285d..0a01dabf417 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1104,8 +1104,16 @@ plots.purge = function(gd) { // remove modebar if(fullLayout._modeBar) fullLayout._modeBar.destroy(); - if(gd._transitionData && gd._transitionData._animationRaf) { - window.cancelAnimationFrame(gd._transitionData._animationRaf); + if(gd._transitionData) { + // Ensure any dangling callbacks are simply dropped if the plot is purged. + // This is more or less only actually important for testing. + if(gd._transitionData._interruptCallbacks) { + gd._transitionData._interruptCallbacks.length = 0; + } + + if(gd._transitionData._animationRaf) { + window.cancelAnimationFrame(gd._transitionData._animationRaf); + } } // data and layout @@ -1714,6 +1722,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) var aborted = false; function executeTransitions() { + + gd.emit('plotly_transitioning', []); + return new Promise(function(resolve) { // This flag is used to disabled things like autorange: gd._transitioning = true; @@ -1798,7 +1809,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) } function completeTransition(callback) { - // Fail-safe against purged plot: + // This a simple workaround for tests which purge the graph before animations + // have completed. That's not a very common case, so this is the simplest + // fix. if(!gd._transitionData) return; flushCallbacks(gd._transitionData._interruptCallbacks); @@ -1848,13 +1861,13 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) var seq = [plots.previousPromises, interruptPreviousTransitions, prepareTransitions, executeTransitions]; - var transitionStarting = Lib.syncOrAsync(seq, gd); - if(!transitionStarting || !transitionStarting.then) transitionStarting = Promise.resolve(); + if(!transitionStarting || !transitionStarting.then) { + transitionStarting = Promise.resolve(); + } return transitionStarting.then(function() { - gd.emit('plotly_transitioning', []); return gd; }); }; diff --git a/test/jasmine/tests/animate_test.js b/test/jasmine/tests/animate_test.js index 752bf8d6496..f3eebb92d56 100644 --- a/test/jasmine/tests/animate_test.js +++ b/test/jasmine/tests/animate_test.js @@ -591,16 +591,14 @@ describe('Test animate API', function() { describe('Animate API details', function() { 'use strict'; - var gd, mockCopy; + var gd; + var dur = 30; + var mockCopy; beforeEach(function(done) { gd = createGraphDiv(); - mockCopy = Lib.extendDeep({}, mock); - - Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - return Plotly.addFrames(gd, mockCopy.frames); - }).then(done); + Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); }); afterEach(function() { @@ -608,15 +606,69 @@ describe('Animate API details', function() { destroyGraphDiv(); }); - it('null frames should not break everything', function(done) { - gd._transitionData._frames.push(null); + it('redraws after a layout animation', function(done) { + var redraws = 0; + gd.on('plotly_redraw', function() {redraws++;}); - Plotly.animate(gd, null, { - frame: {duration: 0}, - transition: {duration: 0} + Plotly.animate(gd, + {layout: {'xaxis.range': [0, 1]}}, + {frame: {redraw: true, duration: dur}, transition: {duration: dur}} + ).then(function() { + expect(redraws).toBe(1); }).catch(fail).then(done); }); + it('forces a relayout after layout animations', function(done) { + var relayouts = 0; + var restyles = 0; + var redraws = 0; + gd.on('plotly_relayout', function() {relayouts++;}); + gd.on('plotly_restyle', function() {restyles++;}); + gd.on('plotly_redraw', function() {redraws++;}); + + Plotly.animate(gd, + {layout: {'xaxis.range': [0, 1]}}, + {frame: {redraw: false, duration: dur}, transition: {duration: dur}} + ).then(function() { + expect(relayouts).toBe(1); + expect(restyles).toBe(0); + expect(redraws).toBe(0); + }).catch(fail).then(done); + }); + + it('triggers plotly_animated after a single layout animation', function(done) { + var animateds = 0; + gd.on('plotly_animated', function() {animateds++;}); + + Plotly.animate(gd, [ + {layout: {'xaxis.range': [0, 1]}}, + ], {frame: {redraw: false, duration: dur}, transition: {duration: dur}} + ).then(function() { + // Wait a bit just to be sure: + setTimeout(function() { + expect(animateds).toBe(1); + done(); + }, dur); + }); + }); + + it('triggers plotly_animated after a multi-step layout animation', function(done) { + var animateds = 0; + gd.on('plotly_animated', function() {animateds++;}); + + Plotly.animate(gd, [ + {layout: {'xaxis.range': [0, 1]}}, + {layout: {'xaxis.range': [2, 4]}}, + ], {frame: {redraw: false, duration: dur}, transition: {duration: dur}} + ).then(function() { + // Wait a bit just to be sure: + setTimeout(function() { + expect(animateds).toBe(1); + done(); + }, dur); + }); + }); + it('does not fail if strings are not used', function(done) { Plotly.addFrames(gd, [{name: 8, data: [{x: [8, 7, 6]}]}]).then(function() { // Verify it was added as a string name: @@ -634,7 +686,9 @@ describe('Animate API details', function() { var cnt = 0; gd.on('plotly_animatingframe', function() {cnt++;}); - Plotly.animate(gd, ['frame0', null, undefined], {transition: {duration: 0}, frame: {duration: 0}}).then(function() { + Plotly.addFrames(gd, mockCopy.frames).then(function() { + return Plotly.animate(gd, ['frame0', null, undefined], {transition: {duration: 0}, frame: {duration: 0}}); + }).then(function() { // Check only one animating was fired: expect(cnt).toEqual(1); @@ -642,4 +696,13 @@ describe('Animate API details', function() { expect(gd._fullLayout._currentFrame).toEqual('frame0'); }).catch(fail).then(done); }); + + it('null frames should not break everything', function(done) { + gd._transitionData._frames.push(null); + + Plotly.animate(gd, null, { + frame: {duration: 0}, + transition: {duration: 0} + }).catch(fail).then(done); + }); });