fix(location-strategy): crash on going back with router-outlet after closing modal (#1748)

* fix(location-strategy): crash on going back with router-outlet after closing modal * tests: add e2e tests in single-page app * chore: bump version to 7.2.2

Author: ADjenkov

e2e/single-page/app/app.css

@@ -1,5 +1,5 @@
 .title {
- font-size: 30;
+ font-size: 15;
  margin: 16;
 }
 

e2e/single-page/app/app.module.ts

@@ -11,14 +11,19 @@ import { AppComponent } from "./app.component";
 
 import { rendererTraceCategory, viewUtilCategory, routeReuseStrategyTraceCategory, routerTraceCategory } from "nativescript-angular/trace";
 import { setCategories, enable } from "tns-core-modules/trace";
+import { ModalComponent } from "./second/modal/modal.component";
 setCategories(routerTraceCategory + "," + routeReuseStrategyTraceCategory);
 enable();
 
 @NgModule({
  declarations: [
  AppComponent,
+ ModalComponent,
  ...navigatableComponents,
  ],
+ entryComponents:[
+ ModalComponent
+ ],
  bootstrap: [AppComponent],
  providers: [
  { provide: NgModuleFactoryLoader, useClass: NSModuleFactoryLoader }

e2e/single-page/app/second/modal/modal.component.html

@@ -0,0 +1,4 @@
+<StackLayout>
+ <Label text="Welcome to modal"></Label>
+ <Button text="Close Modal" (tap)="close()"></Button>
+</StackLayout>

e2e/single-page/app/second/modal/modal.component.ts

@@ -0,0 +1,19 @@
+import { Component } from '@angular/core';
+import { ModalDialogParams } from "nativescript-angular/modal-dialog";
+
+@Component({
+ moduleId: module.id,
+ selector: 'modal',
+ templateUrl: './modal.component.html'
+})
+
+export class ModalComponent {
+
+ constructor(private params: ModalDialogParams) {
+ }
+
+ public close() {
+ this.params.closeCallback();
+ }
+
+}

e2e/single-page/app/second/second.component.ts

@@ -1,10 +1,12 @@
-import { Component, OnInit, OnDestroy } from "@angular/core";
+import { Component, OnInit, OnDestroy, ViewContainerRef } from "@angular/core";
 import { ActivatedRoute, Router, Route } from "@angular/router";
 
+import { ModalDialogService, ModalDialogOptions } from "nativescript-angular";
 import { Page } from "tns-core-modules/ui/page";
 import { Observable } from "rxjs";
 import { map } from "rxjs/operators";
 import { RouterExtensions } from "nativescript-angular/router";
+import { ModalComponent } from "./modal/modal.component";
 
 @Component({
  selector: "second",
@@ -19,12 +21,16 @@ import { RouterExtensions } from "nativescript-angular/router";
 
  <StackLayout>
  <Label [text]="'Second Component: ' + (id$ | async)" class="title"></Label>
+ <Button text="Show Modal" (tap)="onShowModal()"></Button>
  <Button text="Back" (tap)="back()"></Button>
  </StackLayout>`
 })
 export class SecondComponent implements OnInit, OnDestroy {
  public id$: Observable<number>;
- constructor(route: ActivatedRoute, private routerExtensions: RouterExtensions) {
+ constructor(route: ActivatedRoute,
+ private routerExtensions: RouterExtensions,
+ private viewContainerRef: ViewContainerRef,
+ private modalService: ModalDialogService) {
  this.id$ = route.params.pipe(map(r => +r["id"]));
  }
 
@@ -39,4 +45,16 @@ export class SecondComponent implements OnInit, OnDestroy {
  back() {
  this.routerExtensions.back();
  }
+
+ onShowModal() {
+ let options: ModalDialogOptions = {
+ viewContainerRef: this.viewContainerRef,
+ context: {
+ },
+ fullscreen: true
+ };
+
+ this.modalService.showModal(ModalComponent, options).then((dialogResult: string) => {
+ });
+ }
 }

e2e/single-page/e2e/tests.e2e-spec.ts

@@ -19,29 +19,45 @@ describe("Single page app", () => {
  });
 
  it("should load second(1) page", async () => {
- await findAndClick(driver, "SECOND(1)")
+ await findAndClick(driver, "SECOND(1)");
 
  await driver.findElementByAutomationText("Second Component: 1");
- 
+
  // ActionBar Title and item
  await driver.findElementByAutomationText("Second Title");
  await driver.findElementByAutomationText("ACTION2");
  });
 
  it("should load second(2) page", async () => {
- await findAndClick(driver, "SECOND(2)")
+ await findAndClick(driver, "SECOND(2)");
+
+ await driver.findElementByAutomationText("Second Component: 2");
 
- await driver.findElementByAutomationText("Second Component: 1");
- 
  // ActionBar Title and items
  await driver.findElementByAutomationText("Second Title");
  await driver.findElementByAutomationText("ACTION2");
  await driver.findElementByAutomationText("ADD");
  });
+
+ it("should open and close modal view", async () => {
+ await findAndClick(driver, "Show Modal");
+
+ await driver.findElementByAutomationText("Welcome to modal");
+ await findAndClick(driver, "Close Modal");
+
+ await driver.findElementByAutomationText("Second Component: 2");
+ });
+
+ it("should go back to second(1) and first", async () => {
+ await findAndClick(driver, "Back");
+ await driver.findElementByAutomationText("Second Component: 1");
+ await findAndClick(driver, "Back");
+ await driver.findElementByAutomationText("First Title");
+ await driver.findElementByAutomationText("ACTION1");
+ });
 });
 
 async function findAndClick(driver: AppiumDriver, text: string) {
- const navigationButton =
- await driver.findElementByAutomationText(text);
- navigationButton.click();
+ const navigationButton = await driver.findElementByAutomationText(text);
+ await navigationButton.click();
 }

nativescript-angular/directives/dialogs.ts

@@ -87,9 +87,7 @@ export class ModalDialogService {
  frame = (parentView.page && parentView.page.frame) || topmost();
  }
 
- if (frame) {
- this.location._beginModalNavigation(frame);
- }
+ this.location._beginModalNavigation(frame);
 
  return new Promise((resolve, reject) => {
  setTimeout(() => {

nativescript-angular/package.json

@@ -1,6 +1,6 @@
 {
  "name": "nativescript-angular",
- "version": "7.2.2",
+ "version": "7.2.3",
  "description": "An Angular renderer that lets you build mobile apps with NativeScript.",
  "homepage": "https://www.nativescript.org/",
  "bugs": "https://github.com/NativeScript/nativescript-angular/issues",

nativescript-angular/router/ns-location-strategy.ts

@@ -275,9 +275,13 @@ export class NSLocationStrategy extends LocationStrategy {
 
  if (!outlet) {
  const topmostFrame = this.frameService.getFrame();
- this.currentOutlet = this.getOutletByFrame(topmostFrame);
+ this.currentOutlet = this.getOutletByFrame(topmostFrame) || this.currentOutlet;
+ }
+
+ const frameToBack: Frame = this.currentOutlet.getFrameToBack();
+ if (frameToBack) {
+ frameToBack.goBack();
  }
- this.currentOutlet.getFrameToBack().goBack();
  } else {
  // Nested navigation - just pop the state
  if (isLogEnabled()) {
@@ -385,10 +389,11 @@ export class NSLocationStrategy extends LocationStrategy {
  routerLog("NSLocationStrategy._beginModalNavigation()");
  }
 
- this.currentOutlet = this.getOutletByFrame(frame);
+ this.currentOutlet = this.getOutletByFrame(frame) || this.currentOutlet;
 
  // It is possible to have frame, but not corresponding Outlet, if
- // showing modal dialog on app.component.ts ngOnInit() e.g.
+ // showing modal dialog on app.component.ts ngOnInit() e.g. In that case
+ // the modal is treated as none modal navigation.
  if (this.currentOutlet) {
  this.currentOutlet.showingModal = true;
  this._modalNavigationDepth++;
@@ -400,15 +405,18 @@ export class NSLocationStrategy extends LocationStrategy {
  routerLog("NSLocationStrategy.closeModalNavigation()");
  }
 
- if (this._modalNavigationDepth > 0) {
+ const isShowingModal = this._modalNavigationDepth > 0;
+
+ if (isShowingModal) {
  this._modalNavigationDepth--;
  }
 
  // currentOutlet should be the one that corresponds to the topmost() frame
  const topmostOutlet = this.getOutletByFrame(this.frameService.getFrame());
- this.currentOutlet = this.findOutletByModal(this._modalNavigationDepth, true) || topmostOutlet;
+ const outlet = this.findOutletByModal(this._modalNavigationDepth, isShowingModal) || topmostOutlet;
 
- if (this.currentOutlet) {
+ if (outlet) {
+ this.currentOutlet = outlet;
  this.currentOutlet.showingModal = false;
  this.callPopState(this.currentOutlet.peekState(), false);
  }
@@ -481,7 +489,8 @@ export class NSLocationStrategy extends LocationStrategy {
  this.callPopState(null, true, currentOutlet);
  }
 
- if (!currentOutlet.isNSEmptyOutlet) {
+ // Skip frames filtering since currentOutlet is <router-outlet> when no frames available.
+ if (currentOutlet.frames.length && !currentOutlet.isNSEmptyOutlet) {
  currentOutlet.frames = currentOutlet.frames.filter(currentFrame => currentFrame !== frame);
  return currentOutlet.frames.length;
  }
@@ -554,26 +563,32 @@ export class NSLocationStrategy extends LocationStrategy {
  return pathToOutlet || lastPath;
  }
 
- findOutletByModal(modalNavigation: number, isShowingModal?: boolean): Outlet {
- return this.outlets.find((outlet) => {
- let isEqual = outlet.modalNavigationDepth === modalNavigation;
- return isShowingModal ? isEqual && outlet.showingModal : isEqual;
- });
- }
-
  findOutlet(outletKey: string, activatedRouteSnapshot?: ActivatedRouteSnapshot): Outlet {
- let outlet: Outlet = this.outlets.find((currentOutlet) => currentOutlet.outletKeys.indexOf(outletKey) > -1);
+ let outlet: Outlet = this.outlets.find((currentOutlet) => {
+ let equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
+ return equalModalDepth && currentOutlet.outletKeys.indexOf(outletKey) > -1;
+ });
 
  // No Outlet with the given outletKey could happen when using nested unnamed p-r-o
  // primary -> primary -> prymary
  if (!outlet && activatedRouteSnapshot) {
  const pathByOutlets = this.getPathByOutlets(activatedRouteSnapshot);
- outlet = this.outlets.find((currentOutlet) => currentOutlet.pathByOutlets === pathByOutlets);
+ outlet = this.outlets.find((currentOutlet) => {
+ let equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
+ return equalModalDepth && currentOutlet.pathByOutlets === pathByOutlets;
+ });
  }
 
  return outlet;
  }
 
+ private findOutletByModal(modalNavigation: number, isShowingModal?: boolean): Outlet {
+ return this.outlets.find((outlet) => {
+ let equalModalDepth = outlet.modalNavigationDepth === modalNavigation;
+ return isShowingModal ? equalModalDepth && outlet.showingModal : equalModalDepth;
+ });
+ }
+
  private getOutletByFrame(frame: Frame): Outlet {
  let outlet;
 

nativescript-angular/router/page-router-outlet.ts

@@ -422,15 +422,8 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
 
  private getOutlet(activatedRouteSnapshot: ActivatedRouteSnapshot): Outlet {
  const topActivatedRoute = findTopActivatedRouteNodeForOutlet(activatedRouteSnapshot);
- const modalNavigation = this.locationStrategy._modalNavigationDepth;
  const outletKey = this.locationStrategy.getRouteFullPath(topActivatedRoute);
- let outlet;
-
- if (modalNavigation > 0) { // Modal with 'primary' p-r-o
- outlet = this.locationStrategy.findOutletByModal(modalNavigation);
- } else {
- outlet = this.locationStrategy.findOutlet(outletKey, topActivatedRoute);
- }
+ let outlet = this.locationStrategy.findOutlet(outletKey, topActivatedRoute);
 
  // Named lazy loaded outlet.
  if (!outlet && this.isEmptyOutlet) {