From 86fc741e6303a9cd39aaf1a5f8cd541f1b9ae287 Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Wed, 6 Apr 2016 16:51:05 -0500 Subject: [PATCH] fix(nav): portal nav should always animate Closes #6059 --- ionic/components/nav/nav-controller.ts | 34 ++++++---- ionic/components/nav/nav-portal.ts | 1 + .../nav/test/nav-controller.spec.ts | 65 ++++++++++++++++--- 3 files changed, 76 insertions(+), 24 deletions(-) diff --git a/ionic/components/nav/nav-controller.ts b/ionic/components/nav/nav-controller.ts index 2c710c93f3..f12ccc0117 100644 --- a/ionic/components/nav/nav-controller.ts +++ b/ionic/components/nav/nav-controller.ts @@ -142,6 +142,11 @@ export class NavController extends Ion { */ config: Config; + /** + * @private + */ + isPortal: boolean = false; + constructor( parent: any, protected _app: IonicApp, @@ -425,6 +430,10 @@ export class NavController extends Ion { opts = {}; } + if (enteringView.usePortal && this._portal) { + return this._portal.present(enteringView, opts); + } + enteringView.setNav(rootNav); opts.keyboardClose = false; @@ -440,11 +449,6 @@ export class NavController extends Ion { animation: enteringView.getTransitionName('back') }); - if (enteringView.usePortal && this._portal) { - this._portal.present(enteringView); - return; - } - // start the transition return rootNav._insertViews(-1, [enteringView], opts); } @@ -740,7 +744,7 @@ export class NavController extends Ion { // get the view thats ready to enter let enteringView = this.getByState(STATE_INIT_ENTER); - if (!enteringView && this._portal) { + if (!enteringView && !this.isPortal) { // oh nos! no entering view to go to! // if there is no previous view that would enter in this nav stack // and the option is set to climb up the nav parent looking @@ -912,9 +916,7 @@ export class NavController extends Ion { opts = {}; } - if (this.config.get('animate') === false || (this._views.length === 1 && !this._init)) { - opts.animate = false; - } + this._setAnimate(opts); if (!leavingView) { // if no leaving view then create a bogus one @@ -943,6 +945,12 @@ export class NavController extends Ion { }); } + private _setAnimate(opts: NavOptions) { + if ((this._views.length === 1 && !this._init && !this.isPortal) || this.config.get('animate') === false) { + opts.animate = false; + } + } + /** * @private */ @@ -1097,9 +1105,7 @@ export class NavController extends Ion { this._trans && this._trans.destroy(); this._trans = transAnimation; - // Portal elements should always animate - // so ignore this if it is a portal - if (opts.animate === false && this._portal) { + if (opts.animate === false) { // force it to not animate the elements, just apply the "to" styles transAnimation.duration(0); } @@ -1253,7 +1259,7 @@ export class NavController extends Ion { this._app && this._app.setEnabled(true); this.setTransitioning(false); - if (direction !== null && hasCompleted && this._portal) { + if (direction !== null && hasCompleted && !this.isPortal) { // notify router of the state change if a direction was provided // multiple routers can exist and each should be notified this.routers.forEach(router => { @@ -1662,7 +1668,7 @@ export class NavController extends Ion { } else { // this is the initial view - enteringView.setZIndex(this._portal ? INIT_ZINDEX : PORTAL_ZINDEX, this._renderer); + enteringView.setZIndex(this.isPortal ? PORTAL_ZINDEX : INIT_ZINDEX, this._renderer); } } else if (direction === 'back') { diff --git a/ionic/components/nav/nav-portal.ts b/ionic/components/nav/nav-portal.ts index 6e6786ab4a..add121766d 100644 --- a/ionic/components/nav/nav-portal.ts +++ b/ionic/components/nav/nav-portal.ts @@ -26,5 +26,6 @@ export class Portal extends NavController { renderer: Renderer ) { super(hostNavCtrl, app, config, keyboard, elementRef, null, compiler, viewManager, zone, renderer); + this.isPortal = true; } } diff --git a/ionic/components/nav/test/nav-controller.spec.ts b/ionic/components/nav/test/nav-controller.spec.ts index d37c198c28..9d0edf5dd3 100644 --- a/ionic/components/nav/test/nav-controller.spec.ts +++ b/ionic/components/nav/test/nav-controller.spec.ts @@ -641,11 +641,11 @@ export function run() { it('should set zIndex 9999 on first entering portal view', () => { let enteringView = new ViewController(); enteringView.setPageRef({}); - nav._portal = null; + nav.isPortal = true; nav._setZIndex(enteringView, null, 'forward'); expect(enteringView.zIndex).toEqual(9999); }); - + it('should set zIndex 10000 on second entering portal view', () => { let leavingView = new ViewController(); leavingView.zIndex = 9999; @@ -655,8 +655,8 @@ export function run() { nav._portal = null; nav._setZIndex(enteringView, leavingView, 'forward'); expect(enteringView.zIndex).toEqual(10000); - }); - + }); + it('should set zIndex 9999 on entering portal view going back', () => { let leavingView = new ViewController(); leavingView.zIndex = 10000; @@ -666,7 +666,52 @@ export function run() { nav._portal = null; nav._setZIndex(enteringView, leavingView, 'back'); expect(enteringView.zIndex).toEqual(9999); - }); + }); + + }); + + describe('_setAnimate', () => { + + it('should be unchanged when the nav is a portal', () => { + nav._views = [new ViewController()]; + nav._init = false; + nav.isPortal = true; + let opts: NavOptions = {}; + nav._setAnimate(opts); + expect(opts.animate).toBeUndefined(); + }); + + it('should not animate when theres only 1 view, and nav hasnt initialized yet', () => { + nav._views = [new ViewController()]; + nav._init = false; + let opts: NavOptions = {}; + nav._setAnimate(opts); + expect(opts.animate).toEqual(false); + }); + + it('should be unchanged when theres only 1 view, and nav has already initialized', () => { + nav._views = [new ViewController()]; + nav._init = true; + let opts: NavOptions = {}; + nav._setAnimate(opts); + expect(opts.animate).toBeUndefined(); + }); + + it('should not animate with config animate = false, and has initialized', () => { + config.set('animate', false); + nav._init = true; + let opts: NavOptions = {}; + nav._setAnimate(opts); + expect(opts.animate).toEqual(false); + }); + + it('should not animate with config animate = false, and has not initialized', () => { + config.set('animate', false); + nav._init = false; + let opts: NavOptions = {}; + nav._setAnimate(opts); + expect(opts.animate).toEqual(false); + }); }); @@ -1134,25 +1179,25 @@ export function run() { let enteringView = new ViewController(); enteringView.setPageRef({}); enteringView.usePortal = true; - + expect(nav._portal.length()).toBe(0); expect(nav.length()).toBe(0); nav.present(enteringView); expect(nav._portal.length()).toBe(1); expect(nav.length()).toBe(0); }); - + it('should present in main nav', () => { let enteringView = new ViewController(); enteringView.setPageRef({}); enteringView.usePortal = false; - + expect(nav._portal.length()).toBe(0); expect(nav.length()).toBe(0); nav.present(enteringView); expect(nav._portal.length()).toBe(0); expect(nav.length()).toBe(1); - }); + }); }); @@ -1294,7 +1339,7 @@ export function run() { setElementClass: function(){}, setElementStyle: function(){} }; - + nav._portal = new NavController(null, null, config, null, elementRef, null, null, null, null, null); return nav;