From 84c1b0cde4be1df853139288b073a645e9628de2 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 9 Nov 2016 10:16:59 -0500 Subject: [PATCH] Enforce casting requested frame names to strings --- src/plot_api/plot_api.js | 6 ++-- src/plots/plots.js | 6 ++++ test/jasmine/tests/animate_test.js | 53 ++++++++++++++++++++++++++++ test/jasmine/tests/frame_api_test.js | 6 ++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index c707886b07f..fa45556f741 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2258,7 +2258,6 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { frameOpts: frameOpts, transitionOpts: transitionOpts, }; - if(i === frameList.length - 1) { // The last frame in this .animate call stores the promise resolve // and reject callbacks. This is how we ensure that the animation @@ -2410,14 +2409,15 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) { } else if(isFrameArray) { for(i = 0; i < frameOrGroupNameOrFrameList.length; i++) { var frameOrName = frameOrGroupNameOrFrameList[i]; - if(typeof frameOrName === 'string') { + if(['number', 'string'].indexOf(typeof frameOrName) !== -1) { + frameOrName = String(frameOrName); // In this case, there's an array and this frame is a string name: frameList.push({ type: 'byname', name: frameOrName, data: setTransitionConfig({name: frameOrName}) }); - } else { + } else if(Lib.isPlainObject(frameOrName)) { frameList.push({ type: 'object', data: setTransitionConfig(Lib.extendFlat({}, frameOrName)) diff --git a/src/plots/plots.js b/src/plots/plots.js index 1200689c322..4034d8dc40d 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1798,6 +1798,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) } function completeTransition(callback) { + // Fail-safe against purged plot: + if(!gd._transitionData) return; + flushCallbacks(gd._transitionData._interruptCallbacks); return Promise.resolve().then(function() { @@ -1815,6 +1818,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) } function interruptPreviousTransitions() { + // Fail-safe against purged plot: + if(!gd._transitionData) return; + // If a transition is interrupted, set this to false. At the moment, the only thing that would // interrupt a transition is another transition, so that it will momentarily be set to true // again, but this determines whether autorange or dragbox work, so it's for the sake of diff --git a/test/jasmine/tests/animate_test.js b/test/jasmine/tests/animate_test.js index c5aeb5acb3b..c8e926579d9 100644 --- a/test/jasmine/tests/animate_test.js +++ b/test/jasmine/tests/animate_test.js @@ -88,6 +88,14 @@ describe('Test animate API', function() { mockCopy = Lib.extendDeep({}, mock); + // ------------------------------------------------------------ + // NB: TRANSITION IS FAKED + // + // This means that you should not expect `.animate` to actually + // modify the plot in any way in the tests below. For tests + // involvingnon-faked transitions, see the bottom of this file. + // ------------------------------------------------------------ + spyOn(Plots, 'transition').and.callFake(function() { // Transition's fake behavior is just to delay by the duration // and resolve: @@ -579,3 +587,48 @@ describe('Test animate API', function() { }); }); }); + +describe('Test animate API', function() { + 'use strict'; + + var gd, 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); + }); + + afterEach(function() { + Plotly.purge(gd); + destroyGraphDiv(); + }); + + 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: + expect(gd._transitionData._frameHash['8']).not.toBeUndefined(); + + // Transition using a number: + return Plotly.animate(gd, [8], {transition: {duration: 0}, frame: {duration: 0}}); + }).then(function() { + // Confirm the result: + expect(gd.data[0].x).toEqual([8, 7, 6]); + }).catch(fail).then(done); + }); + + it('ignores null and undefined frames', function(done) { + var cnt = 0; + gd.on('plotly_animatingframe', function() {cnt++;}); + + Plotly.animate(gd, ['frame0', null, undefined], {transition: {duration: 0}, frame: {duration: 0}}).then(function() { + // Check only one animating was fired: + expect(cnt).toEqual(1); + + // Check unused frames did not affect the current frame: + expect(gd._fullLayout._currentFrame).toEqual('frame0'); + }).catch(fail).then(done); + }); +}); diff --git a/test/jasmine/tests/frame_api_test.js b/test/jasmine/tests/frame_api_test.js index eddcde880e5..9e006fa018d 100644 --- a/test/jasmine/tests/frame_api_test.js +++ b/test/jasmine/tests/frame_api_test.js @@ -60,6 +60,12 @@ describe('Test frame api', function() { }).catch(fail).then(done); }); + it('casts names to strings', function(done) { + Plotly.addFrames(gd, [{name: 5}]).then(function() { + expect(Object.keys(h)).toEqual(['5']); + }).catch(fail).then(done); + }); + it('creates multiple unnamed frames at the same time', function(done) { Plotly.addFrames(gd, [{}, {}]).then(function() { expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 1'}]);