Skip to content

refactor: requestAnimationFrame #420

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions projects/ngx-page-scroll-core/src/lib/page-scroll-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ export class PageScrollInstance {
/* Whether an interrupt listener is attached to the body or not */
public interruptListenersAttached = false;

/* References to the timer instance that is used to perform the scroll animation to be
/* Value of the requestAnimationFrameId that is used to perform the scroll animation to be
able to clear it on animation end*/
public timer: any = null;

public requestFrameId: number | null = null;

/**
* Private constructor, requires the properties assumed to be the bare minimum.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { PageScrollConfig } from '../types/page-scroll.config';
export const NGXPS_CONFIG = new InjectionToken<PageScrollConfig>('ngxps_config');

export const defaultPageScrollConfig: PageScrollConfig = {
_interval: 10,
_minScrollDistance: 2,
_logLevel: 1,
namespace: 'default',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ export class PageScrollService {
pageScrollInstance.detachInterruptListeners();
}

if (pageScrollInstance.timer) {
if (pageScrollInstance.requestFrameId) {
// Clear/Stop the timer
clearInterval(pageScrollInstance.timer);
pageScrollInstance.pageScrollOptions.document.defaultView.cancelAnimationFrame(pageScrollInstance.requestFrameId)
// Clear the reference to this timer
pageScrollInstance.timer = undefined;
pageScrollInstance.requestFrameId = null;
pageScrollInstance.fireEvent(!interrupted);

return true;
Expand Down Expand Up @@ -157,17 +157,9 @@ export class PageScrollService {
Math.abs(pageScrollInstance.distanceToScroll) / pageScrollInstance.pageScrollOptions.speed * 1000;
}

// We should go there directly, as our "animation" would have one big step
// only anyway and this way we save the interval stuff
const tooShortInterval = pageScrollInstance.executionDuration <= pageScrollInstance.pageScrollOptions._interval;

if (allReadyAtDestination || tooShortInterval) {
if (allReadyAtDestination) {
if (this.config._logLevel >= 2 || (this.config._logLevel >= 1 && isDevMode())) {
if (allReadyAtDestination) {
console.log('Scrolling not possible, as we can\'t get any closer to the destination');
} else {
console.log('Scroll duration shorter that interval length, jumping to target');
}
console.log('Scrolling not possible, as we can\'t get any closer to the destination');
}
pageScrollInstance.setScrollPosition(pageScrollInstance.targetScrollPosition);
pageScrollInstance.fireEvent(true);
Expand Down Expand Up @@ -198,7 +190,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 = pageScrollInstance.pageScrollOptions.document.defaultView.requestAnimationFrame(this.updateScrollPostion(pageScrollInstance));

// Register the instance as running one
this.runningInstances.push(pageScrollInstance);
}

public updateScrollPostion(instance: PageScrollInstance){
return ()=>{
// Take the current time
const currentTime: number = new Date().getTime();

Expand Down Expand Up @@ -231,12 +230,10 @@ export class PageScrollService {
// (otherwise the event might arrive at "too early")
if (stopNow) {
this.stopInternal(false, instance);
} else{
instance.pageScrollOptions.document.defaultView.requestAnimationFrame(this.updateScrollPostion(instance))
}

}, this.config._interval, pageScrollInstance);
Copy link
Owner

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

Copy link
Author

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


// Register the instance as running one
this.runningInstances.push(pageScrollInstance);
}
}

public scroll(options: PageScrollOptions): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import { EasingLogic } from './easing-logic';

export interface PageScrollConfig {

/**
* The number of milliseconds to wait till updating the scroll position again.
* Small amounts may produce smoother animations but require more processing power.
*/
_interval?: number;

/**
* The amount of pixels that need to be between the current scrollTop/scrollLeft position
* and the target position the cause a scroll animation. In case distance is below
Expand Down