From 68eb1b7cb5043dee67920eee06279fcc62e41a5c Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Mon, 5 Dec 2016 21:53:24 +0100 Subject: [PATCH] fix(viewcontroller): onDidDismiss() is always called fixes #8223 --- src/components/app/app.ts | 7 ++- .../nav/test/worst-case/app-module.ts | 47 ++++++++++--------- src/components/tabs/tabs.ts | 2 +- src/navigation/nav-controller-base.ts | 13 +++-- src/navigation/test/view-controller.spec.ts | 3 +- src/navigation/view-controller.ts | 28 +++++++---- 6 files changed, 60 insertions(+), 40 deletions(-) diff --git a/src/components/app/app.ts b/src/components/app/app.ts index 20436c58e7c..f499b970139 100644 --- a/src/components/app/app.ts +++ b/src/components/app/app.ts @@ -80,7 +80,10 @@ export class App { // During developement, navPop can be triggered by calling // window.ClickBackButton(); if (!window['HWBackButton']) { - window['HWBackButton'] = this.goBack.bind(this); + window['HWBackButton'] = () => { + let p = this.goBack(); + p && p.catch(() => console.debug('hardware go back cancelled')); + }; } }); } @@ -233,7 +236,7 @@ export class App { return this._menuCtrl.close(); } - let navPromise = this.navPop(); + const navPromise = this.navPop(); if (navPromise === null) { // no views to go back to // let's exit the app diff --git a/src/components/nav/test/worst-case/app-module.ts b/src/components/nav/test/worst-case/app-module.ts index e3e1e22900b..983c670567f 100644 --- a/src/components/nav/test/worst-case/app-module.ts +++ b/src/components/nav/test/worst-case/app-module.ts @@ -1,10 +1,12 @@ import { Component, NgModule } from '@angular/core'; import { IonicApp, IonicModule, NavController, NavParams } from '../../../..'; +import { DomSanitizer } from '@angular/platform-browser'; + let LOG = ''; -function log(message: string) { - console.log(message); - LOG += message; +function log(page: string, message: string, color: string) { + console.log(`${page}: ${message}`); + LOG += `${page}:${message}`; LOG += '\n'; } @@ -25,32 +27,32 @@ const TEMPLATE: string = ` export class Base { constructor(public _name: string) { } ionViewWillLoad() { - log(`${this._name} willLoad`); + log(this._name, 'willLoad', 'green'); } ionViewDidLoad() { - log(`${this._name} didLoad`); + log(this._name, 'didLoad', 'green'); } ionViewWillEnter() { - log(`${this._name} willEnter`); + log(this._name, 'willEnter', 'greenyellow'); } ionViewDidEnter() { - log(`${this._name} didEnter`); + log(this._name, 'didEnter', 'cyan'); } ionViewWillLeave() { - log(`${this._name} willLeave`); + log(this._name, 'willLeave', 'greenyellow'); } ionViewDidLeave() { - log(`${this._name} didLeave`); + log(this._name, 'didLeave', 'cyan'); } ionViewWillUnload() { - log(`${this._name} willUnload`); + log(this._name, 'willUnload', 'lightgray'); } ionViewCanLeave(): boolean|Promise { - log(`${this._name} canLeave`); + log(this._name, 'canLeave', 'deeppink'); return true; } ionViewCanEnter(): boolean|Promise { - log(`${this._name} canEnter`); + log(this._name, 'canEnter', '#ff78c1'); return true; } } @@ -76,9 +78,9 @@ export class Page2 extends Base { ionViewWillEnter() { super.ionViewWillEnter(); if (this.counter > 0) { - this.nav.push(Page3); + this.nav.push(Page3, { animated: (this.counter !== 2)}); } else if (this.counter === 0) { - this.nav.push(Page4); + this.nav.push(Page4, {animate: false}); } else { throw new Error('should not be here!'); } @@ -90,12 +92,15 @@ export class Page2 extends Base { template: TEMPLATE }) export class Page3 extends Base { - constructor(private nav: NavController) { + animated: boolean; + constructor(private nav: NavController, params: NavParams) { super('Page3'); + this.animated = params.get('animated'); } + ionViewDidEnter() { - super.ionViewWillEnter(); - this.nav.pop(); + super.ionViewDidEnter(); + this.nav.pop({animate: this.animated}); } } @@ -262,16 +267,16 @@ export class Page8 extends Base { - 
{{result}}
+ 

   
 `
 })
 export class Results {
-  result: string = 'Loading...';
-  constructor(private nav: NavController) {
+  result: any;
+  constructor(private nav: NavController, private sanitizer: DomSanitizer) {
   }
   ionViewDidEnter() {
-    this.result = LOG;
+    this.result = this.sanitizer.bypassSecurityTrustHtml(LOG);
   }
 }
 
diff --git a/src/components/tabs/tabs.ts b/src/components/tabs/tabs.ts
index ee680e941bf..0a94f4ff419 100644
--- a/src/components/tabs/tabs.ts
+++ b/src/components/tabs/tabs.ts
@@ -383,7 +383,7 @@ export class Tabs extends Ion implements AfterViewInit {
     let deselectedPage: ViewController;
     if (deselectedTab) {
       deselectedPage = deselectedTab.getActive();
-      deselectedPage && deselectedPage._willLeave();
+      deselectedPage && deselectedPage._willLeave(false);
     }
 
     opts.animate = false;
diff --git a/src/navigation/nav-controller-base.ts b/src/navigation/nav-controller-base.ts
index fcdb828c39c..392d9950133 100644
--- a/src/navigation/nav-controller-base.ts
+++ b/src/navigation/nav-controller-base.ts
@@ -270,7 +270,10 @@ export class NavControllerBase extends Ion implements NavController {
     const leavingView = this.getActive();
     const enteringView = this._getEnteringView(ti, leavingView);
 
-    assert(leavingView || enteringView, 'both leavingView and enteringView are null');
+    if (!leavingView && !enteringView) {
+      ti.reject('leavingView and enteringView are null. stack is already empty');
+      return false;
+    }
 
     // set that this nav is actively transitioning
     this.setTransitioning(true);
@@ -423,7 +426,7 @@ export class NavControllerBase extends Ion implements NavController {
       this._zone.run(() => {
         for (i = 0; i < destroyQueue.length; i++) {
           view = destroyQueue[i];
-          this._willLeave(view);
+          this._willLeave(view, true);
           this._didLeave(view);
           this._willUnload(view);
         }
@@ -682,7 +685,7 @@ export class NavControllerBase extends Ion implements NavController {
     if (enteringView || leavingView) {
       this._zone.run(() => {
         // Here, the order is important. WillLeave must called before WillEnter.
-        leavingView && this._willLeave(leavingView);
+        leavingView && this._willLeave(leavingView, !enteringView);
         enteringView && this._willEnter(enteringView);
       });
     }
@@ -856,11 +859,11 @@ export class NavControllerBase extends Ion implements NavController {
     this._app.viewDidEnter.emit(view);
   }
 
-  _willLeave(view: ViewController) {
+  _willLeave(view: ViewController, willUnload: boolean) {
     assert(this.isTransitioning(), 'nav controller should be transitioning');
     assert(NgZone.isInAngularZone(), 'callback should be zoned');
 
-    view._willLeave();
+    view._willLeave(willUnload);
     this.viewWillLeave.emit(view);
     this._app.viewWillLeave.emit(view);
   }
diff --git a/src/navigation/test/view-controller.spec.ts b/src/navigation/test/view-controller.spec.ts
index e91a7642c6d..5689a4c2741 100644
--- a/src/navigation/test/view-controller.spec.ts
+++ b/src/navigation/test/view-controller.spec.ts
@@ -50,7 +50,7 @@ describe('ViewController', () => {
       });
 
       // act
-      viewController._willLeave();
+      viewController._willLeave(false);
     }, 10000);
   });
 
@@ -92,6 +92,7 @@ describe('ViewController', () => {
       // arrange
       let viewController = mockView();
       let navControllerBase = mockNavController();
+      navControllerBase._isPortal = true;
       mockViews(navControllerBase, [viewController]);
 
       viewController.onWillDismiss((data: any) => {
diff --git a/src/navigation/view-controller.ts b/src/navigation/view-controller.ts
index f1ee623f253..fc07676687c 100644
--- a/src/navigation/view-controller.ts
+++ b/src/navigation/view-controller.ts
@@ -38,6 +38,8 @@ export class ViewController {
   private _nb: Navbar;
   private _onDidDismiss: Function;
   private _onWillDismiss: Function;
+  private _dismissData: any;
+  private _dismissRole: any;
   private _detached: boolean;
 
   /**
@@ -164,20 +166,16 @@ export class ViewController {
    * @param {any} [role ]
    * @param {NavOptions} NavOptions Options for the dismiss navigation.
    * @returns {any} data Returns the data passed in, if any.
-   *
    */
   dismiss(data?: any, role?: any, navOptions: NavOptions = {}): Promise {
     if (!this._nav) {
       return Promise.resolve(false);
     }
+    this._dismissData = data;
+    this._dismissRole = role;
 
-    let options = assign({}, this._leavingOpts, navOptions);
-    this._onWillDismiss && this._onWillDismiss(data, role);
-    return this._nav.removeView(this, options).then(() => {
-      this._onDidDismiss && this._onDidDismiss(data, role);
-      this._onDidDismiss = null;
-      return data;
-    });
+    const options = assign({}, this._leavingOpts, navOptions);
+    return this._nav.removeView(this, options).then(() => data);
   }
 
   /**
@@ -489,9 +487,14 @@ export class ViewController {
    * @private
    * The view has is about to leave and no longer be the active view.
    */
-  _willLeave() {
+  _willLeave(willUnload: boolean) {
     this.willLeave.emit(null);
     this._lifecycle('WillLeave');
+
+    if (willUnload && this._onWillDismiss) {
+      this._onWillDismiss(this._dismissData, this._dismissRole);
+      this._onWillDismiss = null;
+    }
   }
 
   /**
@@ -517,6 +520,11 @@ export class ViewController {
   _willUnload() {
     this.willUnload.emit(null);
     this._lifecycle('WillUnload');
+
+    this._onDidDismiss && this._onDidDismiss(this._dismissData, this._dismissRole);
+    this._onDidDismiss = null;
+    this._dismissData = null;
+    this._dismissRole = null;
   }
 
   /**
@@ -537,7 +545,7 @@ export class ViewController {
       this._cmp.destroy();
     }
 
-    this._nav = this._cmp = this.instance = this._cntDir = this._cntRef = this._hdrDir = this._ftrDir = this._nb = this._onWillDismiss = null;
+    this._nav = this._cmp = this.instance = this._cntDir = this._cntRef = this._hdrDir = this._ftrDir = this._nb = this._onDidDismiss = this._onWillDismiss = null;
   }
 
   /**