diff --git a/packages/vue-router/src/locationHistory.ts b/packages/vue-router/src/locationHistory.ts index 556e3bab6c..c86d474c1f 100644 --- a/packages/vue-router/src/locationHistory.ts +++ b/packages/vue-router/src/locationHistory.ts @@ -83,17 +83,25 @@ 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 @@ -101,19 +109,31 @@ export const createLocationHistory = () => { * 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; } diff --git a/packages/vue-router/src/router.ts b/packages/vue-router/src/router.ts index 81b95ced7e..71fc08c248 100644 --- a/packages/vue-router/src/router.ts +++ b/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 diff --git a/packages/vue-router/src/types.ts b/packages/vue-router/src/types.ts index 10f318a222..76cd7602f3 100644 --- a/packages/vue-router/src/types.ts +++ b/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; diff --git a/packages/vue/test-app/src/main.ts b/packages/vue/test-app/src/main.ts index c86c39adbb..e2782fdd97 100644 --- a/packages/vue/test-app/src/main.ts +++ b/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(() => { diff --git a/packages/vue/test-app/tests/e2e/specs/hbb.js b/packages/vue/test-app/tests/e2e/specs/hbb.js new file mode 100644 index 0000000000..0e95070849 --- /dev/null +++ b/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'); + }); +}) diff --git a/packages/vue/test-app/tests/e2e/specs/tabs.js b/packages/vue/test-app/tests/e2e/specs/tabs.js index ba31e949cd..643196f76b 100644 --- a/packages/vue/test-app/tests/e2e/specs/tabs.js +++ b/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', () => { diff --git a/packages/vue/test-app/tests/e2e/support/commands.js b/packages/vue/test-app/tests/e2e/support/commands.js index 9d0ba08b75..9d3f373664 100644 --- a/packages/vue/test-app/tests/e2e/support/commands.js +++ b/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); + }) +})