-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Enforce casting requested frame names to strings #1124
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
@@ -1815,6 +1818,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) | |||
} | |||
|
|||
function interruptPreviousTransitions() { | |||
// Fail-safe against purged plot: |
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.
thanks for the comment here.
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.
This could go elsewhere so that no failsafe is needed, but it's a trivially inexpensive one-liner, so I think it's a good idea.
beforeEach(function(done) { | ||
gd = createGraphDiv(); | ||
mockCopy = Lib.extendDeep({}, mock); | ||
Plotly.plot(gd, mockCopy.data, mockCopy.layout).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.
why use the two-argument signature here? 😄
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.
historical reasons!
very nice 💃 |
Frame names were successfully cast to strings, but requested frames via
Plotly.animate
were not. This PR fixes it and adds some tests. It also adds one small fail-safe against callbacks that execute after a plot has been purged.See: #1035