fix(kit): prevent navigation to parent page if navigation occurs from dialog

projects/kit/components/routable-dialog/routable-dialog.component.ts

@@ -1,5 +1,5 @@
 import {ChangeDetectionStrategy, Component, Inject, Injector, Self} from '@angular/core';
-import {ActivatedRoute, Router, UrlSegment} from '@angular/router';
+import {ActivatedRoute, Router} from '@angular/router';
 import {TuiDestroyService} from '@taiga-ui/cdk';
 import {TuiDialogService} from '@taiga-ui/core';
 import {PolymorpheusComponent} from '@tinkoff/ng-polymorpheus';
@@ -12,6 +12,8 @@ import {takeUntil} from 'rxjs/operators';
     providers: [TuiDestroyService],
 })
 export class TuiRoutableDialogComponent {
+    private readonly initialUrl = this.router.url;
+
     constructor(
         @Inject(ActivatedRoute) private readonly route: ActivatedRoute,
         @Inject(Router) private readonly router: Router,
@@ -25,26 +27,26 @@ export class TuiRoutableDialogComponent {
                 this.route.snapshot.data['dialogOptions'],
             )
             .pipe(takeUntil(destroy$))
-            .subscribe({
-                complete: () => this.navigateToParent(),
-            });
+            .subscribe({complete: () => this.onDialogClosing()});
     }
 
-    private navigateToParent(): void {
-        const isLazy = this.route.snapshot.data['isLazy'];
+    private get lazyLoadedBackUrl(): string {
+        return (this.route.parent?.snapshot.url || []).map(() => '..').join('/');
+    }
+
+    private onDialogClosing(): void {
+        if (this.initialUrl === this.router.url) {
+            this.navigateToParent();
+        }
+    }
 
-        const backUrl = isLazy
-            ? this.getLazyLoadedBackUrl()
+    private navigateToParent(): void {
+        const backUrl = this.route.snapshot.data['isLazy']
+            ? this.lazyLoadedBackUrl
             : this.route.snapshot.data['backUrl'];
 
         void this.router.navigate([backUrl], {
             relativeTo: this.route,
         });
     }
-
-    private getLazyLoadedBackUrl(): string {
-        const urlSegments: UrlSegment[] = this.route.parent?.snapshot.url || [];
-
-        return urlSegments.map(() => '..').join('/');
-    }
 }

projects/kit/components/routable-dialog/test/routable-dialog.component.spec.ts

@@ -38,7 +38,10 @@ describe('TuiRoutableDialog', () => {
     let tuiDialogService: TuiDialogService;
     let router: Router;
 
-    function createComponent(activatedRoute?: Partial<ActivatedRoute>): void {
+    function createComponent(
+        activatedRoute?: Partial<ActivatedRoute>,
+        closeDialogImmediately = true,
+    ): void {
         tuiDialogService = mock(TuiDialogService);
         router = mock(Router);
 
@@ -54,16 +57,21 @@ describe('TuiRoutableDialog', () => {
             ],
         }).compileComponents();
 
-        when(tuiDialogService.open(anything(), anything())).thenReturn(NEVER);
+        when(tuiDialogService.open(anything(), anything())).thenReturn(
+            closeDialogImmediately ? EMPTY : NEVER,
+        );
 
         fixture = TestBed.createComponent(TuiRoutableDialogComponent);
     }
 
     it('Dialog content component is passed to the dialog open method, when RoutableDialog is created', () => {
+        // arrange
         createComponent();
 
+        // act
         fixture.detectChanges();
 
+        // assert
         verify(
             tuiDialogService.open(
                 deepEqual(new PolymorpheusComponent(DialogComponent, anything())),
@@ -73,6 +81,7 @@ describe('TuiRoutableDialog', () => {
     });
 
     it('dialog options are passed to the dialog open method', () => {
+        // arrange
         const dialogOptions = {
             dismissible: true,
         };
@@ -86,13 +95,16 @@ describe('TuiRoutableDialog', () => {
             } as unknown as ActivatedRouteSnapshot,
         });
 
+        // act
         fixture.detectChanges();
 
+        // assert
         verify(tuiDialogService.open(anything(), deepEqual(dialogOptions))).once();
     });
 
     it('Closing the dialog navigates back to the parent route for lazy loaded case', fakeAsync(() => {
-        createComponent({
+        // arrange
+        const activatedRouteMock = {
             snapshot: {
                 data: {
                     dialog: DialogComponent,
@@ -114,21 +126,26 @@ describe('TuiRoutableDialog', () => {
                     ],
                 } as unknown as ActivatedRouteSnapshot,
             } as unknown as ActivatedRoute,
-        });
+        };
+
+        createComponent(activatedRouteMock);
 
-        when(tuiDialogService.open(anything(), anything())).thenReturn(EMPTY);
+        // act
+        fixture.detectChanges();
 
+        // assert
         verify(
             router.navigate(
                 deepEqual(['../../..']),
                 deepEqual({
-                    relativeTo: DEFAULT_ACTIVATED_ROUTE_MOCK,
+                    relativeTo: activatedRouteMock,
                 }) as unknown as NavigationExtras,
             ),
-        );
+        ).once();
     }));
 
     it('Closing the dialog navigates back to the parent route for eager loaded case', fakeAsync(() => {
+        // arrange
         createComponent({
             snapshot: {
                 data: {
@@ -138,8 +155,35 @@ describe('TuiRoutableDialog', () => {
             } as unknown as ActivatedRouteSnapshot,
         });
 
-        when(tuiDialogService.open(anything(), anything())).thenReturn(EMPTY);
+        // act
+        fixture.detectChanges();
 
-        verify(router.navigate(deepEqual(['../../..']), anything()));
+        // assert
+        verify(router.navigate(deepEqual(['../../..']), anything())).once();
     }));
+
+    it('if navigation occurs from a dialog, then the navigation to parent is not called', () => {
+        // arrange
+        createComponent(
+            {
+                snapshot: {
+                    data: {
+                        dialog: DialogComponent,
+                        backUrl: '../../..',
+                    } as unknown as Data,
+                } as unknown as ActivatedRouteSnapshot,
+            },
+            false, // will close dialog only on destroy
+        );
+
+        fixture.detectChanges();
+
+        when(router.url).thenReturn('new/route/after/navigation'); // means the url has changed
+
+        // act
+        fixture.destroy(); // should trigger dialog closing logic
+
+        // assert
+        verify(router.navigate(anything(), anything())).never();
+    });
 });