From 90acad1837b117542830ec0083389a35c5b4ee76 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Fri, 27 Oct 2023 09:17:42 -0400 Subject: [PATCH] fix(angular): NavController works with nested outlets (#28421) Issue number: resolves #28417 --------- ## What is the current behavior? The common `IonRouterOutlet` was trying to inject another common `IonRouterOutlet` into `parentOutlet`: https://github.com/ionic-team/ionic-framework/blob/dc94ae01fe9e6c1b570559f0c8308b31e96cc5bf/packages/angular/common/src/directives/navigation/router-outlet.ts#L119 None existed, so this field was `null`. This is a problem if developers are using the module `IonRouterOutlet` since parent router outlets will not be currently injected because Angular is trying to use the common `IonRouterOutlet` not the module `IonRouterOutlet`: https://github.com/ionic-team/ionic-framework/blob/main/packages/angular/src/directives/navigation/ion-router-outlet.ts. The same goes for the standalone `IonRouterOutlet`. This resulted in things such as `NavController.pop` not working in nested outlets because the parentOutlet was not defined. ## What is the new behavior? - `IonRouterOutlet` now injects the correct router outlet instance for `parentOutlet` ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `7.5.3-dev.11698328998.1a79f815` --- .../navigation/ion-router-outlet.ts | 25 +++++++++++++++++-- .../src/navigation/router-outlet.ts | 25 +++++++++++++++++-- .../base/e2e/src/lazy/nested-outlet.spec.ts | 4 +++ .../test/base/e2e/src/standalone/tabs.spec.ts | 5 ++++ .../nested-outlet-page.component.html | 3 +++ .../nested-outlet-page.component.ts | 6 +++++ .../src/app/standalone/tabs/tab1.component.ts | 9 +++++++ 7 files changed, 73 insertions(+), 4 deletions(-) diff --git a/packages/angular/src/directives/navigation/ion-router-outlet.ts b/packages/angular/src/directives/navigation/ion-router-outlet.ts index 2187f28b37..c84f5ce23b 100644 --- a/packages/angular/src/directives/navigation/ion-router-outlet.ts +++ b/packages/angular/src/directives/navigation/ion-router-outlet.ts @@ -1,8 +1,29 @@ -import { Directive } from '@angular/core'; +import { Location } from '@angular/common'; +import { Directive, Attribute, Optional, SkipSelf, ElementRef, NgZone } from '@angular/core'; +import { Router, ActivatedRoute } from '@angular/router'; import { IonRouterOutlet as IonRouterOutletBase } from '@ionic/angular/common'; @Directive({ selector: 'ion-router-outlet', }) // eslint-disable-next-line @angular-eslint/directive-class-suffix -export class IonRouterOutlet extends IonRouterOutletBase {} +export class IonRouterOutlet extends IonRouterOutletBase { + /** + * We need to pass in the correct instance of IonRouterOutlet + * otherwise parentOutlet will be null in a nested outlet context. + * This results in APIs such as NavController.pop not working + * in nested outlets because the parent outlet cannot be found. + */ + constructor( + @Attribute('name') name: string, + @Optional() @Attribute('tabs') tabs: string, + commonLocation: Location, + elementRef: ElementRef, + router: Router, + zone: NgZone, + activatedRoute: ActivatedRoute, + @SkipSelf() @Optional() readonly parentOutlet?: IonRouterOutlet + ) { + super(name, tabs, commonLocation, elementRef, router, zone, activatedRoute, parentOutlet); + } +} diff --git a/packages/angular/standalone/src/navigation/router-outlet.ts b/packages/angular/standalone/src/navigation/router-outlet.ts index 2b750736e0..d5e6becab2 100644 --- a/packages/angular/standalone/src/navigation/router-outlet.ts +++ b/packages/angular/standalone/src/navigation/router-outlet.ts @@ -1,4 +1,6 @@ -import { Directive } from '@angular/core'; +import { Location } from '@angular/common'; +import { Directive, Attribute, Optional, SkipSelf, ElementRef, NgZone } from '@angular/core'; +import { Router, ActivatedRoute } from '@angular/router'; import { IonRouterOutlet as IonRouterOutletBase, ProxyCmp } from '@ionic/angular/common'; import { defineCustomElement } from '@ionic/core/components/ion-router-outlet.js'; @@ -10,4 +12,23 @@ import { defineCustomElement } from '@ionic/core/components/ion-router-outlet.js standalone: true, }) // eslint-disable-next-line @angular-eslint/directive-class-suffix -export class IonRouterOutlet extends IonRouterOutletBase {} +export class IonRouterOutlet extends IonRouterOutletBase { + /** + * We need to pass in the correct instance of IonRouterOutlet + * otherwise parentOutlet will be null in a nested outlet context. + * This results in APIs such as NavController.pop not working + * in nested outlets because the parent outlet cannot be found. + */ + constructor( + @Attribute('name') name: string, + @Optional() @Attribute('tabs') tabs: string, + commonLocation: Location, + elementRef: ElementRef, + router: Router, + zone: NgZone, + activatedRoute: ActivatedRoute, + @SkipSelf() @Optional() readonly parentOutlet?: IonRouterOutlet + ) { + super(name, tabs, commonLocation, elementRef, router, zone, activatedRoute, parentOutlet); + } +} diff --git a/packages/angular/test/base/e2e/src/lazy/nested-outlet.spec.ts b/packages/angular/test/base/e2e/src/lazy/nested-outlet.spec.ts index 8a95ab408c..786b2b44ef 100644 --- a/packages/angular/test/base/e2e/src/lazy/nested-outlet.spec.ts +++ b/packages/angular/test/base/e2e/src/lazy/nested-outlet.spec.ts @@ -27,5 +27,9 @@ describe('Nested Outlet', () => { cy.get('#goto-nested-page2').click(); }); + // Fixes https://github.com/ionic-team/ionic-framework/issues/28417 + it('parentOutlet should be defined', () => { + cy.get('#parent-outlet span').should('have.text', 'true'); + }); }); diff --git a/packages/angular/test/base/e2e/src/standalone/tabs.spec.ts b/packages/angular/test/base/e2e/src/standalone/tabs.spec.ts index a0558f299f..bb0901a112 100644 --- a/packages/angular/test/base/e2e/src/standalone/tabs.spec.ts +++ b/packages/angular/test/base/e2e/src/standalone/tabs.spec.ts @@ -13,4 +13,9 @@ describe('Tabs', () => { cy.get('app-tab-two').should('be.visible'); cy.contains('Tab 2'); }); + + // Fixes https://github.com/ionic-team/ionic-framework/issues/28417 + it('parentOutlet should be defined', () => { + cy.get('#parent-outlet span').should('have.text', 'true'); + }); }); diff --git a/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.html b/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.html index 10d40ff324..56ac85776d 100644 --- a/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.html +++ b/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.html @@ -4,4 +4,7 @@ Go To Tabs Go To SECOND

+

+ Has Parent Outlet: {{ hasParentOutlet }} +

diff --git a/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.ts b/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.ts index 755aae861b..1b62c87c71 100644 --- a/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.ts +++ b/packages/angular/test/base/src/app/lazy/nested-outlet-page/nested-outlet-page.component.ts @@ -1,10 +1,16 @@ import { Component, NgZone, OnDestroy, OnInit } from '@angular/core'; +import { IonRouterOutlet } from '@ionic/angular'; @Component({ selector: 'app-nested-outlet-page', templateUrl: './nested-outlet-page.component.html', }) export class NestedOutletPageComponent implements OnDestroy, OnInit { + hasParentOutlet = false; + + constructor(private routerOutlet: IonRouterOutlet) { + this.hasParentOutlet = routerOutlet.parentOutlet != null; + } ngOnInit() { NgZone.assertInAngularZone(); diff --git a/packages/angular/test/base/src/app/standalone/tabs/tab1.component.ts b/packages/angular/test/base/src/app/standalone/tabs/tab1.component.ts index 8b6e8976ec..3bd2bd270b 100644 --- a/packages/angular/test/base/src/app/standalone/tabs/tab1.component.ts +++ b/packages/angular/test/base/src/app/standalone/tabs/tab1.component.ts @@ -1,12 +1,21 @@ import { Component } from '@angular/core'; +import { IonRouterOutlet } from '@ionic/angular/standalone'; @Component({ selector: 'app-tab-one', template: ` Tab 1 + +

+ Has Parent Outlet: {{ hasParentOutlet }} +

`, standalone: true, }) export class TabOneComponent { + hasParentOutlet = false; + constructor(private routerOutlet: IonRouterOutlet) { + this.hasParentOutlet = routerOutlet.parentOutlet != null; + } }