Skip to content
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

clear calcdata before redoing it in animations #1737

Merged
merged 4 commits into from
May 26, 2017

Conversation

alexcjohnson
Copy link
Collaborator

Fixes the issue we're seeing with calcTransforms + animations, which was introduced in #1717
Then the question is: what do we test this with and how?

cc @etpinard @rreusser

@@ -1813,6 +1813,10 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts)
// of essentially the whole supplyDefaults step, so that it seems sensible to just use
// supplyDefaults even though it's heavier than would otherwise be desired for
// transitions:

// first delete calcdata so supplyDefaults knows a calc step is coming
delete gd.calcdata;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a solid fix. It will slow down Plotly.animate though.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's some logic in Plots.doCalcdata involving old versus new calcdata that might be obsolete now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will it? We're calling doCalcdata no matter what so I'd think this will actually speed it up a tiny bit!

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point 👍

@rreusser
Copy link
Contributor

rreusser commented May 26, 2017

Here's at least a codepen that isolates this effect (plotly-latest works though so it's not visible in the codepen)

https://codepen.io/rsreusser/pen/rmbrXg?editors=0010

Correction: not the wrong frame; just the failure to transition.

@alexcjohnson alexcjohnson merged commit a0f8333 into master May 26, 2017
@alexcjohnson alexcjohnson deleted the fix-transform-animate branch May 26, 2017 21:39
@etpinard
Copy link
Contributor

Great thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants