From c8954d8a1677bc5f3b8a7b2fb160e2a832fb9159 Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Tue, 4 Oct 2016 23:28:59 +0200 Subject: [PATCH] fix(nav): ionViewCanLeave does not break navigation fixes #8408 --- src/navigation/nav-controller-base.ts | 286 +++++++++++---------- src/navigation/nav-util.ts | 2 + src/navigation/test/nav-controller.spec.ts | 79 +++++- src/navigation/view-controller.ts | 12 +- src/transitions/transition.ts | 2 +- src/util/mock-providers.ts | 5 +- 6 files changed, 230 insertions(+), 156 deletions(-) diff --git a/src/navigation/nav-controller-base.ts b/src/navigation/nav-controller-base.ts index 28f28676a5..946a074975 100644 --- a/src/navigation/nav-controller-base.ts +++ b/src/navigation/nav-controller-base.ts @@ -4,7 +4,7 @@ import { AnimationOptions } from '../animations/animation'; import { App } from '../components/app/app'; import { Config } from '../config/config'; import { convertToView, convertToViews, NavOptions, DIRECTION_BACK, DIRECTION_FORWARD, INIT_ZINDEX, - TransitionResolveFn, TransitionRejectFn, TransitionInstruction, ViewState } from './nav-util'; + TransitionResolveFn, TransitionInstruction, ViewState } from './nav-util'; import { setZIndex } from './nav-util'; import { DeepLinker } from './deep-linker'; import { GestureController } from '../gestures/gesture-controller'; @@ -254,28 +254,40 @@ export class NavControllerBase extends Ion implements NavController { // there is no transition happening right now // get the next instruction - const ti = this._queue.shift(); + const ti = this._nextTI(); if (!ti) { - this.setTransitioning(false); return false; } - this.setTransitioning(true, ACTIVE_TRANSITION_MAX_TIME); + // Get entering and leaving views + const leavingView = this.getActive(); + const enteringView = this._getEnteringView(ti, leavingView); + + // Initialize enteringView + if (enteringView && isBlank(enteringView._state)) { + // render the entering view, and all child navs and views + // ******** DOM WRITE **************** + this._viewInit(enteringView); + } + + // Only test canLeave/canEnter if there is transition + let requiresTransition = (ti.enteringRequiresTransition || ti.leavingRequiresTransition) && enteringView !== leavingView; + if (requiresTransition) { + // views have been initialized, now let's test + // to see if the transition is even allowed or not + return this._viewTest(enteringView, leavingView, ti); + + } else { + return this._postViewInit(enteringView, leavingView, ti, ti.resolve); + } + } + + _nextTI(): TransitionInstruction { + const ti = this._queue.shift(); + if (!ti) { + return null; + } const viewsLength = this._views.length; - const activeView = this.getActive(); - - let enteringView: ViewController; - let leavingView: ViewController = activeView; - const destroyQueue: ViewController[] = []; - - const opts = ti.opts || {}; - const resolve = ti.resolve; - const reject = ti.reject; - let insertViews = ti.insertViews; - ti.resolve = ti.reject = ti.opts = ti.insertViews = null; - - let enteringRequiresTransition = false; - let leavingRequiresTransition = false; if (isPresent(ti.removeStart)) { if (ti.removeStart < 0) { @@ -284,39 +296,59 @@ export class NavControllerBase extends Ion implements NavController { if (ti.removeCount < 0) { ti.removeCount = (viewsLength - ti.removeStart); } - - leavingRequiresTransition = (ti.removeStart + ti.removeCount === viewsLength); - - for (var i = 0; i < ti.removeCount; i++) { - destroyQueue.push(this._views[i + ti.removeStart]); - } - - for (var i = viewsLength - 1; i >= 0; i--) { - var view = this._views[i]; - if (destroyQueue.indexOf(view) < 0 && view !== leavingView) { - enteringView = view; - break; - } - } - - // default the direction to "back" - opts.direction = opts.direction || DIRECTION_BACK; + ti.leavingRequiresTransition = ((ti.removeStart + ti.removeCount) === viewsLength); } - if (insertViews) { + if (ti.insertViews) { // allow -1 to be passed in to auto push it on the end // and clean up the index if it's larger then the size of the stack if (ti.insertStart < 0 || ti.insertStart > viewsLength) { ti.insertStart = viewsLength; } + ti.enteringRequiresTransition = (ti.insertStart === viewsLength); + } + return ti; + } - // only requires a transition if it's going at the end - enteringRequiresTransition = (ti.insertStart === viewsLength); - + _getEnteringView(ti: TransitionInstruction, leavingView: ViewController): ViewController { + const insertViews = ti.insertViews; + if (insertViews) { // grab the very last view of the views to be inserted // and initialize it as the new entering view - enteringView = insertViews[insertViews.length - 1]; + return insertViews[insertViews.length - 1]; + } + const removeStart = ti.removeStart; + if (isPresent(removeStart)) { + let views = this._views; + const removeEnd = removeStart + ti.removeCount; + for (var i = views.length - 1; i >= 0; i--) { + var view = views[i]; + if ((i < removeStart || i >= removeEnd) && view !== leavingView) { + return view; + } + } + } + return null; + } + + _postViewInit(enteringView: ViewController, leavingView: ViewController, ti: TransitionInstruction, resolve: TransitionResolveFn): boolean { + const opts = ti.opts || {}; + + const insertViews = ti.insertViews; + const removeStart = ti.removeStart; + + let destroyQueue: ViewController[] = []; + + if (isPresent(removeStart)) { + for (var i = 0; i < ti.removeCount; i++) { + destroyQueue.push(this._views[i + removeStart]); + } + // default the direction to "back" + opts.direction = opts.direction || DIRECTION_BACK; + } + + if (insertViews) { // manually set the new view's id if an id was passed in the options if (isPresent(opts.id)) { enteringView.id = opts.id; @@ -345,7 +377,7 @@ export class NavControllerBase extends Ion implements NavController { } } - if (enteringRequiresTransition) { + if (ti.enteringRequiresTransition) { // default to forward if not already set opts.direction = opts.direction || DIRECTION_FORWARD; } @@ -380,7 +412,7 @@ export class NavControllerBase extends Ion implements NavController { } destroyQueue.length = 0; - if (enteringRequiresTransition || leavingRequiresTransition && enteringView !== leavingView) { + if (ti.enteringRequiresTransition || ti.leavingRequiresTransition && enteringView !== leavingView) { // set which animation it should use if it wasn't set yet if (!opts.animation) { if (isPresent(ti.removeStart)) { @@ -391,7 +423,7 @@ export class NavControllerBase extends Ion implements NavController { } // huzzah! let us transition these views - this._transition(enteringView, leavingView, opts, resolve, reject); + this._transition(enteringView, leavingView, opts, resolve); } else { // they're inserting/removing the views somewhere in the middle or @@ -403,7 +435,81 @@ export class NavControllerBase extends Ion implements NavController { return true; } - _transition(enteringView: ViewController, leavingView: ViewController, opts: NavOptions, resolve: TransitionResolveFn, reject: TransitionRejectFn): void { + /** + * DOM WRITE + */ + _viewInit(enteringView: ViewController) { + // entering view has not been initialized yet + const componentProviders = ReflectiveInjector.resolve([ + { provide: NavController, useValue: this }, + { provide: ViewController, useValue: enteringView }, + { provide: NavParams, useValue: enteringView.getNavParams() } + ]); + const componentFactory = this._cfr.resolveComponentFactory(enteringView.component); + const childInjector = ReflectiveInjector.fromResolvedProviders(componentProviders, this._viewport.parentInjector); + + // create ComponentRef and set it to the entering view + enteringView.init(componentFactory.create(childInjector, [])); + enteringView._state = ViewState.INITIALIZED; + } + + _viewTest(enteringView: ViewController, leavingView: ViewController, ti: TransitionInstruction): boolean { + const promises: Promise[] = []; + const reject = ti.reject; + const resolve = ti.resolve; + + if (leavingView) { + const leavingTestResult = leavingView._lifecycleTest('Leave'); + + if (isPresent(leavingTestResult) && leavingTestResult !== true) { + if (leavingTestResult instanceof Promise) { + // async promise + promises.push(leavingTestResult); + + } else { + // synchronous reject + reject((leavingTestResult !== false ? leavingTestResult : `ionViewCanLeave rejected`)); + return false; + } + } + } + + if (enteringView) { + const enteringTestResult = enteringView._lifecycleTest('Enter'); + + if (isPresent(enteringTestResult) && enteringTestResult !== true) { + if (enteringTestResult instanceof Promise) { + // async promise + promises.push(enteringTestResult); + + } else { + // synchronous reject + reject((enteringTestResult !== false ? enteringTestResult : `ionViewCanEnter rejected`)); + return false; + } + } + } + + if (promises.length) { + // darn, async promises, gotta wait for them to resolve + Promise.all(promises).then(() => { + // all promises resolved! let's continue + this._postViewInit(enteringView, leavingView, ti, resolve); + + }).catch((rejectReason) => { + reject(rejectReason); + }); + + return false; + } + + // synchronous and all tests passed! let's move on already + this._postViewInit(enteringView, leavingView, ti, resolve); + + return true; + } + + _transition(enteringView: ViewController, leavingView: ViewController, opts: NavOptions, resolve: TransitionResolveFn): void { // figure out if this transition is the root one or a // child of a parent nav that has the root transition this._trnsId = this._trnsCtrl.getRootTrnsId(this); @@ -449,98 +555,6 @@ export class NavControllerBase extends Ion implements NavController { } }); - if (enteringView && isBlank(enteringView._state)) { - // render the entering view, and all child navs and views - // ******** DOM WRITE **************** - this._viewInit(trns, enteringView, opts); - } - - // views have been initialized, now let's test - // to see if the transition is even allowed or not - const shouldContinue = this._viewTest(trns, enteringView, leavingView, opts, resolve, reject); - if (shouldContinue) { - // synchronous and all tests passed! let's continue - this._postViewInit(trns, enteringView, leavingView, opts, resolve); - } - } - - /** - * DOM WRITE - */ - _viewInit(trns: Transition, enteringView: ViewController, opts: NavOptions) { - // entering view has not been initialized yet - const componentProviders = ReflectiveInjector.resolve([ - { provide: NavController, useValue: this }, - { provide: ViewController, useValue: enteringView }, - { provide: NavParams, useValue: enteringView.getNavParams() } - ]); - const componentFactory = this._cfr.resolveComponentFactory(enteringView.component); - const childInjector = ReflectiveInjector.fromResolvedProviders(componentProviders, this._viewport.parentInjector); - - // create ComponentRef and set it to the entering view - enteringView.init(componentFactory.create(childInjector, [])); - enteringView._state = ViewState.INITIALIZED; - } - - _viewTest(trns: Transition, enteringView: ViewController, leavingView: ViewController, opts: NavOptions, resolve: TransitionResolveFn, reject: TransitionRejectFn): boolean { - const promises: Promise[] = []; - - if (leavingView) { - const leavingTestResult = leavingView._lifecycleTest('Leave'); - - if (isPresent(leavingTestResult) && leavingTestResult !== true) { - if (leavingTestResult instanceof Promise) { - // async promise - promises.push(leavingTestResult); - - } else { - // synchronous reject - reject((leavingTestResult !== false ? leavingTestResult : `ionViewCanLeave rejected`), trns); - return false; - } - } - } - - if (enteringView) { - const enteringTestResult = enteringView._lifecycleTest('Enter'); - - if (isPresent(enteringTestResult) && enteringTestResult !== true) { - if (enteringTestResult instanceof Promise) { - // async promise - promises.push(enteringTestResult); - - } else { - // synchronous reject - reject((enteringTestResult !== false ? enteringTestResult : `ionViewCanEnter rejected`), trns); - return false; - } - } - } - - if (promises.length) { - // darn, async promises, gotta wait for them to resolve - Promise.all(promises).then(() => { - // all promises resolved! let's continue - this._postViewInit(trns, enteringView, leavingView, opts, resolve); - - }, (rejectReason: any) => { - // darn, one of the promises was rejected!! - reject(rejectReason, trns); - - }).catch((rejectReason) => { - // idk, who knows - reject(rejectReason, trns); - }); - - return false; - } - - // synchronous and all tests passed! let's move on already - return true; - } - - _postViewInit(trns: Transition, enteringView: ViewController, leavingView: ViewController, opts: NavOptions, resolve: TransitionResolveFn): void { - // passed both the enter and leave tests if (enteringView && enteringView._state === ViewState.INITIALIZED) { // render the entering component in the DOM // this would also render new child navs/views diff --git a/src/navigation/nav-util.ts b/src/navigation/nav-util.ts index 36bb9b7060..f8a64802aa 100644 --- a/src/navigation/nav-util.ts +++ b/src/navigation/nav-util.ts @@ -170,6 +170,8 @@ export interface TransitionInstruction { removeCount?: number; resolve?: TransitionResolveFn; reject?: TransitionRejectFn; + leavingRequiresTransition?: boolean; + enteringRequiresTransition?: boolean; } export enum ViewState { diff --git a/src/navigation/test/nav-controller.spec.ts b/src/navigation/test/nav-controller.spec.ts index dc7b2722af..206e0f6cf9 100644 --- a/src/navigation/test/nav-controller.spec.ts +++ b/src/navigation/test/nav-controller.spec.ts @@ -231,6 +231,53 @@ describe('NavController', () => { expect(nav.last().component).toEqual(MockView1); }); + it('should not insert any view in the stack if canLeave returns false', () => { + let view1 = mockView(MockView1); + let view2 = mockView(MockView2); + let view3 = mockView(MockView3); + mockViews(nav, [view1, view2]); + + let instance2 = spyOnLifecycles(view2); + + let count = 0; + instance2.ionViewCanLeave = function () { + count++; + return (count === 3); + }; + + nav.push(view3); + expect(nav.length()).toEqual(2); + + nav.push(view3); + expect(nav.length()).toEqual(2); + + nav.push(view3); + expect(nav.length()).toEqual(3); + }); + + it('should not remove any view from the stack if canLeave returns false', () => { + let view1 = mockView(MockView1); + let view2 = mockView(MockView2); + mockViews(nav, [view1, view2]); + + let instance2 = spyOnLifecycles(view2); + + let count = 0; + instance2.ionViewCanLeave = function () { + count++; + return (count === 3); + }; + + nav.pop(); + expect(nav.length()).toEqual(2); + + nav.pop(); + expect(nav.length()).toEqual(2); + + nav.pop(); + expect(nav.length()).toEqual(1); + }); + }); describe('insertPages', () => { @@ -550,14 +597,16 @@ describe('NavController', () => { let view2 = mockView(MockView2); let view3 = mockView(MockView3); let view4 = mockView(MockView4); - mockViews(nav, [view1, view2, view3, view4]); + let view5 = mockView(MockView5); + mockViews(nav, [view1, view2, view3, view4, view5]); let instance1 = spyOnLifecycles(view1); let instance2 = spyOnLifecycles(view2); let instance3 = spyOnLifecycles(view3); let instance4 = spyOnLifecycles(view4); + let instance5 = spyOnLifecycles(view5); - nav.remove(1, 2, null, trnsDone); + nav.remove(2, 2, null, trnsDone); expect(instance1.ionViewDidLoad).not.toHaveBeenCalled(); expect(instance1.ionViewCanEnter).not.toHaveBeenCalled(); @@ -573,9 +622,9 @@ describe('NavController', () => { expect(instance2.ionViewWillEnter).not.toHaveBeenCalled(); expect(instance2.ionViewDidEnter).not.toHaveBeenCalled(); expect(instance2.ionViewCanLeave).not.toHaveBeenCalled(); - expect(instance2.ionViewWillLeave).toHaveBeenCalled(); - expect(instance2.ionViewDidLeave).toHaveBeenCalled(); - expect(instance2.ionViewWillUnload).toHaveBeenCalled(); + expect(instance2.ionViewWillLeave).not.toHaveBeenCalled(); + expect(instance2.ionViewDidLeave).not.toHaveBeenCalled(); + expect(instance2.ionViewWillUnload).not.toHaveBeenCalled(); expect(instance3.ionViewDidLoad).not.toHaveBeenCalled(); expect(instance3.ionViewCanEnter).not.toHaveBeenCalled(); @@ -591,18 +640,28 @@ describe('NavController', () => { expect(instance4.ionViewWillEnter).not.toHaveBeenCalled(); expect(instance4.ionViewDidEnter).not.toHaveBeenCalled(); expect(instance4.ionViewCanLeave).not.toHaveBeenCalled(); - expect(instance4.ionViewWillLeave).not.toHaveBeenCalled(); - expect(instance4.ionViewDidLeave).not.toHaveBeenCalled(); - expect(instance4.ionViewWillUnload).not.toHaveBeenCalled(); + expect(instance4.ionViewWillLeave).toHaveBeenCalled(); + expect(instance4.ionViewDidLeave).toHaveBeenCalled(); + expect(instance4.ionViewWillUnload).toHaveBeenCalled(); + + expect(instance5.ionViewDidLoad).not.toHaveBeenCalled(); + expect(instance5.ionViewCanEnter).not.toHaveBeenCalled(); + expect(instance5.ionViewWillEnter).not.toHaveBeenCalled(); + expect(instance5.ionViewDidEnter).not.toHaveBeenCalled(); + expect(instance5.ionViewCanLeave).not.toHaveBeenCalled(); + expect(instance5.ionViewWillLeave).not.toHaveBeenCalled(); + expect(instance5.ionViewDidLeave).not.toHaveBeenCalled(); + expect(instance5.ionViewWillUnload).not.toHaveBeenCalled(); let hasCompleted = true; let requiresTransition = false; expect(trnsDone).toHaveBeenCalledWith( hasCompleted, requiresTransition, undefined, undefined, undefined ); - expect(nav.length()).toEqual(2); + expect(nav.length()).toEqual(3); expect(nav.getByIndex(0).component).toEqual(MockView1); - expect(nav.getByIndex(1).component).toEqual(MockView4); + expect(nav.getByIndex(1).component).toEqual(MockView2); + expect(nav.getByIndex(2).component).toEqual(MockView5); }); it('should remove the last two views at the end', () => { diff --git a/src/navigation/view-controller.ts b/src/navigation/view-controller.ts index 6075824888..1e0ec901e8 100644 --- a/src/navigation/view-controller.ts +++ b/src/navigation/view-controller.ts @@ -532,18 +532,18 @@ export class ViewController { * @private */ _lifecycleTest(lifecycle: string): boolean | string | Promise { - let result: any = true; - - if (this.instance && this.instance['ionViewCan' + lifecycle]) { + let instance = this.instance; + let methodName = 'ionViewCan' + lifecycle; + if (instance && instance[methodName]) { try { - result = this.instance['ionViewCan' + lifecycle](); + return instance[methodName](); } catch (e) { console.error(`${this.name} ionViewCan${lifecycle} error: ${e}`); - result = false; + return false; } } - return result; + return true; } } diff --git a/src/transitions/transition.ts b/src/transitions/transition.ts index 1a26447a3c..4ac599e3a5 100644 --- a/src/transitions/transition.ts +++ b/src/transitions/transition.ts @@ -21,7 +21,7 @@ export class Transition extends Animation { _trnsStart: Function; parent: Transition; - hasChildTrns: boolean; + hasChildTrns: boolean = false; trnsId: number; diff --git a/src/util/mock-providers.ts b/src/util/mock-providers.ts index 03bea40656..b11cc62a2e 100644 --- a/src/util/mock-providers.ts +++ b/src/util/mock-providers.ts @@ -10,14 +10,13 @@ import { Form } from './form'; import { GestureController } from '../gestures/gesture-controller'; import { Keyboard } from './keyboard'; import { Menu } from '../components/menu/menu'; -import { NavOptions, ViewState, DeepLinkConfig } from '../navigation/nav-util'; +import { ViewState, DeepLinkConfig } from '../navigation/nav-util'; import { OverlayPortal } from '../components/nav/overlay-portal'; import { PageTransition } from '../transitions/page-transition'; import { Platform } from '../platform/platform'; import { QueryParams } from '../platform/query-params'; import { Tab } from '../components/tabs/tab'; import { Tabs } from '../components/tabs/tabs'; -import { Transition } from '../transitions/transition'; import { TransitionController } from '../transitions/transition-controller'; import { UrlSerializer } from '../navigation/url-serializer'; import { ViewController } from '../navigation/view-controller'; @@ -271,7 +270,7 @@ export const mockNavController = function(): NavControllerBase { linker ); - nav._viewInit = function(trns: Transition, enteringView: ViewController, opts: NavOptions) { + nav._viewInit = function(enteringView: ViewController) { enteringView.init(mockComponentRef()); enteringView._state = ViewState.INITIALIZED; };