-
Notifications
You must be signed in to change notification settings - Fork 93
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
Improved code readability #4
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ const MoveTo = (() => { | |
tolerance: 0, | ||
duration: 800, | ||
easing: 'easeOutQuart', | ||
callback: function() {}, | ||
callback: function() {} | ||
}; | ||
|
||
/** | ||
|
@@ -19,10 +19,10 @@ const MoveTo = (() => { | |
* @param {number} d - duration | ||
* @return {number} - calculated value | ||
*/ | ||
function easeOutQuart(t, b, c, d) { | ||
t /= d; | ||
t--; | ||
return -c * (t * t * t * t - 1) + b; | ||
function easeOutQuart(currentTime, startValue, change, duration) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not necessary, the parameters are already explained above and Easing equations is usually known in this way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JSDoc need to update, if these changes will take, but really got better, I would have left. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. |
||
currentTime /= duration; | ||
currentTime--; | ||
return -change * (currentTime * currentTime * currentTime * currentTime - 1) + startValue; | ||
} | ||
|
||
/** | ||
|
@@ -121,7 +121,7 @@ const MoveTo = (() => { | |
* @param {object} options Custom options | ||
*/ | ||
MoveTo.prototype.move = function(target, options = {}) { | ||
if (target !== 0 && !target) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
All these evaluate to false. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes as you said // when target 0
if (!0) {
// will return
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I get it now! |
||
if (!target) { | ||
return; | ||
} | ||
|
||
|
@@ -139,15 +139,15 @@ const MoveTo = (() => { | |
let currentPageYOffset = window.pageYOffset; | ||
|
||
if (!startTime) { | ||
// To starts time from 1, we subtracted -1 from current time | ||
// To starts time from 1, we subtracted 1 from current time | ||
// If time starts from 1 The first loop will not do anything, | ||
// because easing value will be zero | ||
startTime = currentTime - 1; | ||
} | ||
|
||
const timeElapsed = currentTime - startTime; | ||
|
||
if (lastPageYOffset !== 0) { | ||
if (lastPageYOffset) { | ||
if ( | ||
(lastPageYOffset === currentPageYOffset) || | ||
(change > 0 && lastPageYOffset > currentPageYOffset) || | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an eslint rule called the comma-dangle. This should not be erased.