Skip to content

Modify Plotly.plot to accept frames #1014 #1054

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.

Sign up for GitHub

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 3 commits into from
Oct 19, 2016
Merged

Modify Plotly.plot to accept frames #1014 #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6db2451
Modify Plotly.plot to accept frames #1014
rreusser Oct 18, 2016
cec6644
Switch drawframework and addframes order
rreusser Oct 18, 2016
0bc4c6a
COnfirm can add more frames later
rreusser Oct 18, 2016
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
17 changes: 17 additions & 0 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,21 @@ var subroutines = require('./subroutines');
*
*/
Plotly.plot = function(gd, data, layout, config) {
var frames;

gd = helpers.getGraphDiv(gd);

// Events.init is idempotent and bails early if gd has already been init'd
Events.init(gd);

if(Lib.isPlainObject(data)) {
var obj = data;
data = obj.data;
layout = obj.layout;
config = obj.config;
frames = obj.frames;
}

var okToPlot = Events.triggerHandler(gd, 'plotly_beforeplot', [data, layout, config]);
if(okToPlot === false) return Promise.reject();

Expand All @@ -62,6 +72,12 @@ Plotly.plot = function(gd, data, layout, config) {
'but this container doesn\'t yet have a plot.', gd);
}

function addFrames() {
if(frames) {
return Plotly.addFrames(gd, frames);
}
}

// transfer configuration options to gd until we move over to
// a more OO like model
setPlotContext(gd, config);
Expand Down Expand Up @@ -329,6 +345,7 @@ Plotly.plot = function(gd, data, layout, config) {

Lib.syncOrAsync([
Plots.previousPromises,
addFrames,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why after drawFramework?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Plots.supplyDefaults(gd); is called synchronously in the above function body so yeah, switching those two should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Of course you could now ask the opposite question, but I think loading data first is nicer.)

drawFramework,
marginPushers,
marginPushersAgain,
Expand Down
64 changes: 64 additions & 0 deletions test/jasmine/tests/plot_api_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var subroutines = require('@src/plot_api/subroutines');
var d3 = require('d3');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var fail = require('../assets/fail_test');


describe('Test plot api', function() {
Expand All @@ -22,6 +23,69 @@ describe('Test plot api', function() {
});
});

describe('Plotly.plot', function() {
var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('accepts gd, data, layout, and config as args', function(done) {
Plotly.plot(gd,
[{x: [1, 2, 3], y: [1, 2, 3]}],
{width: 500, height: 500},
{editable: true}
).then(function() {
expect(gd.layout.width).toEqual(500);
expect(gd.layout.height).toEqual(500);
expect(gd.data.length).toEqual(1);
expect(gd._context.editable).toBe(true);
}).catch(fail).then(done);
});

it('accepts gd and an object as args', function(done) {
Plotly.plot(gd, {
data: [{x: [1, 2, 3], y: [1, 2, 3]}],
layout: {width: 500, height: 500},
config: {editable: true},
frames: [{y: [2, 1, 0], name: 'frame1'}]
}).then(function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rreusser does

Plotly.plot(gd, {
  data: [],
  frames: [ /* some initial list of frames */ ]
})
.then((gd) => { 

  // add more frames!!
  Plotly.addFrames(gd), [ /* more frames !! */ ]);
})

work as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup!

Unexpected token )

But srsly, added this test and just pushed.

expect(gd.layout.width).toEqual(500);
expect(gd.layout.height).toEqual(500);
expect(gd.data.length).toEqual(1);
expect(gd._transitionData._frames.length).toEqual(1);
expect(gd._context.editable).toBe(true);
}).catch(fail).then(done);
});

it('allows adding more frames to the initial set', function(done) {
Plotly.plot(gd, {
data: [{x: [1, 2, 3], y: [1, 2, 3]}],
layout: {width: 500, height: 500},
config: {editable: true},
frames: [{y: [7, 7, 7], name: 'frame1'}]
}).then(function() {
expect(gd.layout.width).toEqual(500);
expect(gd.layout.height).toEqual(500);
expect(gd.data.length).toEqual(1);
expect(gd._transitionData._frames.length).toEqual(1);
expect(gd._context.editable).toBe(true);

return Plotly.addFrames(gd, [
{y: [8, 8, 8], name: 'frame2'},
{y: [9, 9, 9], name: 'frame3'}
]);
}).then(function() {
expect(gd._transitionData._frames.length).toEqual(3);
expect(gd._transitionData._frames[0].name).toEqual('frame1');
expect(gd._transitionData._frames[1].name).toEqual('frame2');
expect(gd._transitionData._frames[2].name).toEqual('frame3');
}).catch(fail).then(done);
});
});

describe('Plotly.relayout', function() {
var gd;

Expand Down