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

… dialog Co-authored-by: Dima Puris <[email protected]> Co-authored-by: Maksim Ivanov <[email protected]>
taiga-family · Mar 4, 2024 · 936a14b · 936a14b
1 parent a94980c
commit 936a14b
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 18 deletions.
diff --git a/projects/kit/components/routable-dialog/routable-dialog.component.ts b/projects/kit/components/routable-dialog/routable-dialog.component.ts
@@ -14,6 +14,7 @@ import {takeUntil} from 'rxjs';
 export class TuiRoutableDialogComponent {
     private readonly route = inject(ActivatedRoute);
     private readonly router = inject(Router);
+    private readonly initialUrl = this.router.url;
 
     constructor() {
         inject(TuiDialogService)
@@ -25,22 +26,28 @@ export class TuiRoutableDialogComponent {
                 this.route.snapshot.data['dialogOptions'],
             )
             .pipe(takeUntil(inject(TuiDestroyService, {self: true})))
-            .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 {
-        return (this.route.parent?.snapshot.url || []).map(() => '..').join('/');
-    }
 }
diff --git a/projects/kit/components/routable-dialog/test/routable-dialog.component.spec.ts b/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();
+    });
 });