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

Transition fillcolor #1722

Merged
merged 1 commit into from
May 25, 2017
Merged

Transition fillcolor #1722

merged 1 commit into from
May 25, 2017

Conversation

rreusser
Copy link
Contributor

This PR solves #1721 and transitions fillcolor as expected, which currently only updates on an actual redraw.

An example in which it works: http://rickyreusser.com/demos/plotly-unsupported/fillcolor-animation.html

/cc @cpsievert

@rreusser rreusser added status: reviewable bug something broken labels May 24, 2017
@rreusser rreusser force-pushed the fillcolor-animation branch from 88516ea to 66a8abf Compare May 24, 2017 19:34
@rreusser
Copy link
Contributor Author

Test failure is:

Chrome 54.0.2840 (Linux 0.0.0) Test sort transform interactions: does not preserve hover/click `pointNumber` value FAILED

@etpinard is that to be expected? Is that an intentionally failing and not yet resolved test?

@etpinard
Copy link
Contributor

@etpinard is that to be expected?

Nop. Your patch shouldn't brake that test.

@etpinard
Copy link
Contributor

@rreusser I just pressed rebuild on CI, it might just be an intermittent failure.

@etpinard etpinard added this to the 1.28.0 milestone May 24, 2017
@rreusser
Copy link
Contributor Author

Sounds good! Oh… sort suggested deterministic, but hover suggests maybe a little flaky. Thanks!

@etpinard
Copy link
Contributor

@rreusser ok if I change the label to type: feature?

@rreusser
Copy link
Contributor Author

Yup! I debated. Do the labels have an effect/use, or is it just for record-keeping?

@etpinard
Copy link
Contributor

Do the labels have an effect/use

To make sure type: feature don't sneak into patch releases.

Conflicts:
	test/jasmine/tests/scatter_test.js
@rreusser rreusser force-pushed the fillcolor-animation branch from 66a8abf to 96b4e31 Compare May 25, 2017 18:06
@rreusser
Copy link
Contributor Author

@etpinard I've rebased and looks like now passing 👍

@rreusser rreusser added feature something new and removed bug something broken labels May 25, 2017
@etpinard
Copy link
Contributor

Great news 💃

@rreusser rreusser merged commit 0daf073 into master May 25, 2017
@rreusser rreusser deleted the fillcolor-animation branch May 25, 2017 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants