feat(NavController): prevent other lifecycle events from firing

adamdbradley · adamdbradley · commit 6b9e59d0d05d · 2016-02-19T12:06:49.000-06:00
For Alert and ActionSheet, the currently active page’s leaving lifecycle event should not fire. However, for Modal, the currently active page’s leaving lifecycle should fire. Closes #5516
diff --git a/ionic/components/action-sheet/action-sheet.ts b/ionic/components/action-sheet/action-sheet.ts
@@ -80,24 +80,23 @@ import {ViewController} from '../nav/view-controller';
  */
  export class ActionSheet extends ViewController {
 
-   constructor(opts: {
-     title?: string,
-     subTitle?: string,
-     cssClass?: string,
-     enableBackdropDismiss?: boolean,
-     buttons?: Array<any>
-   } = {}) {
+   constructor(opts: ActionSheetOptions = {}) {
      opts.buttons = opts.buttons || [];
      opts.enableBackdropDismiss = isDefined(opts.enableBackdropDismiss) ? !!opts.enableBackdropDismiss : true;
 
      super(ActionSheetCmp, opts);
      this.viewType = 'action-sheet';
+
+    // by default, actionsheets should not fire lifecycle events of other views
+    // for example, when an actionsheets enters, the current active view should
+    // not fire its lifecycle events because it's not conceptually leaving
+    this.fireOtherLifecycles = false;
    }
 
    /**
    * @private
    */
-   getTransitionName(direction) {
+   getTransitionName(direction: string) {
      let key = 'actionSheet' + (direction === 'back' ? 'Leave' : 'Enter');
      return this._nav && this._nav.config.get(key);
    }
@@ -119,7 +118,7 @@ import {ViewController} from '../nav/view-controller';
    /**
     * @param {object} button Action sheet button
     */
-   addButton(button) {
+   addButton(button: any) {
      this.data.buttons.push(button);
    }
 
@@ -148,13 +147,7 @@ import {ViewController} from '../nav/view-controller';
     *
     * @param {object} opts Action sheet options
     */
-   static create(opts: {
-     title?: string,
-     subTitle?: string,
-     cssClass?: string,
-     buttons?: Array<any>,
-     enableBackdropDismiss?: boolean
-   } = {}) {
+   static create(opts: ActionSheetOptions = {}) {
      return new ActionSheet(opts);
    }
 
@@ -294,6 +287,13 @@ class ActionSheetCmp {
   }
 }
 
+export interface ActionSheetOptions {
+  title?: string;
+  subTitle?: string;
+  cssClass?: string;
+  buttons?: Array<any>;
+  enableBackdropDismiss?: boolean;
+}
 
 
 class ActionSheetSlideIn extends Transition {
diff --git a/ionic/components/alert/alert.ts b/ionic/components/alert/alert.ts
@@ -119,35 +119,24 @@ import {ViewController} from '../nav/view-controller';
  */
 export class Alert extends ViewController {
 
-  constructor(opts: {
-    title?: string,
-    subTitle?: string,
-    message?: string,
-    cssClass?: string,
-    inputs?: Array<{
-      type?: string,
-      name?: string,
-      placeholder?: string,
-      value?: string,
-      label?: string,
-      checked?: boolean,
-      id?: string
-    }>,
-    buttons?: Array<any>,
-    enableBackdropDismiss?: boolean
-  } = {}) {
+  constructor(opts: AlertOptions = {}) {
     opts.inputs = opts.inputs || [];
     opts.buttons = opts.buttons || [];
     opts.enableBackdropDismiss = isDefined(opts.enableBackdropDismiss) ? !!opts.enableBackdropDismiss : true;
 
     super(AlertCmp, opts);
     this.viewType = 'alert';
+
+    // by default, alerts should not fire lifecycle events of other views
+    // for example, when an alert enters, the current active view should
+    // not fire its lifecycle events because it's not conceptually leaving
+    this.fireOtherLifecycles = false;
   }
 
   /**
   * @private
   */
-  getTransitionName(direction) {
+  getTransitionName(direction: string) {
     let key = (direction === 'back' ? 'alertLeave' : 'alertEnter');
     return this._nav && this._nav.config.get(key);
   }
@@ -185,15 +174,7 @@ export class Alert extends ViewController {
   /**
    * @param {object} input Alert input
    */
-  addInput(input: {
-    type?: string,
-    name?: string,
-    placeholder?: string,
-    value?: string,
-    label?: string,
-    checked?: boolean,
-    id?: string
-  }) {
+  addInput(input: AlertInputOptions) {
     this.data.inputs.push(input);
   }
 
@@ -214,23 +195,7 @@ export class Alert extends ViewController {
   /**
    * @param {object} opts Alert options
    */
-  static create(opts: {
-    title?: string,
-    subTitle?: string,
-    message?: string,
-    cssClass?: string,
-    inputs?: Array<{
-      type?: string,
-      name?: string,
-      placeholder?: string,
-      value?: string,
-      label?: string,
-      checked?: boolean,
-      id?: string
-    }>,
-    buttons?: Array<any>,
-    enableBackdropDismiss?: boolean
-  } = {}) {
+  static create(opts: AlertOptions = {}) {
     return new Alert(opts);
   }
 
@@ -485,6 +450,26 @@ class AlertCmp {
   }
 }
 
+export interface AlertOptions {
+  title?: string;
+  subTitle?: string;
+  message?: string;
+  cssClass?: string;
+  inputs?: Array<AlertInputOptions>;
+  buttons?: Array<any>;
+  enableBackdropDismiss?: boolean;
+}
+
+export interface AlertInputOptions {
+  type?: string;
+  name?: string;
+  placeholder?: string;
+  value?: string;
+  label?: string;
+  checked?: boolean;
+  id?: string;
+}
+
 
 /**
  * Animations for alerts
diff --git a/ionic/components/alert/test/basic/index.ts b/ionic/components/alert/test/basic/index.ts
@@ -260,6 +260,14 @@ class E2EPage {
 
     this.nav.present(alert);
   }
+
+  onPageDidLeave() {
+    console.log('E2EPage, onPageDidLeave');
+  }
+
+  onPageDidEnter() {
+    console.log('E2EPage, onPageDidEnter');
+  }
 }
 
 
diff --git a/ionic/components/nav/nav-controller.ts b/ionic/components/nav/nav-controller.ts
@@ -978,8 +978,17 @@ export class NavController extends Ion {
       }
 
       // call each view's lifecycle events
-      enteringView.willEnter();
-      leavingView.willLeave();
+      if (leavingView.fireOtherLifecycles) {
+        // only fire entering lifecycle if the leaving
+        // view hasn't explicitly set not to
+        enteringView.willEnter();
+      }
+
+      if (enteringView.fireOtherLifecycles) {
+        // only fire leaving lifecycle if the entering
+        // view hasn't explicitly set not to
+        leavingView.willLeave();
+      }
 
     } else {
       // this view is being preloaded, don't call lifecycle events
@@ -1080,8 +1089,17 @@ export class NavController extends Ion {
     this._zone.run(() => {
 
       if (!opts.preload && hasCompleted) {
-        enteringView.didEnter();
-        leavingView.didLeave();
+        if (leavingView.fireOtherLifecycles) {
+          // only fire entering lifecycle if the leaving
+          // view hasn't explicitly set not to
+          enteringView.didEnter();
+        }
+
+        if (enteringView.fireOtherLifecycles) {
+          // only fire leaving lifecycle if the entering
+          // view hasn't explicitly set not to
+          leavingView.didLeave();
+        }
       }
 
       if (enteringView.state === STATE_INACTIVE) {
diff --git a/ionic/components/nav/test/nav-controller.spec.ts b/ionic/components/nav/test/nav-controller.spec.ts
@@ -393,6 +393,42 @@ export function run() {
         expect(leavingView.willLeave).toHaveBeenCalled();
       });
 
+      it('should not call willEnter when the leaving view has fireOtherLifecycles not true', () => {
+        let enteringView = new ViewController(Page1);
+        let leavingView = new ViewController(Page2);
+        var navOptions: NavOptions = {};
+        var done = () => {};
+        nav._beforeTrans = () => {}; //prevent running beforeTrans for tests
+
+        spyOn(enteringView, 'willEnter');
+        spyOn(leavingView, 'willLeave');
+
+        leavingView.fireOtherLifecycles = false;
+
+        nav._postRender(1, enteringView, leavingView, false, navOptions, done);
+
+        expect(enteringView.willEnter).not.toHaveBeenCalled();
+        expect(leavingView.willLeave).toHaveBeenCalled();
+      });
+
+      it('should not call willLeave when the entering view has fireOtherLifecycles not true', () => {
+        let enteringView = new ViewController(Page1);
+        let leavingView = new ViewController(Page2);
+        var navOptions: NavOptions = {};
+        var done = () => {};
+        nav._beforeTrans = () => {}; //prevent running beforeTrans for tests
+
+        spyOn(enteringView, 'willEnter');
+        spyOn(leavingView, 'willLeave');
+
+        enteringView.fireOtherLifecycles = false;
+
+        nav._postRender(1, enteringView, leavingView, false, navOptions, done);
+
+        expect(enteringView.willEnter).toHaveBeenCalled();
+        expect(leavingView.willLeave).not.toHaveBeenCalled();
+      });
+
       it('should not call willLeave on leaving view when it is being preloaded', () => {
         let enteringView = new ViewController(Page1);
         let leavingView = new ViewController(Page2);
@@ -495,6 +531,46 @@ export function run() {
         expect(doneCalled).toBe(true);
       });
 
+      it('should not call didLeave when enteringView set fireOtherLifecycles to false', () => {
+        let enteringView = new ViewController();
+        let leavingView = new ViewController();
+        let navOpts: NavOptions = {};
+        let hasCompleted = true;
+        let doneCalled = false;
+        let done = () => {doneCalled = true;}
+
+        enteringView.fireOtherLifecycles = false;
+
+        spyOn(enteringView, 'didEnter');
+        spyOn(leavingView, 'didLeave');
+
+        nav._afterTrans(enteringView, leavingView, navOpts, hasCompleted, done);
+
+        expect(enteringView.didEnter).toHaveBeenCalled();
+        expect(leavingView.didLeave).not.toHaveBeenCalled();
+        expect(doneCalled).toBe(true);
+      });
+
+      it('should not call didEnter when leavingView set fireOtherLifecycles to false', () => {
+        let enteringView = new ViewController();
+        let leavingView = new ViewController();
+        let navOpts: NavOptions = {};
+        let hasCompleted = true;
+        let doneCalled = false;
+        let done = () => {doneCalled = true;}
+
+        leavingView.fireOtherLifecycles = false;
+
+        spyOn(enteringView, 'didEnter');
+        spyOn(leavingView, 'didLeave');
+
+        nav._afterTrans(enteringView, leavingView, navOpts, hasCompleted, done);
+
+        expect(enteringView.didEnter).not.toHaveBeenCalled();
+        expect(leavingView.didLeave).toHaveBeenCalled();
+        expect(doneCalled).toBe(true);
+      });
+
       it('should not call didEnter/didLeave when not hasCompleted', () => {
         let enteringView = new ViewController();
         let leavingView = new ViewController();
diff --git a/ionic/components/nav/view-controller.ts b/ionic/components/nav/view-controller.ts
diff --git a/ionic/components/navbar/navbar.ts b/ionic/components/navbar/navbar.ts

Original file line number	Diff line number	Diff line change
`@@ -260,6 +260,14 @@ class E2EPage {`
`260`	`260`
`261`	`261`	`this.nav.present(alert);`
`262`	`262`	`}`
	`263`	`+`
	`264`	`+ onPageDidLeave() {`
	`265`	`+ console.log('E2EPage, onPageDidLeave');`
	`266`	`+ }`
	`267`	`+`
	`268`	`+ onPageDidEnter() {`
	`269`	`+ console.log('E2EPage, onPageDidEnter');`
	`270`	`+ }`
`263`	`271`	`}`
`264`	`272`
`265`	`273`