Skip to content

Conversation

@simonbrunel
Copy link
Member

@simonbrunel simonbrunel commented Jul 8, 2017

Also removed 1*, /1 and replace 1/2 by 0.5

@simonbrunel simonbrunel added this to the Version 2.7 milestone Jul 8, 2017
@simonbrunel simonbrunel requested a review from etimberg July 8, 2017 07:45
Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

A few minor comments. Other than that, it looks good 👍

},

easeInExpo: function(t) {
return (t === 0) ? 0 : Math.pow(2, 10 * (t - 1));
Copy link
Member

Choose a reason for hiding this comment

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

I think this changed when t === 0. Was that intentional? The old version was

easeInExpo: function(t) {
  return (t === 0) ? 1 : 1 * Math.pow(2, 10 * (t / 1 - 1));
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it was incorrectly adapted from the original method:

// t: current time, b: start value, c: end value, d: duration
easeInExpo: function (x, t, b, c, d) {
   return (t==0) ? b : c * Math.pow(2, 10 * (t/d - 1)) + b;
}

All these methods assume b == 0 and c == 1 and d == 1

Copy link
Member

Choose a reason for hiding this comment

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

ah, ok. Good catch!

return 1;
}
if (!p) {
p = 1 * 0.3;
Copy link
Member

Choose a reason for hiding this comment

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

could we simplify this too as p = 0.3?

if (!p) {
p = 1 * 0.3;
}
if (a < Math.abs(1)) {
Copy link
Member

Choose a reason for hiding this comment

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

Could also remove Math.abs(1) too

return 1;
}
if (!p) {
p = 1 * 0.3;
Copy link
Member

Choose a reason for hiding this comment

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

same as previous function

if (!p) {
p = 1 * 0.3;
}
if (a < Math.abs(1)) {
Copy link
Member

Choose a reason for hiding this comment

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

same as previous function

return 1;
}
if (!p) {
p = 0.3 * 1.5;
Copy link
Member

Choose a reason for hiding this comment

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

could we change this to p = 0.45 ?

if (!p) {
p = 0.3 * 1.5;
}
if (a < Math.abs(1)) {
Copy link
Member

Choose a reason for hiding this comment

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

same as previous function

@etimberg etimberg merged commit 463a8dd into chartjs:master Jul 8, 2017
@simonbrunel simonbrunel deleted the helpers-easing branch July 8, 2017 16:05
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