Skip to content

Conversation

@simonbrunel
Copy link
Member

If a value is set on the model after pivot() has been called, the view wasn't initialized and the animation started from 0. Now, _start and incomplete _view are initialized to the model value during the transition (no initial implicit transition).

Also remove exception handling when animating a string (color), which is faster when string are not valid colors (e.g. tooltip position). It requires to update chartjs-color to version 2.1.0.

}
}
} else if (type === 'number') {
view[key] = origin + (target - origin) * ease;
Copy link
Member

Choose a reason for hiding this comment

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

The only thing that's lost here is the NaN check which we already discussed about.

Copy link
Member Author

@simonbrunel simonbrunel Feb 25, 2017

Choose a reason for hiding this comment

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

Good catch, I will add that check back!

Copy link
Member Author

@simonbrunel simonbrunel Feb 25, 2017

Choose a reason for hiding this comment

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

What is the case already where isNaN(foo) === true and typeof(foo) === "number"?

Edit: answer typeof(NaN) and typeof(Infinity)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added isFinite checks on origin and target!

Copy link
Member

Choose a reason for hiding this comment

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

Perfect!

If a value is set on the model after `pivot()` has been called, the view wasn't initialized and the animation started from 0. Now, `_start` and incomplete `_view` are initialized to the model value during the transition (no initial implicit transition).

Also remove exception handling when animating a string (color), which is faster when string are not valid colors (e.g. tooltip position). It requires to update `chartjs-color` to version 2.1.0.
@etimberg etimberg merged commit d25e7b1 into chartjs:master Mar 3, 2017
@simonbrunel simonbrunel deleted the transition-cleanup branch March 3, 2017 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants