-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Conversation
@@ -330,6 +346,7 @@ Plotly.plot = function(gd, data, layout, config) { | |||
Lib.syncOrAsync([ | |||
Plots.previousPromises, | |||
drawFramework, | |||
addFrames, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why after drawFramework
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched.
There was a problem hiding this comment.
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.)
layout: {width: 500, height: 500}, | ||
config: {editable: true}, | ||
frames: [{y: [2, 1, 0], name: 'frame1'}] | ||
}).then(function() { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
nicely done 💃 |
This PR overloads
Plotly.plot
so that loading frames isn't a second, promise-chained step.Original (still works)
Variation
Now unnecessary