Skip to content

Commit 1488fa5

Browse files
committed
refactor(docs-infra): drive the progress bar from the currentNavigation signal
The progress bar listened to `NavigationStart` on `router.events`, started a timer to show the bar after `PROGRESS_BAR_DELAY`, and used `switchMap` to wait for the first `NavigationEnd`/`Cancel`/`Skipped`/`Error` event to clear the timer and complete the bar. Replace this with a single `effect` on the router's `currentNavigation` signal: while a navigation is ongoing the effect schedules the timer, and its cleanup, which runs when the navigation ends or is superseded, clears the timer and completes the bar. The effect tracks the navigation id rather than the navigation object, since `currentNavigation` notifies on every update of an ongoing navigation and would otherwise restart the timer mid-navigation. Replace the disabled spec with specs that keep a navigation pending through a resolver, covering slow, fast and canceled navigations.
1 parent 82a4289 commit 1488fa5

2 files changed

Lines changed: 75 additions & 48 deletions

File tree

‎adev/src/app/core/layout/progress-bar/progress-bar.component.spec.ts‎

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,37 +6,83 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import {Component} from '@angular/core';
910
import {ComponentFixture, TestBed} from '@angular/core/testing';
11+
import {provideRouter, Router} from '@angular/router';
1012

1113
import {PROGRESS_BAR_DELAY, ProgressBarComponent} from './progress-bar.component';
12-
import {RouterTestingHarness} from '@angular/router/testing';
13-
import {provideRouter} from '@angular/router';
14+
15+
@Component({template: ''})
16+
class Page {}
17+
18+
function wait(ms: number): Promise<void> {
19+
return new Promise((resolve) => setTimeout(resolve, ms));
20+
}
1421

