-
Notifications
You must be signed in to change notification settings - Fork 107
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
refactor: requestAnimationFrame #420
base: master
Are you sure you want to change the base?
Conversation
Migrating from setInterval to requestAnimationFrame for a more optimize animation
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.
Hi @NormanV41,
thank you very much for the PR. I only have two notes that have to be checked. Feel free to do that, otherwise I'll try to squeeze that into my schedule this week.
} | ||
|
||
}, this.config._interval, pageScrollInstance); |
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.
As the config _interval
is not needed any longer to "customize" the animation/repaint, it should be removed
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.
I have deleted everything related with config._interval
@@ -198,7 +198,14 @@ export class PageScrollService { | |||
// .. and calculate the end time (when we need to finish at last) | |||
pageScrollInstance.endTime = pageScrollInstance.startTime + pageScrollInstance.executionDuration; | |||
|
|||
pageScrollInstance.timer = setInterval((instance: PageScrollInstance) => { | |||
pageScrollInstance.requestFrameId = window.requestAnimationFrame(this.updateScrollPostion(pageScrollInstance)) |
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.
The global window
object shouldn't be access directly, rather the little bit more complicated angular way via document
object, that we already got hold of at the pageScrollInstance config
https://stackoverflow.com/a/50023378/2225619 , https://stackoverflow.com/a/52620181/2225619)
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.
You are right, I changed it, now I am using pageScrollInstance.pageScrollOptions.document.defalutView
instead of window
Now use the injected document to reference the window object.
the _interval option is no longer needed
Migrating from setInterval to requestAnimationFrame for a more smooth animation