From 6b9e59d0d05df9259b2d1701f5c91e0541a7556b Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Fri, 19 Feb 2016 12:06:46 -0600 Subject: [PATCH] feat(NavController): prevent other lifecycle events from firing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ionic/components/action-sheet/action-sheet.ts | 32 ++++---- ionic/components/alert/alert.ts | 73 +++++++----------- ionic/components/alert/test/basic/index.ts | 8 ++ ionic/components/nav/nav-controller.ts | 26 ++++++- .../nav/test/nav-controller.spec.ts | 76 +++++++++++++++++++ ionic/components/nav/view-controller.ts | 33 ++++---- ionic/components/navbar/navbar.ts | 24 +++--- 7 files changed, 182 insertions(+), 90 deletions(-) diff --git a/ionic/components/action-sheet/action-sheet.ts b/ionic/components/action-sheet/action-sheet.ts index 87f4e7031f6..b93db0ed95d 100644 --- 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 - } = {}) { + 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, - 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; + enableBackdropDismiss?: boolean; +} class ActionSheetSlideIn extends Transition { diff --git a/ionic/components/alert/alert.ts b/ionic/components/alert/alert.ts index 3f56addfc7f..57199e68785 100644 --- 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, - 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, - 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; + buttons?: Array; + 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 index 03543005f83..aeea05ede20 100644 --- 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 index eb06c68423f..5797e09d555 100644 --- 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 index 0ceff3f1262..c62dc7159f1 100644 --- 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 index 268d589ff1c..ad8792aaa3f 100644 --- a/ionic/components/nav/view-controller.ts +++ b/ionic/components/nav/view-controller.ts @@ -64,6 +64,13 @@ export class ViewController { */ onReady: Function; + /** + * @private + * If this is currently the active view, then set to false + * if it does not want the other views to fire their own lifecycles. + */ + fireOtherLifecycles: boolean = true; + /** * @private */ @@ -86,7 +93,7 @@ export class ViewController { /** * @private */ - emit(data: any) { + emit(data?: any) { this._emitter.emit(data); } @@ -94,7 +101,7 @@ export class ViewController { this._onDismiss = callback; } - dismiss(data: any, role?: any) { + dismiss(data?: any, role?: any) { this._onDismiss && this._onDismiss(data, role); return this._nav.remove(this._nav.indexOf(this), 1, this._leavingOpts); } @@ -319,14 +326,14 @@ export class ViewController { * * @returns {boolean} Returns a boolean if this Page has a navbar or not. */ - hasNavbar() { + hasNavbar(): boolean { return !!this.getNavbar(); } /** * @private */ - navbarRef() { + navbarRef(): ElementRef { let navbar = this.getNavbar(); return navbar && navbar.getElementRef(); } @@ -334,7 +341,7 @@ export class ViewController { /** * @private */ - titleRef() { + titleRef(): ElementRef { let navbar = this.getNavbar(); return navbar && navbar.getTitleRef(); } @@ -342,7 +349,7 @@ export class ViewController { /** * @private */ - navbarItemRefs() { + navbarItemRefs(): Array { let navbar = this.getNavbar(); return navbar && navbar.getItemRefs(); } @@ -350,7 +357,7 @@ export class ViewController { /** * @private */ - backBtnRef() { + backBtnRef(): ElementRef { let navbar = this.getNavbar(); return navbar && navbar.getBackButtonRef(); } @@ -358,7 +365,7 @@ export class ViewController { /** * @private */ - backBtnTextRef() { + backBtnTextRef(): ElementRef { let navbar = this.getNavbar(); return navbar && navbar.getBackButtonTextRef(); } @@ -366,7 +373,7 @@ export class ViewController { /** * @private */ - navbarBgRef() { + navbarBgRef(): ElementRef { let navbar = this.getNavbar(); return navbar && navbar.getBackgroundRef(); } @@ -388,7 +395,7 @@ export class ViewController { * * @param {string} backButtonText Set the back button text. */ - setBackButtonText(val) { + setBackButtonText(val: string) { let navbar = this.getNavbar(); if (navbar) { navbar.setBackButtonText(val); @@ -399,7 +406,7 @@ export class ViewController { * Set if the back button for the current view is visible or not. Be sure to wrap this in `onPageWillEnter` to make sure the has been compleltly rendered. * @param {boolean} Set if this Page's back button should show or not. */ - showBackButton(shouldShow) { + showBackButton(shouldShow: boolean) { let navbar = this.getNavbar(); if (navbar) { navbar.hideBackButton = !shouldShow; @@ -491,12 +498,12 @@ export class ViewController { } -function ctrlFn(viewCtrl, fnName) { +function ctrlFn(viewCtrl: ViewController, fnName: string) { if (viewCtrl.instance && viewCtrl.instance[fnName]) { try { viewCtrl.instance[fnName](); } catch(e) { - console.error(fnName + ': ' + e.message); + console.error(viewCtrl.name + ' ' + fnName + ': ' + e.message); } } } diff --git a/ionic/components/navbar/navbar.ts b/ionic/components/navbar/navbar.ts index ea43804530e..3505766bf00 100644 --- a/ionic/components/navbar/navbar.ts +++ b/ionic/components/navbar/navbar.ts @@ -5,6 +5,7 @@ import {Icon} from '../icon/icon'; import {ToolbarBase} from '../toolbar/toolbar'; import {Config} from '../../config/config'; import {IonicApp} from '../app/app'; +import {isTrueProperty} from '../../util/util'; import {ViewController} from '../nav/view-controller'; import {NavController} from '../nav/nav-controller'; @@ -93,7 +94,7 @@ class ToolbarBackground { selector: 'ion-navbar', template: '
' + - '