Skip to content
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.

Sign up for GitHub

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

fix(infinite list): add throttling to scroll listener #2948

Merged
merged 23 commits into from
Dec 1, 2021
Merged

fix(infinite list): add throttling to scroll listener #2948

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b800865
fix(infinity-list): emit issues
evtkhvch Nov 18, 2021
5005802
fix(layout): layout emit before scroll
evtkhvch Nov 19, 2021
563980a
fix(infinite-list): remove getContainerDimensions params
evtkhvch Nov 22, 2021
70ffe54
Revert "fix(layout): layout emit before scroll"
evtkhvch Nov 22, 2021
fff69c6
fix(infinite-list): add throttle to tick
evtkhvch Nov 22, 2021
80e1aab
fix(infinite-list): remove extra import
evtkhvch Nov 22, 2021
40a0e20
fix(infinite-list): add throttleTime property
evtkhvch Nov 24, 2021
69f8ca3
fix(infinite-list): add default throttling
evtkhvch Nov 25, 2021
1637179
Merge branch 'master' into fix-infinity-list-emit-issues
yggg Nov 25, 2021
f3131c8
refactor(infinite list): remove throttling by default
yggg Nov 25, 2021
4556f3d
Merge branch 'master' into fix-infinity-list-emit-issues
yggg Nov 25, 2021
14d2642
fix(infinite-list): move throttle to getContainerDimensions
evtkhvch Nov 26, 2021
5ed5f55
fix(infinite-list): change docs for throttleTime
evtkhvch Nov 26, 2021
4562ec9
fix(infinite-list): element scroll throttling
evtkhvch Nov 26, 2021
4817863
feat(infinite-list): add throttleTime specs
evtkhvch Nov 26, 2021
a926c3b
fix(infinite-list): change throttle specs
evtkhvch Nov 30, 2021
4651954
style(infinite-list): remove extra import
evtkhvch Nov 30, 2021
839d3ff
Merge branch 'master' into fix-infinity-list-emit-issues
yggg Dec 1, 2021
723061a
test(infinite list): remove redundant variable
yggg Dec 1, 2021
76a4e5c
style(test): fix type
yggg Dec 1, 2021
1348edf
refactor(infinite list): emit window scroll only when we listen for w…
yggg Dec 1, 2021
75abd7e
test(infinite list): move first cd run into fakeAsync
yggg Dec 1, 2021
e9ad0a3
Merge branch 'master' into fix-infinity-list-emit-issues
yggg Dec 1, 2021
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
32 changes: 18 additions & 14 deletions src/framework/theme/components/list/infinite-list.directive.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,28 +140,32 @@ describe('Directive: NbScrollDirective', () => {
expect(checkPositionSpy).toHaveBeenCalledTimes(1);
});

it('should ignore element scroll when listening to window or layout scroll', () => {
it('should ignore element scroll when listening to window or layout scroll', fakeAsync(() => {
testComponent.listenWindowScroll = true;
fixture.detectChanges();

const checkPositionSpy = spyOn(infiniteListDirective, 'checkPosition');

listElementRef.nativeElement.dispatchEvent(new Event('scroll'));
tick(infiniteListDirective.throttleTime);
expect(checkPositionSpy).toHaveBeenCalledTimes(0);

window.dispatchEvent(new Event('scroll'));
tick(infiniteListDirective.throttleTime);
expect(checkPositionSpy).toHaveBeenCalledTimes(1);

testComponent.withScroll = true;
fixture.detectChanges();

listElementRef.nativeElement.dispatchEvent(new Event('scroll'));
tick(infiniteListDirective.throttleTime);
expect(checkPositionSpy).toHaveBeenCalledTimes(1);

const layoutScrollContainer = layoutComponent.scrollableContainerRef.nativeElement;
layoutScrollContainer.dispatchEvent(new Event('scroll'));
tick(infiniteListDirective.throttleTime);
expect(checkPositionSpy).toHaveBeenCalledTimes(2);
});
}));

