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.

Sign up for GitHub

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

Jump to bottom

Animate API #802

Merged
merged 45 commits into from
Sep 6, 2016
Merged

Animate API #802

merged 45 commits into from
Sep 6, 2016

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Aug 1, 2016 •
edited
Loading

I made a mess of #717 by letting it go too long and performing a bad merge/rebase or two. This PR handles animation and attempts to get all tests passing.

Checklist:

  • disable dragbox while transitioning
  • master autorange-disable override (set/unset before/after transition)
  • .animate queues frames
    • frame groups
    • frame lists
    • individual frames
    • raf loop timing
    • accept and coerce animation config
    • 'immediate' mode to flush queue and interrupt
    • add defaultduration and frameduration defaults
  • maybe improve how error bars are updated
  • confirm whether subplot transition was getting interrupted by the other subplot
  • restore point fade in/out behavior
  • ensure all fields are purged: https://github.com/plotly/plotly.js/blob/master/src/plots/plots.js#L779

@rreusser rreusser force-pushed the animate-api-take-4 branch 4 times, most recently from 7bf81a0 to 06d1e46 Compare August 5, 2016 16:57
@etpinard etpinard added this to the v1.17.0 milestone Aug 5, 2016
@rreusser rreusser force-pushed the animate-api-take-4 branch 3 times, most recently from d14cc15 to fbfbf53 Compare August 11, 2016 19:52
@rreusser rreusser mentioned this pull request Aug 12, 2016
Closed
@rreusser rreusser force-pushed the animate-api-take-4 branch 3 times, most recently from 0f92220 to 7447887 Compare August 12, 2016 17:16
@etpinard
Copy link
Contributor

@rreusser what's the current status of this PR?

@rreusser
Copy link
Contributor Author

rreusser commented Aug 22, 2016 •
edited
Loading

Currently trying to solve race condition with the autolayout feature. I was cheating a bit and timing transitions independently of d3's execution of the transitions, but that tends to result in problematic race conditions in which every now and then a d3 update comes through after the transitions has been redrawn. I feel like I'm attempting to group my transitions correctly, but the just don't group.

@monfera, do you have experience with grouping d3 v3 transitions? I put large chunks that create more transitions inside a .transition().each(...), but the properties don't seem to get inherited.

Also, @etpinard, are you aware of a straightforward way to temporarily disable autorange while performing a redraw? With the cleanup, it now autoranges when redraw is called at the end of transitions, which is rarely desirable.

@rreusser rreusser force-pushed the animate-api-take-4 branch from 7447887 to 0946987 Compare August 23, 2016 16:09
@rreusser
Copy link
Contributor Author

rreusser commented Aug 23, 2016 •
edited
Loading

To do:

  • disable dragbox while axis transition is in progress

etpinard
etpinard reviewed Aug 23, 2016
View reviewed changes
src/components/errorbars/plot.js
}

if(xObj.visible && isNumeric(coords.y) &&
if(xObj.visible && isNumeric(coords.x) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wow, good catch. 😬

@etpinard
Copy link
Contributor

etpinard commented Sep 6, 2016

@rreusser a very well-deserved 💃

cc @chriddyp @jackparmer @cldougl

@rreusser rreusser merged commit d736910 into master Sep 6, 2016
@rreusser rreusser mentioned this pull request Sep 6, 2016
Closed
@jackparmer
Copy link
Contributor

image

An epic piece of work @rreusser !

@etpinard etpinard deleted the animate-api-take-4 branch September 6, 2016 22:09
@etpinard etpinard mentioned this pull request Sep 6, 2016
Merged
etpinard added a commit that referenced this pull request Sep 8, 2016
@etpinard
clean up old scatter traces that were restyle to 'visible: false'
ce58977 
- as of PR #802, scatter traces are no longer purged
  in the per-subplot cartesian plot step - to maintain
  consistency between transitions.
- While the scatter plot module can remove individual
  'visible: false' traces, when all scatter traces are restyled
  to 'visible: false' the scatter plot module is currently not called!
etpinard added a commit that referenced this pull request Sep 8, 2016
@etpinard
clean up old scatter traces that were restyle to 'visible: false'
83d6888 
- as of PR #802, scatter traces are no longer purged
  in the per-subplot cartesian plot step - to maintain
  consistency between transitions.
- While the scatter plot module can remove individual
  'visible: false' traces, when all scatter traces are restyled
  to 'visible: false' the scatter plot module is currently not called!
This was referenced Sep 8, 2016
Merged
Closed
This was referenced Sep 15, 2016
Merged
Closed
Merged
@etpinard etpinard mentioned this pull request Nov 24, 2016
Merged
@etpinard etpinard mentioned this pull request Jun 7, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature something new
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@rreusser @etpinard @jackparmer @cldougl