From a6ee4c44fb57492e16183a82cb2b7f25531851dd Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 12 Mar 2026 06:43:01 -0700 Subject: [PATCH] fix(react-router): cleaning up tab navigation to use proper navigation step post debugging --- .../src/ReactRouter/IonRouter.tsx | 76 +++++++++++------- .../e2e/specs/tab-history-isolation.cy.js | 80 +++++++++++++++++++ packages/react/src/routing/LocationHistory.ts | 12 +++ 3 files changed, 141 insertions(+), 27 deletions(-) diff --git a/packages/react-router/src/ReactRouter/IonRouter.tsx b/packages/react-router/src/ReactRouter/IonRouter.tsx index 23f08fd946..0f77eddd2c 100644 --- a/packages/react-router/src/ReactRouter/IonRouter.tsx +++ b/packages/react-router/src/ReactRouter/IonRouter.tsx @@ -114,6 +114,18 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr // for future navigations once React has committed the mount. This avoids // duplicate entries when React StrictMode runs an extra render pre-commit. locationHistory.current.add(routeInfo); + + // If IonTabBar already called handleSetCurrentTab during render (before this + // effect), the tab was stored in currentTab.current but the history entry was + // not yet seeded. Apply the pending tab to the seed entry now. + if (currentTab.current) { + const ri = { ...locationHistory.current.current() }; + if (ri.tab !== currentTab.current) { + ri.tab = currentTab.current; + locationHistory.current.update(ri); + } + } + registerHistoryListener(handleHistoryChange); didMountRef.current = true; @@ -179,14 +191,17 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr const leavingUrl = leavingLocationInfo.pathname + leavingLocationInfo.search; if (leavingUrl !== location.pathname + location.search) { if (!incomingRouteParams.current) { - // Determine if the destination is a tab route by checking if it matches - // the pattern of tab routes (containing /tabs/ in the path) - const isTabRoute = /\/tabs(\/|$)/.test(location.pathname); - const tabToUse = isTabRoute ? currentTab.current : undefined; - - // If we're leaving tabs entirely, clear the current tab - if (!isTabRoute && currentTab.current) { - currentTab.current = undefined; + // Use history-based tab detection instead of URL-pattern heuristics, + // so tab routes work with any URL structure (not just paths containing "/tabs"). + // Fall back to currentTab.current only when the destination is within the + // current tab's path hierarchy (prevents non-tab routes from inheriting a tab). + let tabToUse = locationHistory.current.findTabForPathname(location.pathname); + if (!tabToUse && currentTab.current) { + const tabFirstRoute = locationHistory.current.getFirstRouteInfoForTab(currentTab.current); + const tabRootPath = tabFirstRoute?.pathname; + if (tabRootPath && (location.pathname === tabRootPath || location.pathname.startsWith(tabRootPath + '/'))) { + tabToUse = currentTab.current; + } } /** @@ -256,7 +271,7 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr let routeInfo: RouteInfo; // If we're navigating away from tabs to a non-tab route, clear the current tab - if (!/\/tabs(\/|$)/.test(location.pathname) && currentTab.current) { + if (!locationHistory.current.findTabForPathname(location.pathname) && currentTab.current) { currentTab.current = undefined; } @@ -441,7 +456,13 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr */ const handleSetCurrentTab = (tab: string) => { currentTab.current = tab; - const ri = { ...locationHistory.current.current() }; + const current = locationHistory.current.current(); + if (!current) { + // locationHistory not yet seeded (e.g., called during initial render + // before mount effect). The mount effect will seed the correct entry. + return; + } + const ri = { ...current }; if (ri.tab !== tab) { ri.tab = tab; locationHistory.current.update(ri); @@ -553,26 +574,27 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr let navigationTab = tab; // If no explicit tab is provided and we're in a tab context, - // check if the destination path is outside of the current tab context + // check if the destination path is outside of the current tab context. + // Uses history-based tab detection instead of URL pattern matching, + // so it works with any tab URL structure. if (!tab && currentTab.current && path) { - // Get the current route info to understand where we are - const currentRoute = locationHistory.current.current(); - - // If we're navigating from a tab route to a completely different path structure, - // we should clear the tab context. This is a simplified check that assumes - // tab routes share a common parent path. - if (currentRoute && currentRoute.pathname) { - // Extract the base tab path (e.g., /routing/tabs from /routing/tabs/home) - const tabBaseMatch = currentRoute.pathname.match(/^(.*\/tabs)/); - if (tabBaseMatch) { - const tabBasePath = tabBaseMatch[1]; - // If the new path doesn't start with the tab base path, we're leaving tabs - if (!path.startsWith(tabBasePath)) { + // Check if destination was previously visited in a tab context + const destinationTab = locationHistory.current.findTabForPathname(path); + if (destinationTab) { + // Previously visited as a tab route - use the known tab + navigationTab = destinationTab; + } else { + // New destination - check if it's a child of the current tab's root path + const tabFirstRoute = locationHistory.current.getFirstRouteInfoForTab(currentTab.current); + if (tabFirstRoute) { + const tabRootPath = tabFirstRoute.pathname; + if (path === tabRootPath || path.startsWith(tabRootPath + '/')) { + // Still within the current tab's path hierarchy + navigationTab = currentTab.current; + } else { + // Destination is outside the current tab context currentTab.current = undefined; navigationTab = undefined; - } else { - // Still within tabs, preserve the tab context - navigationTab = currentTab.current; } } } diff --git a/packages/react-router/test/base/tests/e2e/specs/tab-history-isolation.cy.js b/packages/react-router/test/base/tests/e2e/specs/tab-history-isolation.cy.js index 84eeeb3488..eb8c1a1feb 100644 --- a/packages/react-router/test/base/tests/e2e/specs/tab-history-isolation.cy.js +++ b/packages/react-router/test/base/tests/e2e/specs/tab-history-isolation.cy.js @@ -121,4 +121,84 @@ describe('Tab History Isolation', () => { cy.ionPageVisible('tab-a'); cy.url().should('include', '/tab-history-isolation/a'); }); + + /** + * Browser back/forward tests for non-"/tabs/" URL paths. + * + * These tests verify that per-tab history isolation works correctly + * when using browser back/forward buttons (POP events) with tab routes + * that do NOT contain "/tabs/" in their URL path. + * + * The tab-history-isolation routes use paths like /tab-history-isolation/a, + * /tab-history-isolation/b, etc. — no "/tabs/" segment. This exercises + * the context-driven tab detection (via location history) rather than + * URL-pattern-based detection. + */ + it('should preserve tab context through browser back from detail page within a tab', () => { + cy.visit(`http://localhost:${port}/tab-history-isolation/a`); + cy.ionPageVisible('tab-a'); + + // Navigate to details within Tab A + cy.get('#go-to-a-details').click(); + cy.ionPageHidden('tab-a'); + cy.ionPageVisible('tab-a-details'); + + // Use browser back - should go back within the same tab + cy.go('back'); + cy.ionPageVisible('tab-a'); + cy.url().should('include', '/tab-history-isolation/a'); + cy.url().should('not.include', '/details'); + }); + + it('should handle browser forward after browser back within a tab', () => { + cy.visit(`http://localhost:${port}/tab-history-isolation/a`); + cy.ionPageVisible('tab-a'); + + // Navigate to details within Tab A + cy.get('#go-to-a-details').click(); + cy.ionPageHidden('tab-a'); + cy.ionPageVisible('tab-a-details'); + + // Browser back + cy.go('back'); + cy.ionPageVisible('tab-a'); + cy.url().should('not.include', '/details'); + + // Browser forward - should return to details + cy.go('forward'); + cy.ionPageVisible('tab-a-details'); + cy.url().should('include', '/tab-history-isolation/a/details'); + }); + + it('should preserve per-tab history when using browser back after navigating within a tab and switching tabs', () => { + cy.visit(`http://localhost:${port}/tab-history-isolation/a`); + cy.ionPageVisible('tab-a'); + + // Navigate to details within Tab A + cy.get('#go-to-a-details').click(); + cy.ionPageHidden('tab-a'); + cy.ionPageVisible('tab-a-details'); + + // Switch to Tab B via tab bar + cy.ionTabClick('Tab B'); + cy.ionPageHidden('tab-a-details'); + cy.ionPageVisible('tab-b'); + + // Navigate to details within Tab B + cy.get('#go-to-b-details').click(); + cy.ionPageHidden('tab-b'); + cy.ionPageVisible('tab-b-details'); + + // Use browser back from Tab B details - should go back to Tab B root + cy.go('back'); + cy.ionPageVisible('tab-b'); + cy.url().should('include', '/tab-history-isolation/b'); + cy.url().should('not.include', '/details'); + + // Switch back to Tab A - should still show Tab A details (preserved) + cy.ionTabClick('Tab A'); + cy.ionPageHidden('tab-b'); + cy.ionPageVisible('tab-a-details'); + cy.url().should('include', '/tab-history-isolation/a/details'); + }); }); diff --git a/packages/react/src/routing/LocationHistory.ts b/packages/react/src/routing/LocationHistory.ts index bd2d3dd24c..53579324fe 100644 --- a/packages/react/src/routing/LocationHistory.ts +++ b/packages/react/src/routing/LocationHistory.ts @@ -175,4 +175,16 @@ export class LocationHistory { canGoBack() { return this.locationHistory.length > 1; } + + findTabForPathname(pathname: string): string | undefined { + for (const tab of Object.keys(this.tabHistory)) { + const routeInfos = this.tabHistory[tab]; + for (let i = routeInfos.length - 1; i >= 0; i--) { + if (routeInfos[i].pathname === pathname) { + return tab; + } + } + } + return undefined; + } }