it('should trigger bottomThreshold only when treshold reached (element scroll)', fakeAsync(() => {
const scrollingNativeElement = listElementRef.nativeElement;
Expand All @@ -170,13 +174,13 @@ describe('Directive: NbScrollDirective', () => {
const positionUnderThreshold = CONTENT_HEIGHT - THRESHOLD - ELEMENT_HEIGHT - 1;
scrollingNativeElement.scrollTop = positionUnderThreshold;
scrollingNativeElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

const positionBelowThreshold = CONTENT_HEIGHT - THRESHOLD / 2;
scrollingNativeElement.scrollTop = positionBelowThreshold;
scrollingNativeElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -192,13 +196,13 @@ describe('Directive: NbScrollDirective', () => {
const positionUnderThreshold = CONTENT_HEIGHT - THRESHOLD - reporterHeight;
documentElement.scrollTop = positionUnderThreshold;
window.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

const positionBelowThreshold = CONTENT_HEIGHT - THRESHOLD / 2;
documentElement.scrollTop = positionBelowThreshold;
window.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -214,13 +218,13 @@ describe('Directive: NbScrollDirective', () => {
const positionUnderThreshold = CONTENT_HEIGHT - THRESHOLD - scroller.clientHeight - 1;
scroller.scrollTop = positionUnderThreshold;
scroller.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

const positionBelowThreshold = CONTENT_HEIGHT - THRESHOLD / 2;
scroller.scrollTop = positionBelowThreshold;
scroller.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -230,12 +234,12 @@ describe('Directive: NbScrollDirective', () => {

scrollingElement.scrollTop = THRESHOLD + 1;
scrollingElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

scrollingElement.scrollTop = THRESHOLD - 1;
scrollingElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -248,12 +252,12 @@ describe('Directive: NbScrollDirective', () => {

documentElement.scrollTop = THRESHOLD + 1;
window.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

documentElement.scrollTop = THRESHOLD - 1;
window.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -267,12 +271,12 @@ describe('Directive: NbScrollDirective', () => {

layoutElement.scrollTop = THRESHOLD + 1;
layoutElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(0);

layoutElement.scrollTop = THRESHOLD - 1;
layoutElement.dispatchEvent(new Event('scroll'));
tick();
tick(infiniteListDirective.throttleTime);
expect(tresholdSpy).toHaveBeenCalledTimes(1);
}));
});
60 changes: 34 additions & 26 deletions src/framework/theme/components/list/infinite-list.directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {
ContentChildren,
QueryList,
} from '@angular/core';
import { Observable, forkJoin, of as observableOf, interval, timer, Subject } from 'rxjs';
import { filter, switchMap, map, takeUntil, take } from 'rxjs/operators';
import { Observable, forkJoin, of as observableOf, interval, timer, Subject, merge } from 'rxjs';
import { filter, switchMap, map, takeUntil, take, throttle } from 'rxjs/operators';
import { convertToBoolProperty, NbBooleanInput } from '../helpers';
import { NbLayoutScrollService } from '../../services/scroll.service';
import { NbLayoutRulerService } from '../../services/ruler.service';
import { NbListItemComponent } from './list.component';

export class NbScrollableContainerDimentions {
export class NbScrollableContainerDimensions {
scrollTop: number;
scrollHeight: number;
clientHeight: number;
Expand Down Expand Up @@ -55,13 +55,13 @@ export class NbScrollableContainerDimentions {
selector: '[nbInfiniteList]',
})
export class NbInfiniteListDirective implements AfterViewInit, OnDestroy {

private destroy$ = new Subject<void>();
private lastScrollPosition;
windowScroll = false;
private get elementScroll() {
return !this.windowScroll;
}
private elementScroll$ = new Subject();
yggg marked this conversation as resolved.
Show resolved Hide resolved

/**
* Threshold after which event load more event will be emited.
Expand All @@ -70,6 +70,13 @@ export class NbInfiniteListDirective implements AfterViewInit, OnDestroy {
@Input()
threshold: number;

/**
* Prevent subsequent bottom/topThreshold emissions for specified duration after emitting once.
* In milliseconds.
*/
@Input()
throttleTime = 0;

/**
* By default component observes list scroll position.
* If set to `true`, component will observe position of page scroll instead.
Expand All @@ -95,7 +102,7 @@ export class NbInfiniteListDirective implements AfterViewInit, OnDestroy {
@HostListener('scroll')
onElementScroll() {
if (this.elementScroll) {
this.checkPosition(this.elementRef.nativeElement);
this.elementScroll$.next();
}
}

Expand All @@ -108,46 +115,48 @@ export class NbInfiniteListDirective implements AfterViewInit, OnDestroy {
) {}

ngAfterViewInit() {
this.scrollService.onScroll()
merge(this.scrollService.onScroll(), this.elementScroll$)
.pipe(
filter(() => this.windowScroll),
throttle(() => interval(this.throttleTime)),
switchMap(() => this.getContainerDimensions()),
takeUntil(this.destroy$),
)
.subscribe(dimentions => this.checkPosition(dimentions));
.subscribe((dimensions) => this.checkPosition(dimensions));

this.listItems.changes
.pipe(
// For some reason, changes are emitted before list item removed from dom,
// so dimensions will be incorrect.
// Check every 50ms for a second if dom and query are in sync.
// Once they synchronized, we can get proper dimensions.
switchMap(() => interval(50).pipe(
filter(() => this.inSyncWithDom()),
take(1),
takeUntil(timer(1000)),
)),
switchMap(() =>
interval(50).pipe(
filter(() => this.inSyncWithDom()),
take(1),
takeUntil(timer(1000)),
),
),
switchMap(() => this.getContainerDimensions()),
takeUntil(this.destroy$),
)
.subscribe(dimentions => this.checkPosition(dimentions));
.subscribe((dimensions) => this.checkPosition(dimensions));

this.getContainerDimensions().subscribe(dimentions => this.checkPosition(dimentions));
this.getContainerDimensions().subscribe((dimensions) => this.checkPosition(dimensions));
}

ngOnDestroy() {
this.destroy$.next();
this.destroy$.complete();
}

checkPosition({ scrollHeight, scrollTop, clientHeight }: NbScrollableContainerDimentions) {
checkPosition({ scrollHeight, scrollTop, clientHeight }: NbScrollableContainerDimensions) {
const initialCheck = this.lastScrollPosition == null;
const manualCheck = this.lastScrollPosition === scrollTop;
const scrollUp = scrollTop < this.lastScrollPosition;
const scrollDown = scrollTop > this.lastScrollPosition;
const distanceToBottom = scrollHeight - scrollTop - clientHeight;

if ((initialCheck || manualCheck || scrollDown) && distanceToBottom <= this.threshold) {
if ((initialCheck || manualCheck || scrollDown) && distanceToBottom <= this.threshold) {
this.bottomThreshold.emit();
}
if ((initialCheck || scrollUp) && scrollTop <= this.threshold) {
Expand All @@ -157,20 +166,19 @@ export class NbInfiniteListDirective implements AfterViewInit, OnDestroy {
this.lastScrollPosition = scrollTop;
}

private getContainerDimensions(): Observable<NbScrollableContainerDimentions> {
private getContainerDimensions(): Observable<NbScrollableContainerDimensions> {
if (this.elementScroll) {
const { scrollTop, scrollHeight, clientHeight } = this.elementRef.nativeElement;
return observableOf({ scrollTop, scrollHeight, clientHeight });
}

return forkJoin([this.scrollService.getPosition(), this.dimensionsService.getDimensions()])
.pipe(
map(([scrollPosition, dimensions]) => ({
scrollTop: scrollPosition.y,
scrollHeight: dimensions.scrollHeight,
clientHeight: dimensions.clientHeight,
})),
);
return forkJoin([this.scrollService.getPosition(), this.dimensionsService.getDimensions()]).pipe(
map(([scrollPosition, dimensions]) => ({
scrollTop: scrollPosition.y,
scrollHeight: dimensions.scrollHeight,
clientHeight: dimensions.clientHeight,
})),
);
}

private inSyncWithDom(): boolean {
Expand Down