-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
A few more things to clean up in the promise land #76
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
Comments
From a separate convo
It's not the end of the world if plotting is less synchronous than it could be, but the experience is better for the viewer (at the very least fewer flashes of half-built plots, and likely faster display overall) if it's sync. For the developer using plotly.js it's probably no different, they should get used to expecting |
@etpinard @alexcjohnson I see that the promise resolve isn't passed an argument - should we pass in the gd? That's what I would expect to be handed when using the library, and is trivial to implement because the gd is always in scope when Promise.resolve is called. |
@mdtusz I vote for passing the |
@mdtusz sure, if you think it would be useful. Generally I don't see why it would, as all these functions take |
@alexcjohnson my main use-case is if someone does something like this: // In some place, dynamically adding plot(s)
var myPlot = Plotly.plot('someDiv', data, layout);
var plots = [myPlot];
// Somewhere else in the code, maybe another file
plots.forEach(function(plot){
plot.then(function(gd){
gd.className = "visible";
});
}); It's a bit of a contrived example, but the expected behaviour of promises is generally to receive some success data, or an error. Doesn't hurt to pass it along in any case! |
👍 yup, that was basically the kind of case I was imagining, go for it. |
Added promises to all the Plotly.____ methods here. |
@alexcjohnson @etpinard Can we close this? |
Ideally it would be nice to do a little exploration of whether our async behavior really is what we think it is, ie only MathJax (and maybe gl?) make plotting be async. Mainly to make it easier to write tests around plotting. Also when it is async, it looks to me as though we need |
considering this closed by #1991 and many before it |
plot
,newPlot
,redraw
,restyle
, andrelayout
do, but notextendTraces
,moveTraces
etc.The text was updated successfully, but these errors were encountered: