fix(angular): fix navigation stack when using browser back and forward button

fix(angular): respect direction set via the NavController

fix(angular): adapt tests

fix(angular): fixes

Author: philipp-stsm

packages/angular/src/directives/navigation/stack-controller.ts

@@ -1,7 +1,7 @@
 import { Location } from '@angular/common';
 import { ComponentRef, NgZone } from '@angular/core';
 import { ActivatedRoute, Router } from '@angular/router';
-import { AnimationBuilder, RouterDirection } from '@ionic/core';
+import { AnimationBuilder, NavDirection, RouterDirection } from '@ionic/core';
 
 import { bindLifecycleEvents } from '../../providers/angular-delegate';
 import { NavController } from '../../providers/nav-controller';
@@ -62,14 +62,8 @@ export class StackController {
   }
 
   setActive(enteringView: RouteView): Promise<StackDidChangeEvent> {
-    const consumeResult = this.navCtrl.consumeTransition();
+    const { isDirectionBasedOnNavigationIds, ...consumeResult } = this.navCtrl.consumeTransition();
     let { direction, animation, animationBuilder } = consumeResult;
-    const leavingView = this.activeView;
-    const tabSwitch = isTabSwitch(enteringView, leavingView);
-    if (tabSwitch) {
-      direction = 'back';
-      animation = undefined;
-    }
 
     const viewsSnapshot = this.views.slice();
 
@@ -87,25 +81,65 @@ export class StackController {
     }
 
     /**
-     * If the navigation action
-     * sets `replaceUrl: true`
-     * then we need to make sure
-     * we remove the last item
-     * from our views stack
+     * If the navigation action sets `replaceUrl: true` then we need to make sure
+     * we remove the last item from our views stack
      */
-    if (currentNavigation?.extras?.replaceUrl) {
+    if (currentNavigation?.extras?.replaceUrl && currentNavigation?.trigger !== 'popstate') {
       if (this.views.length > 0) {
         this.views.splice(-1, 1);
       }
     }
 
-    const reused = this.views.includes(enteringView);
+    // determine direction based on the order of the views in the stack
+    const leavingView = this.activeView;
+    const isEnteringViewReused = this.views.includes(enteringView);
+    const leavingViewIndex = leavingView ? this.views.indexOf(leavingView) : -1;
+    const enteringViewIndex = isEnteringViewReused ? this.views.indexOf(enteringView) : this.views.length;
+    const suggestedDirectionBasedOnStackOrder: NavDirection | undefined =
+      leavingViewIndex === -1 ? undefined : enteringViewIndex < leavingViewIndex ? 'back' : 'forward';
+
+    /**
+     * The user triggered a back navigation on a page that was navigated to with root. In this case, the new page
+     * becomes the root and the leavingView is removed.
+     *
+     * This can happen when e.g.:
+     * - using the NavController's navigateBack on a root page
+     * - navigating to a page with navigateRoot and then using the browser back button
+     */
+    if (
+      direction === 'back' &&
+      isDirectionBasedOnNavigationIds &&
+      leavingView?.root &&
+      currentNavigation?.trigger === 'popstate'
+    ) {
+      if (leavingViewIndex >= 0) {
+        this.views.splice(leavingViewIndex, 1);
+      }
+    }
+
+    /**
+     * direction based on stack order takes precedence over direction based on navigation ids
+     *
+     * only applied if the user did not explicitly set the direction
+     * (e.g. via the NavController, routerLink directive etc.)
+     */
+    if (isDirectionBasedOnNavigationIds && suggestedDirectionBasedOnStackOrder) {
+      direction = suggestedDirectionBasedOnStackOrder;
+      animation = suggestedDirectionBasedOnStackOrder;
+    }
+
+    const tabSwitch = isTabSwitch(enteringView, leavingView);
+    if (tabSwitch) {
+      direction = 'back';
+      animation = undefined;
+    }
+
     const views = this.insertView(enteringView, direction);
 
     // Trigger change detection before transition starts
     // This will call ngOnInit() the first time too, just after the view
     // was attached to the dom, but BEFORE the transition starts
-    if (!reused) {
+    if (!isEnteringViewReused) {
       enteringView.ref.changeDetectorRef.detectChanges();
     }
 

packages/angular/src/directives/navigation/stack-utils.ts

@@ -14,6 +14,7 @@ export const insertView = (views: RouteView[], view: RouteView, direction: Route
 
 const setRoot = (views: RouteView[], view: RouteView) => {
   views = views.filter((v) => v.stackId !== view.stackId);
+  view.root = true;
   views.push(view);
   return views;
 };
@@ -110,4 +111,5 @@ export interface RouteView {
   savedExtras?: NavigationExtras;
   unlistenEvents: () => void;
   animationBuilder?: AnimationBuilder;
+  root?: boolean;
 }

packages/angular/src/providers/nav-controller.ts

@@ -37,10 +37,11 @@ export class NavController {
     if (router) {
       router.events.subscribe((ev) => {
         if (ev instanceof NavigationStart) {
+          // restoredState is set if the browser back/forward button is used
           const id = ev.restoredState ? ev.restoredState.navigationId : ev.id;
           this.guessDirection = id < this.lastNavId ? 'back' : 'forward';
-          this.guessAnimation = !ev.restoredState ? this.guessDirection : undefined;
-          this.lastNavId = this.guessDirection === 'forward' ? ev.id : id;
+          this.guessAnimation = this.guessDirection;
+          this.lastNavId = id;
         }
       });
     }
@@ -180,11 +181,14 @@ export class NavController {
     direction: RouterDirection;
     animation: NavDirection | undefined;
     animationBuilder: AnimationBuilder | undefined;
+    isDirectionBasedOnNavigationIds: boolean;
   } {
     let direction: RouterDirection = 'root';
     let animation: NavDirection | undefined;
     const animationBuilder = this.animationBuilder;
 
+    const isDirectionBasedOnNavigationIds = this.direction === 'auto';
+
     if (this.direction === 'auto') {
       direction = this.guessDirection;
       animation = this.guessAnimation;
@@ -200,6 +204,7 @@ export class NavController {
       direction,
       animation,
       animationBuilder,
+      isDirectionBasedOnNavigationIds,
     };
   }
 

packages/angular/test/base/e2e/src/router-link.spec.ts

@@ -113,6 +113,7 @@ describe('Router Link', () => {
   describe('back', () => {
     it('should go back with ion-button[routerLink][routerDirection=back]', () => {
       cy.get('#routerLink-back').click();
+      testBack();
     });
 
     it('should go back with a[routerLink][routerDirection=back]', () => {
@@ -138,6 +139,7 @@ function testForward() {
   cy.get('app-router-link-page #canGoBack').should('have.text', 'true');
 
   cy.go('back');
+  cy.wait(500);
   cy.testStack('ion-router-outlet', ['app-router-link']);
   cy.testLifeCycle('app-router-link', {
     ionViewWillEnter: 2,
@@ -159,7 +161,7 @@ function testRoot() {
   cy.get('app-router-link-page #canGoBack').should('have.text', 'false');
 
   cy.go('back');
-  cy.wait(100);
+  cy.wait(500);
   cy.testStack('ion-router-outlet', ['app-router-link']);
   cy.testLifeCycle('app-router-link', {
     ionViewWillEnter: 1,
@@ -181,7 +183,7 @@ function testBack() {
   cy.get('app-router-link-page #canGoBack').should('have.text', 'false');
 
   cy.go('back');
-  cy.wait(100);
+  cy.wait(500);
   cy.testStack('ion-router-outlet', ['app-router-link']);
   cy.testLifeCycle('app-router-link', {
     ionViewWillEnter: 1,