1522
describe('ProgressBarComponent', () => {
1623
let component: ProgressBarComponent;
1724
let fixture: ComponentFixture<ProgressBarComponent>;
25+
let router: Router;
26+
let resolveSlowPage: () => void;
27+
let startSpy: jasmine.Spy;
28+
let completeSpy: jasmine.Spy;
1829

1930
beforeEach(async () => {
2031
TestBed.configureTestingModule({
2132
imports: [ProgressBarComponent],
22-
providers: [provideRouter([])],
33+
providers: [
34+
provideRouter([
35+
{path: 'fast', component: Page},
36+
{
37+
path: 'slow',
38+
component: Page,
39+
resolve: {data: () => new Promise<void>((resolve) => (resolveSlowPage = resolve))},
40+
},
41+
]),
42+
],
2343
});
2444

2545
fixture = TestBed.createComponent(ProgressBarComponent);
2646
component = fixture.componentInstance;
47+
router = TestBed.inject(Router);
48+
await fixture.whenStable();
49+
50+
startSpy = spyOn(component.progressBar(), 'start');
51+
completeSpy = spyOn(component.progressBar(), 'complete');
52+
});
53+
54+
it('should start the progress bar for slow navigations and complete it when they end', async () => {
55+
const navigation = router.navigateByUrl('/slow');
56+
await wait(PROGRESS_BAR_DELAY * 2);
57+
58+
expect(startSpy).toHaveBeenCalledTimes(1);
59+
expect(completeSpy).not.toHaveBeenCalled();
60+
61+
resolveSlowPage();
62+
await navigation;
2763
await fixture.whenStable();
64+
65+
expect(startSpy).toHaveBeenCalledTimes(1);
66+
expect(completeSpy).toHaveBeenCalledTimes(1);
2867
});
2968

30-
// This test often timeouts
31-
// We suspect a racing condition inside the RouterTestingHarness.
32-
// Until this has been investigated, we will skip this test.
33-
xit('should call progressBar.complete() on route change', async () => {
34-
const progressBarCompleteSpy = spyOn(component.progressBar(), 'complete');
69+
it('should not start the progress bar for navigations faster than the delay', async () => {
70+
await router.navigateByUrl('/fast');
71+
await fixture.whenStable();
72+
await wait(PROGRESS_BAR_DELAY * 2);
3573

36-
const harness = await RouterTestingHarness.create();
37-
await harness.navigateByUrl('/');
74+
expect(startSpy).not.toHaveBeenCalled();
75+
});
76+
77+
it('should complete the progress bar when a navigation is canceled', async () => {
78+
const navigation = router.navigateByUrl('/slow');
79+
await wait(PROGRESS_BAR_DELAY * 2);
80+
expect(startSpy).toHaveBeenCalledTimes(1);
81+
82+
router.currentNavigation()?.abort();
83+
await navigation;
84+
await fixture.whenStable();
3885

39-
await new Promise((resolve) => setTimeout(resolve, PROGRESS_BAR_DELAY));
40-
expect(progressBarCompleteSpy).toHaveBeenCalled();
86+
expect(completeSpy).toHaveBeenCalledTimes(1);
4187
});
4288
});

‎adev/src/app/core/layout/progress-bar/progress-bar.component.ts‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,9 @@
77
*/
88

99
import {isPlatformServer} from '@angular/common';
10-
import {Component, inject, PLATFORM_ID, viewChild} from '@angular/core';
11-
import {
12-
NavigationCancel,
13-
NavigationEnd,
14-
NavigationError,
15-
NavigationSkipped,
16-
NavigationStart,
17-
Router,
18-
} from '@angular/router';
10+
import {Component, computed, effect, inject, PLATFORM_ID, viewChild} from '@angular/core';
11+
import {Router} from '@angular/router';
1912
import {NgProgressbar, NgProgressRef} from 'ngx-progressbar';
20-
import {filter, map, switchMap, take} from 'rxjs';
2113

2214
/** Time to wait after navigation starts before showing the progress bar. This delay allows a small amount of time to skip showing the progress bar when a navigation is effectively immediate. 30ms is approximately the amount of time we can wait before a delay is perceptible.*/
2315
export const PROGRESS_BAR_DELAY = 30;
@@ -34,6 +26,12 @@ export class ProgressBarComponent {
3426

3527
isServer = isPlatformServer(inject(PLATFORM_ID));
3628

29+
/**
30+
* Id of the ongoing navigation, or `null` when there is none. The `currentNavigation` signal
31+
* notifies on every update of the same navigation, the id only changes once per navigation.
32+
*/
33+
private readonly navigationId = computed(() => this.router.currentNavigation()?.id ?? null);
34+
3735
constructor() {
3836
this.setupPageNavigationDimming();
3937
}
@@ -45,34 +43,17 @@ export class ProgressBarComponent {
4543
if (this.isServer) {
4644
return;
4745
}
48-
this.router.events
49-
.pipe(
50-
filter((e) => e instanceof NavigationStart),
51-
map(() => {
52-
// Only apply set the property if the navigation is not "immediate"
53-
return setTimeout(() => {
54-
this.progressBar().start();
55-
}, PROGRESS_BAR_DELAY);
56-
}),
57-
switchMap((timeoutId) => {
58-
return this.router.events.pipe(
59-
filter((e) => {
60-
return (
61-
e instanceof NavigationEnd ||
62-
e instanceof NavigationCancel ||
63-
e instanceof NavigationSkipped ||
64-
e instanceof NavigationError
65-
);
66-
}),
67-
take(1),
68-
map(() => timeoutId),
69-
);
70-
}),
71-
)
72-
.subscribe((timeoutId) => {
73-
// When the navigation finishes, prevent the navigating class from being applied in the timeout.
46+
effect((onCleanup) => {
47+
if (this.navigationId() === null) {
48+
return;
49+
}
50+
// Only show the progress bar if the navigation is not "immediate".
51+
const timeoutId = setTimeout(() => this.progressBar().start(), PROGRESS_BAR_DELAY);
52+
// Runs when the navigation ends (completed, skipped, canceled or errored) or is superseded.
53+
onCleanup(() => {
7454
clearTimeout(timeoutId);
7555
this.progressBar().complete();
7656
});
57+
});
7758
}
7859
}

0 commit comments

Comments
 (0)