fix(vue): switching between tabs and going back resolves to correct r…

…oute (#25206)

resolves #24303

packages/vue-router/src/locationHistory.ts

@@ -83,37 +83,57 @@ export const createLocationHistory = () => {
    */
   const clearHistory = (routeInfo?: RouteInfo) => {
     if (routeInfo) {
+      const { position, tab } = routeInfo;
 
       /**
        * If there is no route index in locationHistory
        * then there will not be any route index in
        * tabs either.
        */
-      const existingRouteIndex = locationHistory.findIndex(r => r.position === routeInfo.position);
+      const existingRouteIndex = locationHistory.findIndex(r => r.position === position);
       if (existingRouteIndex === -1) return;
 
       locationHistory.splice(existingRouteIndex);
 
+      const clearTabHistory = (tab: string) => {
+        const existingTabRouteIndex = tabsHistory[tab].findIndex(r => r.position === position);
+        if (existingTabRouteIndex === -1) return;
+
+        tabsHistory[tab].splice(existingTabRouteIndex);
+      }
+
       /**
        * We also need to search the current tab
        * to correctly reset the individual tab
        * stack. We should not clear the entire
        * tab stack as that means we will lose
        * a reference to the root tab route.
        */
-      const { tab } = routeInfo;
       const tabHistory = tabsHistory[tab];
       if (tab && tabHistory) {
-        const existingTabRouteIndex = tabHistory.findIndex(r => r.position === routeInfo.position);
-        if (existingTabRouteIndex === -1) return;
-
-        tabsHistory[tab].splice(existingTabRouteIndex);
+        clearTabHistory(tab);
+      /**
+       * If we are not clearing items after
+       * a tabs page, it is still possible
+       * that there are future tabs pages to clear.
+       * As a result, we need to search through
+       * all the tab stacks and remove views that appear
+       * after the given routeInfo.
+       *
+       * Example: /non-tabs-page --> /tabs/tab1 --> /non-tabs-page
+       * (via router.go(-1)) --> /tabs/tab2. The /tabs/tab1 history
+       * has been overwritten with /tabs/tab2. As a result,
+       * the /tabs/tab1 route info in the Tab 1 stack should be removed.
+       */
+      } else {
+        for (const tab in tabsHistory) {
+          clearTabHistory(tab);
+        }
       }
-
     } else {
-      Object.keys(tabsHistory).forEach(key => {
-        tabsHistory[key] = [];
-      });
+      for (const tab in tabsHistory) {
+        tabsHistory[tab] = [];
+      }
 
       locationHistory.length = 0;
     }

packages/vue-router/src/router.ts

@@ -125,7 +125,34 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
         ) {
           router.back();
         } else {
-          router.replace({ path: prevInfo.pathname, query: parseQuery(prevInfo.search) });
+
+          /**
+           * When going back to a child page of a tab
+           * after being on another tab, we need to use
+           * router.go() here instead of pushing or replacing.
+           * Consider the following example:
+           * /tabs/tab1 --> /tabs/tab1/child1 --> /tabs/tab1/child2
+           * --> /tabs/tab2 (via Tab 2 button) --> /tabs/tab1/child2 (via Tab 1 button)
+           *
+           * Pressing the ion-back-button on /tabs/tab1/child2 should take
+           * us back to /tabs/tab1/child1 not /tabs/tab2 because each tab
+           * is its own stack.
+           *
+           * If we called pressed the ion-back-button and this code called
+           * router.replace, then the state of /tabs/tab1/child2 would
+           * be replaced with /tabs/tab1/child1. However, this means that
+           * there would be two /tabs/tab1/child1 entries in the location
+           * history as the original /tabs/tab1/child1 entry is still there.
+           * As a result, clicking the ion-back-button on /tabs/tab1/child1 does
+           * nothing because this code would try to route to the same page
+           * we are currently on.
+           *
+           * If we called router.push instead then we would push a
+           * new /tabs/tab1/child1 entry to the location history. This
+           * is not good because we would have two /tabs/tab1/child1 entries
+           * separated by a /tabs/tab1/child2 entry.
+           */
+          router.go(prevInfo.position - routeInfo.position);
         }
       } else {
         handleNavigate(defaultHref, 'pop', 'back');
@@ -468,11 +495,65 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
    * then IonTabs will not invoke this.
    */
   const handleSetCurrentTab = (tab: string) => {
-    const ri = { ...locationHistory.last() };
+    /**
+     * Note that the current page that we
+     * are on is not necessarily the last item
+     * in the locationHistory stack. As a result,
+     * we cannot use locationHistory.last() here.
+     */
+    const ri = { ...locationHistory.current(initialHistoryPosition, currentHistoryPosition) };
+
+    /**
+     * handleHistoryChange is tabs-agnostic by design.
+     * One side effect of this is that certain tabs
+     * routes have extraneous/incorrect information
+     * that we need to remove. To not tightly couple
+     * handleHistoryChange with tabs, we let the
+     * handleSetCurrentTab function. This function is
+     * only called by IonTabs.
+     */
+
     if (ri.tab !== tab) {
       ri.tab = tab;
       locationHistory.update(ri);
     }
+
+    /**
+     * lastPathname typically equals pushedByRoute
+     * when navigating in a linear manner. When switching between
+     * tabs, this is almost never the case.
+     *
+     * Example: /tabs/tabs1 --> /tabs/tab2 --> /tabs/tab1
+     * The latest Tab 1 route would have the following information
+     * lastPathname: '/tabs/tab2'
+     * pushedByRoute: '/tabs/tab2'
+     *
+     * A tab cannot push another tab, so we need to set
+     * pushedByRoute to `undefined`. Alternative way of thinking
+     * about this: You cannot swipe to go back from Tab 1 to Tab 2.
+     *
+     * However, there are some instances where we do want to keep
+     * the pushedByRoute. As a result, we need to ensure that
+     * we only wipe the pushedByRoute state when the both of the
+     * following conditions are met:
+     * 1. pushedByRoute is different from lastPathname
+     * 2. The tab for the pushedByRoute info is different
+     * from the current route tab.
+     *
+     * Example of when we would not want to clear pushedByRoute:
+     * /tabs/tab1 --> /tabs/tab1/child --> /tabs/tab2 --> /tabs/tab1/child
+     * The latest Tab 1 Child route would have the following information:
+     * lastPathname: '/tabs/tab2'
+     * pushedByRoute: '/tabs/tab1
+     *
+     * In this case, /tabs/tab1/child should be able to swipe to go back
+     * to /tabs/tab1 so we want to keep the pushedByRoute.
+     */
+    const pushedByRoute = locationHistory.findLastLocation(ri);
+    if (ri.pushedByRoute !== ri.lastPathname && pushedByRoute?.tab !== tab) {
+      ri.pushedByRoute = undefined;
+      locationHistory.update(ri);
+    }
   }
 
   // TODO types

packages/vue-router/src/types.ts

@@ -19,11 +19,27 @@ export interface RouteInfo {
   routerAction?: RouteAction;
   routerDirection?: RouteDirection;
   routerAnimation?: AnimationBuilder;
+
+  /**
+   * The previous route you were on if you were to
+   * navigate backwards in a linear manner.
+   * i.e. If you pressed the browser back button,
+   * this is the route you would land on.
+   */
   lastPathname?: string;
   prevRouteLastPathname?: string;
   pathname?: string;
   search?: string;
   params?: { [k: string]: any };
+
+  /**
+   * The route that pushed the current route.
+   * This is used to determine if a route can swipe
+   * to go back to a previous route. This is
+   * usually the same as lastPathname when navigating
+   * in a linear manner but is almost always different
+   * when using tabs.
+   */
   pushedByRoute?: string;
   tab?: string;
   position?: number;

packages/vue/test-app/src/main.ts

@@ -38,7 +38,7 @@ window.addEventListener('unhandledrejection', (err) => {
 });
 
 const app = createApp(App)
-  .use(IonicVue)
+  .use(IonicVue, { hardwareBackButton: true })
   .use(router);
 
 router.isReady().then(() => {

packages/vue/test-app/tests/e2e/specs/hbb.js

@@ -0,0 +1,96 @@
+describe('Hardware Back Button', () => {
+  it('should correctly go back to Tab 1', () => {
+    cy.visit('http://localhost:8080/tabs');
+
+    cy.get('ion-tab-button#tab-button-tab2').click();
+    cy.ionPageHidden('tab1');
+    cy.ionPageVisible('tab2');
+
+    cy.get('#add-tab').click();
+
+    cy.get('ion-tab-button#tab-button-tab4').click();
+    cy.ionPageHidden('tab2');
+    cy.ionPageVisible('tab4');
+
+    cy.get('ion-tab-button#tab-button-tab1').click();
+    cy.ionPageHidden('tab4');
+    cy.ionPageVisible('tab1');
+
+    cy.hardwareBackButton();
+    cy.ionPageHidden('tab1');
+    cy.ionPageVisible('tab4');
+
+    cy.hardwareBackButton();
+    cy.ionPageHidden('tab4');
+    cy.ionPageVisible('tab2');
+
+    cy.hardwareBackButton();
+    cy.ionPageHidden('tab2');
+    cy.ionPageVisible('tab1');
+  });
+
+  it('should correctly go back to the root tab from child pages', () => {
+    cy.visit('http://localhost:8080');
+
+    cy.routerPush('/tabs');
+    cy.ionPageHidden('home');
+    cy.ionPageVisible('tab1');
+
+    cy.routerPush('/tabs/tab1/childone');
+    cy.ionPageHidden('tab1');
+    cy.ionPageVisible('tab1childone');
+
+    cy.routerPush('/tabs/tab1/childtwo');
+    cy.ionPageHidden('tab1childone');
+    cy.ionPageVisible('tab1childtwo');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1childtwo');
+    cy.ionPageVisible('tab1childone');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1childone');
+    cy.ionPageVisible('tab1');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1');
+    cy.ionPageVisible('home');
+  });
+
+  // TODO FW-1389
+  it.skip('should correctly go back to the root tab after switching pages', () => {
+    cy.visit('http://localhost:8080');
+
+    cy.routerPush('/tabs');
+    cy.ionPageHidden('home');
+    cy.ionPageVisible('tab1');
+
+    cy.routerPush('/tabs/tab1/childone');
+    cy.ionPageHidden('tab1');
+    cy.ionPageVisible('tab1childone');
+
+    cy.routerPush('/tabs/tab1/childtwo');
+    cy.ionPageHidden('tab1childone');
+    cy.ionPageVisible('tab1childtwo');
+
+    cy.get('ion-tab-button#tab-button-tab2').click();
+    cy.ionPageHidden('tab1childtwo');
+    cy.ionPageVisible('tab2');
+
+    cy.get('ion-tab-button#tab-button-tab1').click();
+    cy.ionPageHidden('tab2');
+    cy.ionPageVisible('tab1childtwo');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1childtwo');
+    cy.ionPageVisible('tab1childone');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1childone');
+    cy.ionPageVisible('tab1');
+
+    cy.hardwareBackButton();
+    cy.ionPageDoesNotExist('tab1');
+    cy.ionPageVisible('home');
+  });
+})

packages/vue/test-app/tests/e2e/specs/tabs.js

@@ -476,6 +476,64 @@ describe('Tabs', () => {
     cy.ionPageVisible('home');
     cy.ionPageDoesNotExist('tabs');
   });
+
+  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24936
+  it('should correctly go back after changing tabs', () => {
+    cy.visit('http://localhost:8080/tabs/tab1');
+
+    cy.routerPush('/tabs/tab1/childone');
+    cy.ionPageVisible('tab1childone');
+    cy.ionPageHidden('tab1');
+
+    cy.routerPush('/tabs/tab1/childtwo');
+    cy.ionPageVisible('tab1childtwo');
+    cy.ionPageHidden('tab1childone');
+
+    cy.get('ion-tab-button#tab-button-tab2').click();
+    cy.ionPageHidden('tab1childtwo');
+    cy.ionPageVisible('tab2');
+
+    cy.get('ion-tab-button#tab-button-tab1').click();
+    cy.ionPageHidden('tab2');
+    cy.ionPageVisible('tab1childtwo');
+
+    cy.ionBackClick('tab1childtwo');
+    cy.ionPageVisible('tab1childone');
+    cy.ionPageDoesNotExist('tab1childtwo');
+
+    cy.ionBackClick('tab1childone');
+    cy.ionPageVisible('tab1');
+    cy.ionPageDoesNotExist('tab1childone');
+  });
+
+  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24303
+  it('should correctly perform router.go without errors after navigating into tabs', () => {
+    cy.visit('http://localhost:8080/');
+
+    cy.routerPush('/inputs');
+    cy.ionPageVisible('inputs');
+    cy.ionPageHidden('home');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('inputs');
+
+    cy.routerPush('/tabs/tab1');
+    cy.ionPageVisible('tab1');
+    cy.ionPageHidden('routing');
+
+    cy.get('ion-tab-button#tab-button-tab2').click();
+    cy.ionPageHidden('tab1');
+    cy.ionPageVisible('tab2');
+
+    cy.get('ion-tab-button#tab-button-tab1').click();
+    cy.ionPageHidden('tab2');
+    cy.ionPageVisible('tab1');
+
+    cy.routerGo(-1);
+    cy.ionPageVisible('tab2');
+    cy.ionPageHidden('tab1');
+  });
 })
 
 describe('Tabs - Swipe to Go Back', () => {

packages/vue/test-app/tests/e2e/support/commands.js

@@ -106,3 +106,15 @@ Cypress.Commands.add('ionBackButtonHidden', (pageId) => {
     .find('ion-back-button')
     .should('not.be.visible')
 });
+
+/**
+ * If running in a browser, hardwareBackButton: true
+ * must be set in Ionic config for this to work.
+ */
+Cypress.Commands.add('hardwareBackButton', () => {
+  cy.document().then(doc => {
+    const ev = new CustomEvent('backbutton');
+
+    doc.dispatchEvent(ev);
+  })
+})