From f7d5d29d77cd0a7df09811db6f1c5c66009f4d08 Mon Sep 17 00:00:00 2001 From: gnom7 Date: Sun, 16 Sep 2018 13:38:37 +0300 Subject: [PATCH 1/7] ISSUE-90: added support for `extraDuration` feature --- .../components/ng-http-loader.component.ts | 46 +++++++------------ .../ng-http-loader.component.spec.ts | 34 ++++++++++++++ 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index 64aa1f4..4fbfee7 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -8,8 +8,8 @@ */ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; -import { EMPTY, merge, Observable, Subscription, timer } from 'rxjs'; -import { debounce, delayWhen } from 'rxjs/operators'; +import { merge, Subscription, timer } from 'rxjs'; +import { debounce, distinctUntilChanged, filter, switchMap } from 'rxjs/operators'; import { PendingInterceptorService } from '../services/pending-interceptor.service'; import { SpinnerVisibilityService } from '../services/spinner-visibility.service'; import { Spinkit } from '../spinkits'; @@ -40,17 +40,22 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { @Input() public minDuration = 0; @Input() + public extraDuration = 0; + @Input() public entryComponent: any = null; constructor(private pendingInterceptorService: PendingInterceptorService, private spinnerVisibilityService: SpinnerVisibilityService) { + const showSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => it)); + const hideSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => !it)); this.subscriptions = merge( - this.pendingInterceptorService.pendingRequestsStatus$.pipe( - debounce(h => this.handleDebounceDelay(h)), - delayWhen(h => this.handleMinDuration(h)) + showSpinner.pipe(debounce(() => timer(this.debounceDelay))), + showSpinner.pipe( + switchMap(() => hideSpinner.pipe( + debounce(() => timer(Math.max(this.extraDuration, this.minDuration - this.spinnerVisibleMillis()))) + )) ), this.spinnerVisibilityService.visibilityObservable$, - ) - .subscribe(h => this.handleSpinnerVisibility(h)); + ).pipe(distinctUntilChanged()).subscribe(status => this.handleSpinnerVisibility(status)); } ngOnInit(): void { @@ -101,30 +106,13 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { } private handleSpinnerVisibility(hasPendingRequests: boolean): void { - this.isSpinnerVisible = hasPendingRequests; - } - - private handleDebounceDelay(hasPendingRequests: boolean): Observable { - if (hasPendingRequests && !!this.debounceDelay) { - return timer(this.debounceDelay); + if (hasPendingRequests) { + this.startTime = Date.now(); } - - return EMPTY; - } - - private handleMinDuration(hasPendingRequests: boolean): Observable { - if (hasPendingRequests || !this.minDuration) { - if (this.shouldInitStartTime()) { - this.startTime = Date.now(); - } - - return timer(0); - } - - return timer(this.minDuration - (Date.now() - this.startTime)); + this.isSpinnerVisible = hasPendingRequests; } - private shouldInitStartTime(): boolean { - return !this.isSpinnerVisible; + private spinnerVisibleMillis(): number { + return Date.now() - this.startTime; } } diff --git a/src/test/components/ng-http-loader.component.spec.ts b/src/test/components/ng-http-loader.component.spec.ts index ce72561..06bb3c7 100644 --- a/src/test/components/ng-http-loader.component.spec.ts +++ b/src/test/components/ng-http-loader.component.spec.ts @@ -531,6 +531,40 @@ describe('NgHttpLoaderComponent', () => { } ))); + it('should handle the extra spinner duration for multiple HTTP requests ran one after the others', fakeAsync(inject( + [HttpClient, HttpTestingController], (http: HttpClient, httpMock: HttpTestingController) => { + component.extraDuration = 10; + + function runQuery(url: string): Observable { + return http.get(url); + } + + runQuery('/fake').subscribe(); + const firstRequest = httpMock.expectOne('/fake'); + + tick(1000); + expect(component.isSpinnerVisible).toBeTruthy(); + + // the first HTTP request is finally over, the spinner is still visible for at least 10ms + firstRequest.flush({}); + tick(5); + expect(component.isSpinnerVisible).toBeTruthy(); + + // But 5 ms after the first HTTP request has finished, a second HTTP request has been launched + runQuery('/fake2').subscribe(); + const secondRequest = httpMock.expectOne('/fake2'); + + // After 700ms, the second http request ends. The spinner is still visible + tick(700); + secondRequest.flush({}); + expect(component.isSpinnerVisible).toBeTruthy(); + + // 10ms later, the spinner should be hidden (extraDuration) + tick(10); + expect(component.isSpinnerVisible).toBeFalsy(); + } + ))); + it('should still display the spinner when the minimum duration is inferior to the HTTP request duration', fakeAsync(inject( [HttpClient, HttpTestingController], (http: HttpClient, httpMock: HttpTestingController) => { component.minDuration = 1000; From 71d96edfeebd9b082f3c928b93db982bfadeab42 Mon Sep 17 00:00:00 2001 From: Michel Palourdio Date: Sun, 16 Sep 2018 22:02:39 +0200 Subject: [PATCH 2/7] Adjust code formatting --- src/lib/components/ng-http-loader.component.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index 4fbfee7..4584af5 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -47,6 +47,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { constructor(private pendingInterceptorService: PendingInterceptorService, private spinnerVisibilityService: SpinnerVisibilityService) { const showSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => it)); const hideSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => !it)); + this.subscriptions = merge( showSpinner.pipe(debounce(() => timer(this.debounceDelay))), showSpinner.pipe( @@ -55,7 +56,9 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { )) ), this.spinnerVisibilityService.visibilityObservable$, - ).pipe(distinctUntilChanged()).subscribe(status => this.handleSpinnerVisibility(status)); + ) + .pipe(distinctUntilChanged()) + .subscribe(status => this.handleSpinnerVisibility(status)); } ngOnInit(): void { From cdba2be000b62f4d5ac54a4e9f73b37bc5bc20bf Mon Sep 17 00:00:00 2001 From: Michel Palourdio Date: Sun, 16 Sep 2018 22:17:18 +0200 Subject: [PATCH 3/7] Add missing return type --- src/lib/components/ng-http-loader.component.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index 4584af5..c8eb338 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -76,13 +76,13 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { } } - private initFilters() { + private initFilters(): void { this.initFilteredUrlPatterns(); this.initFilteredMethods(); this.initFilteredHeaders(); } - private initFilteredUrlPatterns() { + private initFilteredUrlPatterns(): void { if (!(this.filteredUrlPatterns instanceof Array)) { throw new TypeError('`filteredUrlPatterns` must be an array.'); } @@ -94,14 +94,14 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { } } - private initFilteredMethods() { + private initFilteredMethods(): void { if (!(this.filteredMethods instanceof Array)) { throw new TypeError('`filteredMethods` must be an array.'); } this.pendingInterceptorService.filteredMethods = this.filteredMethods; } - private initFilteredHeaders() { + private initFilteredHeaders(): void { if (!(this.filteredHeaders instanceof Array)) { throw new TypeError('`filteredHeaders` must be an array.'); } From beea1cdb45e73d12420dc1df8e249dfcfde7ebff Mon Sep 17 00:00:00 2001 From: Michel Palourdio Date: Sun, 16 Sep 2018 22:19:47 +0200 Subject: [PATCH 4/7] Extract code for clarity and rename spinnerVisibleMillis method --- src/lib/components/ng-http-loader.component.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index c8eb338..0834a91 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -8,7 +8,7 @@ */ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; -import { merge, Subscription, timer } from 'rxjs'; +import { merge, Observable, Subscription, timer } from 'rxjs'; import { debounce, distinctUntilChanged, filter, switchMap } from 'rxjs/operators'; import { PendingInterceptorService } from '../services/pending-interceptor.service'; import { SpinnerVisibilityService } from '../services/spinner-visibility.service'; @@ -51,9 +51,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { this.subscriptions = merge( showSpinner.pipe(debounce(() => timer(this.debounceDelay))), showSpinner.pipe( - switchMap(() => hideSpinner.pipe( - debounce(() => timer(Math.max(this.extraDuration, this.minDuration - this.spinnerVisibleMillis()))) - )) + switchMap(() => hideSpinner.pipe(debounce(() => this.getHiddingTimer()))) ), this.spinnerVisibilityService.visibilityObservable$, ) @@ -115,7 +113,12 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { this.isSpinnerVisible = hasPendingRequests; } - private spinnerVisibleMillis(): number { + private getSpinnerVisibilityDuration(): number { return Date.now() - this.startTime; } + + + private getHiddingTimer(): Observable { + return timer(Math.max(this.extraDuration, this.minDuration - this.getSpinnerVisibilityDuration())); + } } From 8cc18ffb04d75f961ccaeba08bb68c4efd712653 Mon Sep 17 00:00:00 2001 From: Michel Palourdio Date: Sun, 16 Sep 2018 22:26:20 +0200 Subject: [PATCH 5/7] Use partition and reintroduce 'h' as function argument --- src/lib/components/ng-http-loader.component.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index 0834a91..1be073c 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -9,7 +9,7 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { merge, Observable, Subscription, timer } from 'rxjs'; -import { debounce, distinctUntilChanged, filter, switchMap } from 'rxjs/operators'; +import { debounce, distinctUntilChanged, partition, switchMap } from 'rxjs/operators'; import { PendingInterceptorService } from '../services/pending-interceptor.service'; import { SpinnerVisibilityService } from '../services/spinner-visibility.service'; import { Spinkit } from '../spinkits'; @@ -45,8 +45,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { public entryComponent: any = null; constructor(private pendingInterceptorService: PendingInterceptorService, private spinnerVisibilityService: SpinnerVisibilityService) { - const showSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => it)); - const hideSpinner = this.pendingInterceptorService.pendingRequestsStatus$.pipe(filter(it => !it)); + const [showSpinner, hideSpinner] = partition((h: boolean) => h)(this.pendingInterceptorService.pendingRequestsStatus$); this.subscriptions = merge( showSpinner.pipe(debounce(() => timer(this.debounceDelay))), @@ -56,7 +55,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { this.spinnerVisibilityService.visibilityObservable$, ) .pipe(distinctUntilChanged()) - .subscribe(status => this.handleSpinnerVisibility(status)); + .subscribe(h => this.handleSpinnerVisibility(h)); } ngOnInit(): void { From 124373cbb772043e7b577720581840a4ce327437 Mon Sep 17 00:00:00 2001 From: gnom7 Date: Sun, 16 Sep 2018 23:31:47 +0300 Subject: [PATCH 6/7] ISSUE-90: Better handling of synchronous (sequential) requests * trying to fix failing test case on CI --- src/lib/components/ng-http-loader.component.ts | 18 +++++++----------- .../ng-http-loader.component.spec.ts | 5 +++-- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index 0834a91..9888024 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -23,7 +23,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { public isSpinnerVisible: boolean; public spinkit = Spinkit; private subscriptions: Subscription; - private startTime: number; + private visibleUntil: number = Date.now(); @Input() public backgroundColor: string; @@ -106,19 +106,15 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { this.pendingInterceptorService.filteredHeaders = this.filteredHeaders; } - private handleSpinnerVisibility(hasPendingRequests: boolean): void { - if (hasPendingRequests) { - this.startTime = Date.now(); + private handleSpinnerVisibility(showSpinner: boolean): void { + const now = Date.now(); + if (showSpinner && this.visibleUntil < now) { + this.visibleUntil = now + this.minDuration; } - this.isSpinnerVisible = hasPendingRequests; + this.isSpinnerVisible = showSpinner; } - private getSpinnerVisibilityDuration(): number { - return Date.now() - this.startTime; - } - - private getHiddingTimer(): Observable { - return timer(Math.max(this.extraDuration, this.minDuration - this.getSpinnerVisibilityDuration())); + return timer(Math.max(this.extraDuration, this.visibleUntil - Date.now())); } } diff --git a/src/test/components/ng-http-loader.component.spec.ts b/src/test/components/ng-http-loader.component.spec.ts index 06bb3c7..671b9e4 100644 --- a/src/test/components/ng-http-loader.component.spec.ts +++ b/src/test/components/ng-http-loader.component.spec.ts @@ -518,15 +518,16 @@ describe('NgHttpLoaderComponent', () => { const secondRequest = httpMock.expectOne('/fake2'); expect(component.isSpinnerVisible).toBeTruthy(); - // After 900ms, the second http request ends. The spinner should + // After 900ms, the spinner should // still be visible because the second HTTP request is still pending tick(900); expect(component.isSpinnerVisible).toBeTruthy(); // 500 ms later, the second http request ends. The spinner should be hidden - // Total time spent visible (1000+200+900+500==2600 > minDuration) + // Total time spent visible (1000+200+1400==2600 > minDuration) tick(500); secondRequest.flush({}); + tick(); expect(component.isSpinnerVisible).toBeFalsy(); } ))); From de85cd05a59917e0d9714336fd61d55943fa4b91 Mon Sep 17 00:00:00 2001 From: gnom7 Date: Sun, 16 Sep 2018 23:50:00 +0300 Subject: [PATCH 7/7] ISSUE-90: Better handling of synchronous (sequential) requests * trying to fix failing test case on CI --- src/lib/components/ng-http-loader.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/components/ng-http-loader.component.ts b/src/lib/components/ng-http-loader.component.ts index b59d63c..89cd093 100644 --- a/src/lib/components/ng-http-loader.component.ts +++ b/src/lib/components/ng-http-loader.component.ts @@ -107,7 +107,7 @@ export class NgHttpLoaderComponent implements OnDestroy, OnInit { private handleSpinnerVisibility(showSpinner: boolean): void { const now = Date.now(); - if (showSpinner && this.visibleUntil < now) { + if (showSpinner && this.visibleUntil <= now) { this.visibleUntil = now + this.minDuration; } this.isSpinnerVisible = showSpinner;