fix(nav): do not hide pages if an overlay is in the stack

Closes #5430

Author: adamdbradley

ionic/components/action-sheet/test/basic/index.ts

@@ -1,4 +1,4 @@
-import {App, Page, ActionSheet, Modal, NavController, ViewController, Platform} from 'ionic-angular';
+import {App, Page, ActionSheet, Alert, Modal, NavController, ViewController, Platform} from 'ionic-angular';
 
 
 @Page({
@@ -101,6 +101,54 @@ class E2EPage {
     this.nav.present(actionSheet);
   }
 
+  presentActionSheet3(ev) {
+    this.result = '';
+
+    let actionSheet = ActionSheet.create({
+      buttons: [
+        {
+          text: 'Open Alert',
+          handler: () => {
+            this.result = 'Opened alert';
+
+            let alert = Alert.create();
+            alert.setTitle('Alert!');
+            alert.setMessage('Alert opened from an action sheet');
+            alert.addButton({
+              text: 'Cancel',
+              role: 'cancel',
+              handler: () => {
+                this.result = 'pressed Cancel button in alert from action sheet';
+              }
+            });
+            alert.addButton({
+              text: 'Okay',
+              handler: () => {
+                this.result = 'pressed Okay button in alert from action sheet';
+              }
+            });
+
+            this.nav.present(alert).then(() => {
+              this.result = 'Alert from action sheet opened';
+            });
+
+            // do not close the action sheet yet
+            return false;
+          }
+        },
+        {
+          text: 'Cancel',
+          role: 'cancel',
+          handler: () => {
+            this.result = 'Canceled';
+          }
+        }
+      ]
+    });
+
+    this.nav.present(actionSheet);
+  }
+
 }
 
 @Page({

ionic/components/action-sheet/test/basic/main.html

@@ -6,6 +6,7 @@
 <ion-content padding>
   <button block class="e2eOpenActionSheet" (click)="presentActionSheet1()">Present Action Sheet 1</button>
   <button block (click)="presentActionSheet2()">Present Action Sheet 2</button>
+  <button block (click)="presentActionSheet3()">Present Action Sheet 3</button>
 
   <pre>
     Result: {{result}}

ionic/components/nav/nav-controller.ts

@@ -984,8 +984,8 @@ export class NavController extends Ion {
         // the previous transition was still going when this one started
         // so to be safe, only update showing the entering/leaving
         // don't hide the others when they could still be transitioning
-        enteringView.domCache(true, this._renderer);
-        leavingView.domCache(true, this._renderer);
+        enteringView.domShow(true, this._renderer);
+        leavingView.domShow(true, this._renderer);
 
       } else {
         // there are no other transitions happening but this one
@@ -1001,7 +1001,7 @@ export class NavController extends Ion {
                        (view === leavingView) ||
                        view.isOverlay ||
                        (i < ii - 1 ? this._views[i + 1].isOverlay : false);
-          view.domCache(shouldShow, this._renderer);
+          view.domShow(shouldShow, this._renderer);
         }
       }
 
@@ -1182,11 +1182,20 @@ export class NavController extends Ion {
 
         // make sure only this entering view and PREVIOUS view are the
         // only two views that are not display:none
+        // do not make any changes to the stack's current visibility
+        // if there is an overlay somewhere in the stack
         leavingView = this.getPrevious(enteringView);
-        this._views.forEach(view => {
-          let shouldShow = (view === enteringView) || (view === leavingView);
-          view.domCache(shouldShow, this._renderer);
-        });
+        if (this.hasOverlay()) {
+          // ensure the entering view is showing
+          enteringView.domShow(true, this._renderer);
+
+        } else {
+          // only possibly hide a view if there are no overlays in the stack
+          this._views.forEach(view => {
+            let shouldShow = (view === enteringView) || (view === leavingView);
+            view.domShow(shouldShow, this._renderer);
+          });
+        }
 
         // this check only needs to happen once, which will add the css
         // class to the nav when it's finished its first transition
@@ -1476,6 +1485,19 @@ export class NavController extends Ion {
     this._trnsTime = (isTransitioning ? Date.now() + fallback : 0);
   }
 
+  /**
+   * @private
+   * @returns {boolean}
+   */
+  hasOverlay(): boolean {
+    for (var i = this._views.length - 1; i >= 0; i--) {
+      if (this._views[i].isOverlay) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * @private
    * @returns {ViewController}

ionic/components/nav/test/nav-controller.spec.ts

@@ -477,7 +477,7 @@ export function run() {
         expect(navOptions.animate).toBe(false);
       });
 
-      it('should set domCache true when isAlreadyTransitioning', () => {
+      it('should set domShow true when isAlreadyTransitioning', () => {
         let enteringView = new ViewController(Page1);
         let leavingView = new ViewController(Page2);
         let isAlreadyTransitioning = true;
@@ -486,16 +486,16 @@ export function run() {
         nav._beforeTrans = () => {}; //prevent running beforeTrans for tests
         nav._renderer = null;
 
-        spyOn(enteringView, 'domCache');
-        spyOn(leavingView, 'domCache');
+        spyOn(enteringView, 'domShow');
+        spyOn(leavingView, 'domShow');
 
         nav._postRender(1, enteringView, leavingView, isAlreadyTransitioning, navOptions, done);
 
-        expect(enteringView.domCache).toHaveBeenCalledWith(true, nav._renderer);
-        expect(leavingView.domCache).toHaveBeenCalledWith(true, nav._renderer);
+        expect(enteringView.domShow).toHaveBeenCalledWith(true, nav._renderer);
+        expect(leavingView.domShow).toHaveBeenCalledWith(true, nav._renderer);
       });
 
-      it('should set domCache true when isAlreadyTransitioning false for the entering/leaving views', () => {
+      it('should set domShow true when isAlreadyTransitioning false for the entering/leaving views', () => {
         let view1 = new ViewController(Page1);
         let view2 = new ViewController(Page2);
         let view3 = new ViewController(Page3);
@@ -506,18 +506,18 @@ export function run() {
         nav._renderer = null;
         nav._views = [view1, view2, view3];
 
-        spyOn(view1, 'domCache');
-        spyOn(view2, 'domCache');
-        spyOn(view3, 'domCache');
+        spyOn(view1, 'domShow');
+        spyOn(view2, 'domShow');
+        spyOn(view3, 'domShow');
 
         nav._postRender(1, view3, view2, isAlreadyTransitioning, navOptions, done);
 
-        expect(view1.domCache).toHaveBeenCalledWith(false, nav._renderer);
-        expect(view2.domCache).toHaveBeenCalledWith(true, nav._renderer);
-        expect(view3.domCache).toHaveBeenCalledWith(true, nav._renderer);
+        expect(view1.domShow).toHaveBeenCalledWith(false, nav._renderer);
+        expect(view2.domShow).toHaveBeenCalledWith(true, nav._renderer);
+        expect(view3.domShow).toHaveBeenCalledWith(true, nav._renderer);
       });
 
-      it('should set domCache true when isAlreadyTransitioning false for views when a view has isOverlay=true', () => {
+      it('should set domShow true when isAlreadyTransitioning false for views when a view has isOverlay=true', () => {
         let view1 = new ViewController(Page1);
         let view2 = new ViewController(Page2);
         let view3 = new ViewController(Page3);
@@ -531,17 +531,17 @@ export function run() {
 
         view3.isOverlay = true;
 
-        spyOn(view1, 'domCache');
-        spyOn(view2, 'domCache');
-        spyOn(view3, 'domCache');
-        spyOn(view4, 'domCache');
+        spyOn(view1, 'domShow');
+        spyOn(view2, 'domShow');
+        spyOn(view3, 'domShow');
+        spyOn(view4, 'domShow');
 
         nav._postRender(1, view4, view3, isAlreadyTransitioning, navOptions, done);
 
-        expect(view1.domCache).toHaveBeenCalledWith(false, nav._renderer);
-        expect(view2.domCache).toHaveBeenCalledWith(true, nav._renderer);
-        expect(view3.domCache).toHaveBeenCalledWith(true, nav._renderer);
-        expect(view4.domCache).toHaveBeenCalledWith(true, nav._renderer);
+        expect(view1.domShow).toHaveBeenCalledWith(false, nav._renderer);
+        expect(view2.domShow).toHaveBeenCalledWith(true, nav._renderer);
+        expect(view3.domShow).toHaveBeenCalledWith(true, nav._renderer);
+        expect(view4.domShow).toHaveBeenCalledWith(true, nav._renderer);
       });
 
     });
@@ -814,6 +814,31 @@ export function run() {
         expect(nav.setTransitioning).not.toHaveBeenCalled();
       });
 
+      it('should set not run domShow when when any view in the stack has isOverlay=true', () => {
+        let view1 = new ViewController(Page1);
+        let view2 = new ViewController(Page2);
+        let view3 = new ViewController(Page3);
+        let view4 = new ViewController(Page4);
+        let hasCompleted = true;
+        nav._views = [view1, view2, view3, view4];
+
+        view1.isOverlay = true;
+
+        nav._transIds = 1;
+
+        spyOn(view1, 'domShow');
+        spyOn(view2, 'domShow');
+        spyOn(view3, 'domShow');
+        spyOn(view4, 'domShow');
+
+        nav._transFinish(1, view4, view3, 'forward', hasCompleted);
+
+        expect(view1.domShow).not.toHaveBeenCalled();
+        expect(view2.domShow).not.toHaveBeenCalled();
+        expect(view3.domShow).not.toHaveBeenCalled();
+        expect(view4.domShow).toHaveBeenCalled();
+      });
+
     });
 
     describe('_insert', () => {

ionic/components/nav/view-controller.ts

@@ -198,7 +198,7 @@ export class ViewController {
   /**
    * @private
    */
-  domCache(shouldShow: boolean, renderer: Renderer) {
+  domShow(shouldShow: boolean, renderer: Renderer) {
     // using hidden element attribute to display:none and not render views
     // renderAttr of '' means the hidden attribute will be added
     // renderAttr of null means the hidden attribute will be removed