Feature/child nav fixes (#6907)

* fix(navigation): fixes bug where navigating to page with child-nav immediately re-enables app

fixes bug where navigating to page with child-nav immediately re-enables app

closes #6752

* test(navigation): added some additional tests

added some additional tests

* test(tabs): added multiple nested children test

added multiple nested children test
This commit is contained in:
Dan Bucholtz
2016-06-14 15:52:24 -05:00
committed by GitHub
parent c65a4fb3e9
commit a655cd78ac
6 changed files with 732 additions and 90 deletions

View File

@ -169,7 +169,6 @@ export class NavController extends Ion {
protected _sbEnabled: boolean;
protected _ids: number = -1;
protected _trnsDelay: any;
protected _trnsTime: number = 0;
protected _views: ViewController[] = [];
viewDidLoad: EventEmitter<any>;
@ -205,6 +204,11 @@ export class NavController extends Ion {
*/
isPortal: boolean = false;
/**
* @private
*/
_trnsTime: number = 0;
constructor(
parent: any,
protected _app: App,
@ -1167,9 +1171,7 @@ export class NavController extends Ion {
ev: opts.ev,
};
let transAnimation = Transition.createTransition(enteringView,
leavingView,
transitionOpts);
let transAnimation = this.createTransitionWrapper(enteringView, leavingView, transitionOpts);
this._trans && this._trans.destroy();
this._trans = transAnimation;
@ -1179,17 +1181,25 @@ export class NavController extends Ion {
transAnimation.duration(0);
}
let keyboardDurationPadding = 0;
if ( this._keyboard.isOpen() ) {
// add XXms to the duration the app is disabled when the keyboard is open
keyboardDurationPadding = 600;
// check if a parent is transitioning and get the time that it ends
let parentTransitionEndTime = this._getLongestTrans(Date.now());
if (parentTransitionEndTime > 0) {
// the parent is already transitioning and has disabled the app
// so just update the local transitioning information
let duration = parentTransitionEndTime - Date.now();
this.setTransitioning(true, duration);
} else {
// this is the only active transition (for now), so disable the app
let keyboardDurationPadding = 0;
if (this._keyboard.isOpen()) {
// add XXms to the duration the app is disabled when the keyboard is open
keyboardDurationPadding = 600;
}
let duration = transAnimation.getDuration() + keyboardDurationPadding;
let enableApp = (duration < 64);
this._app.setEnabled(enableApp, duration);
this.setTransitioning(!enableApp, duration);
}
let duration = transAnimation.getDuration() + keyboardDurationPadding;
let enableApp = (duration < 64);
// block any clicks during the transition and provide a
// fallback to remove the clickblock if something goes wrong
this._app.setEnabled(enableApp, duration);
this.setTransitioning(!enableApp, duration);
if (enteringView.viewType) {
transAnimation.before.addClass(enteringView.viewType);
@ -1333,8 +1343,14 @@ export class NavController extends Ion {
enteringView.state = STATE_INACTIVE;
}
// allow clicks and enable the app again
this._app && this._app.setEnabled(true);
// check if there is a parent actively transitioning
let transitionEndTime = this._getLongestTrans(Date.now());
// if transitionEndTime is greater than 0, there is a parent transition occurring
// so delegate enabling the app to the parent. If it <= 0, go ahead and enable the app
if (transitionEndTime <= 0) {
this._app && this._app.setEnabled(true);
}
this.setTransitioning(false);
if (direction !== null && hasCompleted && !this.isPortal) {
@ -1372,6 +1388,16 @@ export class NavController extends Ion {
}
}
/**
*@private
* This method is just a wrapper to the Transition function of same name
* to make it easy/possible to mock the method call by overriding the function.
* In testing we don't want to actually do the animation, we want to return a stub instead
*/
private createTransitionWrapper(enteringView: ViewController, leavingView: ViewController, transitionOpts: any) {
return Transition.createTransition(enteringView, leavingView, transitionOpts);
}
private _cleanup() {
// ok, cleanup time!! Destroy all of the views that are
// INACTIVE and come after the active view
@ -1648,6 +1674,26 @@ export class NavController extends Ion {
this._trnsTime = (isTransitioning ? Date.now() + fallback : 0);
}
/**
* @private
* This method traverses the tree of parents upwards
* and looks at the time the transition ends (if it's transitioning)
* and returns the value that is the furthest into the future
* thus giving us the longest transition duration
*/
private _getLongestTrans(now: number) {
let parentNav: NavController = <NavController> this.parent;
let transitionEndTime: number = -1;
while (parentNav) {
if (parentNav._trnsTime > transitionEndTime) {
transitionEndTime = parentNav._trnsTime;
}
parentNav = <NavController> parentNav.parent;
}
// only check if the transitionTime is greater than the current time once
return transitionEndTime > 0 && transitionEndTime > now ? transitionEndTime : 0;
}
/**
* @private
*/

View File

@ -0,0 +1,3 @@
it('should go to page and open child navs', function() {
element(by.css('.nested-children-test')).click();
});

View File

@ -0,0 +1,113 @@
import {Component} from '@angular/core';
import {ionicBootstrap, NavController} from '../../../../../src';
@Component({
template: `<ion-nav [root]="root"></ion-nav>`,
})
class E2EApp {
root = LandingPage;
}
ionicBootstrap(E2EApp);
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Landing Page Comp
</ion-title>
</ion-navbar>
<ion-content class="home">
<button primary (click)="goToPage()" class="nested-children-test">
Nested Children Test
</button>
</ion-content>
`
})
class LandingPage{
constructor(private _navController: NavController){
}
goToPage(){
this._navController.push(FirstPage);
}
}
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
First Page Comp
</ion-title>
</ion-navbar>
<ion-content class="home">
<h3>Sub Header First Page</h3>
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
class FirstPage{
root = SecondPage;
}
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Second Page Comp
</ion-title>
</ion-navbar>
<ion-content class="home">
<h3>Sub Header Second Page</h3>
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
class SecondPage{
root = ThirdPage;
}
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Third Page Comp
</ion-title>
</ion-navbar>
<ion-content class="home">
<h3>Sub Header Third Page</h3>
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
class ThirdPage{
root = FourthPage;
}
@Component({
template: `
<ion-content>
<ion-list>
<ion-item *ngFor="let item of items">
{{item}}
</ion-item>
</ion-list>
</ion-content>
`
})
class FourthPage{
private items: string[];
ionViewWillEnter(){
let items: string[] = [];
for ( let i = 0 ; i < 500; i++ ){
items.push(`Item ${(i + 1)}`);
}
this.items = items;
}
}

View File

@ -978,6 +978,52 @@ export function run() {
expect(view4.domShow).toHaveBeenCalled();
});
it('should re-enable the app when transition time <= 0', () => {
// arrange
let enteringView = new ViewController(Page1);
enteringView.state = 'somethingelse';
let leavingView = new ViewController(Page2);
leavingView.state = 'somethingelse';
nav._transIds = 1;
nav._app = {
setEnabled: () => {}
};
spyOn(nav._app, 'setEnabled');
spyOn(nav, 'setTransitioning');
// act
nav._transFinish(nav._transIds, enteringView, leavingView, 'forward', true);
// assert
expect(nav._app.setEnabled).toHaveBeenCalledWith(true);
expect(nav.setTransitioning).toHaveBeenCalledWith(false);
});
it('should not re-enable app when transition time > 0', () => {
// arrange
let enteringView = new ViewController(Page1);
enteringView.state = 'somethingelse';
let leavingView = new ViewController(Page2);
leavingView.state = 'somethingelse';
nav._transIds = 1;
nav._app = {
setEnabled: () => {}
};
spyOn(nav._app, 'setEnabled');
spyOn(nav, 'setTransitioning');
nav._getLongestTrans = () => { return 50 };
// act
nav._transFinish(nav._transIds, enteringView, leavingView, 'forward', true);
// assert
expect(nav._app.setEnabled).not.toHaveBeenCalled();
expect(nav.setTransitioning).toHaveBeenCalledWith(false);
});
});
describe('_insert', () => {
@ -1198,108 +1244,402 @@ export function run() {
expect(nav._portal.length()).toBe(0);
expect(nav.length()).toBe(1);
});
});
it('should getActive()', () => {
expect(nav.getActive()).toBe(null);
let view1 = new ViewController(Page1);
view1.state = STATE_INIT_ENTER;
nav.views = [view1];
expect(nav.getActive()).toBe(null);
view1.state = STATE_ACTIVE;
expect(nav.getActive()).toBe(view1);
describe('getActive', () => {
it('should getActive()', () => {
expect(nav.getActive()).toBe(null);
let view1 = new ViewController(Page1);
view1.state = STATE_INIT_ENTER;
nav.views = [view1];
expect(nav.getActive()).toBe(null);
view1.state = STATE_ACTIVE;
expect(nav.getActive()).toBe(view1);
});
});
it('should getByState()', () => {
expect(nav.getByState(null)).toBe(null);
describe('getByState', () => {
it('should getByState()', () => {
expect(nav.getByState(null)).toBe(null);
let view1 = new ViewController(Page1);
view1.state = STATE_INIT_ENTER;
let view2 = new ViewController(Page2);
view2.state = STATE_INIT_ENTER;
nav.views = [view1, view2];
let view1 = new ViewController(Page1);
view1.state = STATE_INIT_ENTER;
let view2 = new ViewController(Page2);
view2.state = STATE_INIT_ENTER;
nav.views = [view1, view2];
expect(nav.getByState('whatever')).toBe(null);
expect(nav.getByState(STATE_INIT_ENTER)).toBe(view2);
expect(nav.getByState('whatever')).toBe(null);
expect(nav.getByState(STATE_INIT_ENTER)).toBe(view2);
view2.state = STATE_INACTIVE;
expect(nav.getByState(STATE_INIT_ENTER)).toBe(view1);
view2.state = STATE_INACTIVE;
expect(nav.getByState(STATE_INIT_ENTER)).toBe(view1);
view2.state = STATE_ACTIVE;
expect(nav.getActive()).toBe(view2);
view2.state = STATE_ACTIVE;
expect(nav.getActive()).toBe(view2);
});
});
it('should getPrevious()', () => {
expect(nav.getPrevious(null)).toBe(null);
describe('getPrevious', () => {
it('should getPrevious()', () => {
expect(nav.getPrevious(null)).toBe(null);
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
nav.views = [view1, view2];
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
nav.views = [view1, view2];
expect(nav.getPrevious(view1)).toBe(null);
expect(nav.getPrevious(view2)).toBe(view1);
expect(nav.getPrevious(view1)).toBe(null);
expect(nav.getPrevious(view2)).toBe(view1);
});
});
it('should get first()', () => {
expect(nav.first()).toBe(null);
let view1 = new ViewController(Page1);
view1.setNav(nav);
let view2 = new ViewController(Page2);
view2.setNav(nav);
nav.views = [view1];
describe('first', () => {
it('should get first()', () => {
expect(nav.first()).toBe(null);
let view1 = new ViewController(Page1);
view1.setNav(nav);
let view2 = new ViewController(Page2);
view2.setNav(nav);
nav.views = [view1];
expect(nav.first()).toBe(view1);
expect(view1.isFirst()).toBe(true);
expect(nav.first()).toBe(view1);
expect(view1.isFirst()).toBe(true);
nav.views = [view1, view2];
expect(nav.first()).toBe(view1);
expect(view1.isFirst()).toBe(true);
expect(view2.isFirst()).toBe(false);
nav.views = [view1, view2];
expect(nav.first()).toBe(view1);
expect(view1.isFirst()).toBe(true);
expect(view2.isFirst()).toBe(false);
});
});
it('should get last()', () => {
expect(nav.last()).toBe(null);
let view1 = new ViewController(Page1);
view1.setNav(nav);
let view2 = new ViewController(Page2);
view2.setNav(nav);
nav.views = [view1];
describe('last', () => {
it('should get last()', () => {
expect(nav.last()).toBe(null);
let view1 = new ViewController(Page1);
view1.setNav(nav);
let view2 = new ViewController(Page2);
view2.setNav(nav);
nav.views = [view1];
expect(nav.last()).toBe(view1);
expect(view1.isLast()).toBe(true);
expect(nav.last()).toBe(view1);
expect(view1.isLast()).toBe(true);
nav.views = [view1, view2];
expect(nav.last()).toBe(view2);
expect(view1.isLast()).toBe(false);
expect(view2.isLast()).toBe(true);
nav.views = [view1, view2];
expect(nav.last()).toBe(view2);
expect(view1.isLast()).toBe(false);
expect(view2.isLast()).toBe(true);
});
});
it('should get indexOf()', () => {
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
describe('indexOf', () => {
it('should get indexOf()', () => {
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
expect(nav.length()).toBe(0);
expect(nav.indexOf(view1)).toBe(-1);
expect(nav.length()).toBe(0);
expect(nav.indexOf(view1)).toBe(-1);
nav.views = [view1, view2];
expect(nav.indexOf(view1)).toBe(0);
expect(nav.indexOf(view2)).toBe(1);
expect(nav.length()).toBe(2);
nav.views = [view1, view2];
expect(nav.indexOf(view1)).toBe(0);
expect(nav.indexOf(view2)).toBe(1);
expect(nav.length()).toBe(2);
});
});
it('should get getByIndex()', () => {
expect(nav.getByIndex(-99)).toBe(null);
expect(nav.getByIndex(99)).toBe(null);
describe('getByIndex', () => {
it('should get getByIndex()', () => {
expect(nav.getByIndex(-99)).toBe(null);
expect(nav.getByIndex(99)).toBe(null);
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
nav.views = [view1, view2];
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
nav.views = [view1, view2];
expect(nav.getByIndex(-1)).toBe(null);
expect(nav.getByIndex(0)).toBe(view1);
expect(nav.getByIndex(1)).toBe(view2);
expect(nav.getByIndex(2)).toBe(null);
expect(nav.getByIndex(-1)).toBe(null);
expect(nav.getByIndex(0)).toBe(view1);
expect(nav.getByIndex(1)).toBe(view2);
expect(nav.getByIndex(2)).toBe(null);
});
});
/* private method */
describe('_beforeTrans', () => {
it('shouldnt disable app on short transition', () => {
// arrange
let executeAssertions = () => {
// assertions triggerd by callbacks
expect(app.setEnabled).toHaveBeenCalledWith(true, 50);
expect(nav.setTransitioning).toHaveBeenCalledWith(false, 50);
};
let mockTransition = {
play: () => {
executeAssertions();
},
getDuration: () => { return 50},
onFinish: () => {}
};
nav.createTransitionWrapper = () => {
return mockTransition;
};
nav.config = {
platform : {
isRTL: () => {}
}
};
let app = {
setEnabled: () => {}
};
nav._app = app;
spyOn(app, 'setEnabled');
spyOn(nav, 'setTransitioning');
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
// act
nav._beforeTrans(view1, view2, {}, () => {});
});
it('should disable app on longer transition', () => {
// arrange
let executeAssertions = () => {
// assertions triggerd by callbacks
expect(app.setEnabled).toHaveBeenCalledWith(false, 200);
expect(nav.setTransitioning).toHaveBeenCalledWith(true, 200);
};
let mockTransition = {
play: () => {
executeAssertions();
},
getDuration: () => { return 200},
onFinish: () => {}
};
nav.createTransitionWrapper = () => {
return mockTransition;
};
nav.config = {
platform : {
isRTL: () => {}
}
};
let app = {
setEnabled: () => {}
};
nav._app = app;
spyOn(app, 'setEnabled');
spyOn(nav, 'setTransitioning');
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
// act
nav._beforeTrans(view1, view2, {}, () => {});
});
it('should disable app w/ padding when keyboard is open', () => {
// arrange
let executeAssertions = () => {
// assertions triggerd by callbacks
expect(app.setEnabled.calls.mostRecent().args[0]).toEqual(false);
expect(app.setEnabled.calls.mostRecent().args[1]).toBeGreaterThan(200);
expect(nav.setTransitioning.calls.mostRecent().args[0]).toEqual(true);
expect(nav.setTransitioning.calls.mostRecent().args[1]).toBeGreaterThan(200);
};
let mockTransition = {
play: () => {
executeAssertions();
},
getDuration: () => { return 200},
onFinish: () => {}
};
nav.createTransitionWrapper = () => {
return mockTransition;
};
nav.config = {
platform : {
isRTL: () => {}
}
};
let app = {
setEnabled: () => {}
};
nav._app = app;
nav._keyboard = {
isOpen: () => true
};
spyOn(app, 'setEnabled');
spyOn(nav, 'setTransitioning');
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
// act
nav._beforeTrans(view1, view2, {}, () => {});
});
it('shouldnt update app enabled when parent transition is occurring', () => {
// arrange
let executeAssertions = () => {
// assertions triggerd by callbacks
expect(app.setEnabled).not.toHaveBeenCalled();
expect(nav.setTransitioning.calls.mostRecent().args[0]).toEqual(true);
};
let mockTransition = {
play: () => {
executeAssertions();
},
getDuration: () => { return 200},
onFinish: () => {}
};
nav.createTransitionWrapper = () => {
return mockTransition;
};
nav.config = {
platform : {
isRTL: () => {}
}
};
let app = {
setEnabled: () => {}
};
nav._app = app;
spyOn(app, 'setEnabled');
spyOn(nav, 'setTransitioning');
nav._getLongestTrans = () => { return Date.now() + 100 };
let view1 = new ViewController(Page1);
let view2 = new ViewController(Page2);
// act
nav._beforeTrans(view1, view2, {}, () => {});
});
});
/* private method */
describe('_getLongestTrans', () => {
it('should return 0 when transition end time is less than 0', () => {
// arrange
nav.parent = null;
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(0);
});
it('should return 0 when transition end time is less than now', () => {
// arrange
nav.parent = {
_trnsTime: Date.now() - 5
};
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(0);
});
it('should return 0 when parent transition time not set', () => {
// arrange
nav.parent = {
_trnsTime: undefined
};
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(0);
});
it('should return transitionEndTime when transition end time is greater than now', () => {
// arrange
let expectedReturnValue = Date.now() + 100;
nav.parent = {
_trnsTime: expectedReturnValue
};
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(expectedReturnValue);
});
it('should return the greatest end of transition time if found on first parent', () => {
// arrange
let expectedReturnValue = Date.now() + 100;
let firstParent = {
_trnsTime: expectedReturnValue
};
let secondParent = {
_trnsTime: Date.now() + 50
};
let thirdParent = {
_trnsTime: Date.now()
};
let fourthParent = {
_trnsTime: Date.now() + 20
};
firstParent.parent = secondParent;
secondParent.parent = thirdParent;
thirdParent.parent = fourthParent;
nav.parent = firstParent;
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(expectedReturnValue);
});
it('should return the greatest end of transition time if found on middle parent', () => {
// arrange
let expectedReturnValue = Date.now() + 100;
let firstParent = {
_trnsTime: Date.now()
};
let secondParent = {
_trnsTime: Date.now() + 50
};
let thirdParent = {
_trnsTime: expectedReturnValue
};
let fourthParent = {
_trnsTime: Date.now() + 20
};
firstParent.parent = secondParent;
secondParent.parent = thirdParent;
thirdParent.parent = fourthParent;
nav.parent = firstParent;
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(expectedReturnValue);
});
it('should return the greatest end of transition time if found on last parent', () => {
// arrange
let expectedReturnValue = Date.now() + 100;
let firstParent = {
_trnsTime: Date.now()
};
let secondParent = {
_trnsTime: Date.now() + 50
};
let thirdParent = {
_trnsTime: Date.now() + 20
};
let fourthParent = {
_trnsTime: expectedReturnValue
};
firstParent.parent = secondParent;
secondParent.parent = thirdParent;
thirdParent.parent = fourthParent;
nav.parent = firstParent;
// act
let returnedValue = nav._getLongestTrans(Date.now());
// asssert
expect(returnedValue).toEqual(expectedReturnValue);
});
});
// setup stuff

View File

@ -0,0 +1,140 @@
import {Component} from '@angular/core';
import {ionicBootstrap, NavController, App, Alert, Modal, ViewController, Tab, Tabs} from '../../../../../src';
//
// Tab 1
//
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Tab 1
</ion-title>
</ion-navbar>
<ion-content class="home">
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
export class Tab1 {
root = SecondPage;
}
//
// Tab 2
//
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Tab 2
</ion-title>
</ion-navbar>
<ion-content class="home">
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
export class Tab2 {
root = SecondPage;
}
//
// Tab 3
//
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Tab 3
</ion-title>
</ion-navbar>
<ion-content class="home">
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
export class Tab3 {
root = SecondPage;
}
@Component({
template: `
<ion-content class="home">
<div>SecondPage Cmp</div>
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
class SecondPage{
root = ThirdPage;
}
@Component({
template: `
<ion-content class="home">
<div>ThirdPage Cmp</div>
<ion-nav [root]="root"></ion-nav>
</ion-content>
`
})
class ThirdPage{
root = FourthPage;
}
@Component({
template: `
<ion-navbar *navbar>
<ion-title>
Fourth Page Comp
</ion-title>
</ion-navbar>
<ion-content>
<ion-list>
<ion-item *ngFor="let item of items">
{{item}}
</ion-item>
</ion-list>
</ion-content>
`
})
class FourthPage{
private items: string[];
ionViewWillEnter(){
let items: string[] = [];
for ( let i = 0 ; i < 500; i++ ){
items.push(`Item ${(i + 1)}`);
}
this.items = items;
}
}
@Component({
template: `
<ion-tabs #content>
<ion-tab tabTitle="Tab 1" tabIcon="star" [root]="root1"></ion-tab>
<ion-tab tabTitle="Tab 2" tabIcon="globe" [root]="root2"></ion-tab>
<ion-tab tabTitle="Tab 3" tabIcon="stopwatch" [root]="root3"></ion-tab>
</ion-tabs>
`
})
export class TabsPage {
root1 = Tab1;
root2 = Tab2;
root3 = Tab3;
}
@Component({
template: `<ion-nav [root]="root"></ion-nav>`
})
export class E2EApp {
root = TabsPage;
}
ionicBootstrap(E2EApp);