Skip to content

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

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Nov 9, 2016

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

@rreusser rreusser added status: reviewable bug something broken labels Nov 9, 2016
@etpinard etpinard added this to the v1.20.0 milestone Nov 9, 2016
@@ -1815,6 +1818,9 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts)
}

function interruptPreviousTransitions() {
// Fail-safe against purged plot:
Copy link
Contributor

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.

Copy link
Contributor Author

@rreusser rreusser Nov 9, 2016

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() {
Copy link
Contributor

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? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

historical reasons!

@etpinard
Copy link
Contributor

etpinard commented Nov 9, 2016

very nice 💃

@rreusser rreusser merged commit 62b33d3 into master Nov 9, 2016
@rreusser rreusser deleted the cast-frame-names-to-string branch November 9, 2016 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants