From a877abdc3f8dfa32ca98afee5f45ac73cf6ade91 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 25 Sep 2026 06:57:42 -0700 Subject: [PATCH] fix(react-router): allow swipe back over a splat container page --- .../src/ReactRouter/StackManager.tsx | 15 +- packages/react-router/test/base/src/App.tsx | 30 +--- packages/react-router/test/base/src/index.tsx | 16 +- .../react-router/test/base/src/ionic-setup.ts | 28 ++++ .../react-router/test/base/src/pages/Main.tsx | 5 + .../RootSplatSiblingApp.tsx | 152 ++++++++++++++++++ .../base/src/root-splat-sibling/basename.ts | 8 + .../e2e/playwright/root-splat-sibling.spec.ts | 151 +++++++++++++++++ 8 files changed, 370 insertions(+), 35 deletions(-) create mode 100644 packages/react-router/test/base/src/ionic-setup.ts create mode 100644 packages/react-router/test/base/src/root-splat-sibling/RootSplatSiblingApp.tsx create mode 100644 packages/react-router/test/base/src/root-splat-sibling/basename.ts create mode 100644 packages/react-router/test/base/tests/e2e/playwright/root-splat-sibling.spec.ts diff --git a/packages/react-router/src/ReactRouter/StackManager.tsx b/packages/react-router/src/ReactRouter/StackManager.tsx index 41ea6069ef3..2f76dd15421 100644 --- a/packages/react-router/src/ReactRouter/StackManager.tsx +++ b/packages/react-router/src/ReactRouter/StackManager.tsx @@ -1417,22 +1417,22 @@ export class StackManager extends React.PureComponent { const { routeInfo } = this.props; const swipeBackRouteInfo = this.getSwipeBackRouteInfo(); const enteringViewItem = this.findEnteringViewForSwipe(swipeBackRouteInfo); + const leavingViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id, false); // View might have mount=false but ionPageElement still in DOM const ionPageInDocument = Boolean( enteringViewItem?.ionPageElement && document.body.contains(enteringViewItem.ionPageElement) ); - // For wildcard/parameterized routes, the pattern path (e.g. "/foo/*") will - // never equal the resolved pathname (e.g. "/foo/bar"), so the pattern check - // alone isn't sufficient. Also, verify the entering view's resolved pathname - // differs from the current pathname — if they match, the entering and leaving - // views are the same and the swipe gesture shouldn't start. + // A splat consumes whatever is left of the pathname, so its match resolves to the whole + // current pathname and comparing pathnames can't tell a container page underneath a + // pushed sibling from the page being left. Compare the view items instead, like onEnd + // does. Without a leaving view onStart skips the transition, so reject that here too. const canStartSwipe = !!enteringViewItem && + !!leavingViewItem && (enteringViewItem.mount || ionPageInDocument) && - enteringViewItem.routeData.match.pattern.path !== routeInfo.pathname && - enteringViewItem.routeData.match.pathname !== routeInfo.pathname; + enteringViewItem !== leavingViewItem; debug('SwipeBackCanStart', () => ({ outletId: this.id, @@ -1442,6 +1442,7 @@ export class StackManager extends React.PureComponent { enteringViewPath: enteringViewItem?.reactElement?.props?.path, enteringMount: enteringViewItem?.mount, ionPageInDocument, + leavingViewId: leavingViewItem?.id, canStartSwipe, })); diff --git a/packages/react-router/test/base/src/App.tsx b/packages/react-router/test/base/src/App.tsx index 5020d292fc7..96cb242cedc 100644 --- a/packages/react-router/test/base/src/App.tsx +++ b/packages/react-router/test/base/src/App.tsx @@ -1,25 +1,9 @@ -import { IonApp, setupIonicReact, LogLevel, IonRouterOutlet } from '@ionic/react'; +import { IonApp, IonRouterOutlet } from '@ionic/react'; import React from 'react'; import { Route, Navigate } from 'react-router-dom'; -/* Core CSS required for Ionic components to work properly */ -import '@ionic/react/css/core.css'; - -/* Basic CSS for apps built with Ionic */ -import '@ionic/react/css/normalize.css'; -import '@ionic/react/css/structure.css'; -import '@ionic/react/css/typography.css'; - -/* Optional CSS utils that can be commented out */ -import '@ionic/react/css/display.css'; -import '@ionic/react/css/flex-utils.css'; -import '@ionic/react/css/float-elements.css'; -import '@ionic/react/css/padding.css'; -import '@ionic/react/css/text-alignment.css'; -import '@ionic/react/css/text-transformation.css'; - -/* Theme variables */ -import './theme/variables.css'; +/* Ionic CSS and setupIonicReact */ +import './ionic-setup'; import Main from './pages/Main'; import { IonReactRouter } from '@ionic/react-router'; @@ -66,7 +50,10 @@ import { Step1, Step2, Step3, Step4 } from './pages/replace-params/ReplaceParams import { ParamSwipeBack, ParamSwipeBackB } from './pages/param-swipe-back/ParamSwipeBack'; import TabLifecycle from './pages/tab-lifecycle/TabLifecycle'; import TabLifecycleOutside from './pages/tab-lifecycle/TabLifecycleOutside'; -import { RouterLinkModifierClick, RouterLinkModifierClickTarget } from './pages/router-link-modifier-click/RouterLinkModifierClick'; +import { + RouterLinkModifierClick, + RouterLinkModifierClickTarget, +} from './pages/router-link-modifier-click/RouterLinkModifierClick'; import { NavigateRootPageA, NavigateRootPageB, NavigateRootPageC } from './pages/navigate-root/NavigateRoot'; import SuspenseOutlet from './pages/suspense-outlet/SuspenseOutlet'; import { PropsUpdateDirect, PropsUpdateRoutesWrapper } from './pages/props-update/PropsUpdate'; @@ -74,9 +61,6 @@ import DisabledButton from './pages/disabled-button/DisabledButton'; import SplatSibling from './pages/splat-sibling/SplatSibling'; import { EmptyPathSibling, IndexSibling } from './pages/index-sibling/IndexSibling'; -// Debug logs on so failing specs include the navigation diagnostics. -setupIonicReact({ logLevel: LogLevel.DEBUG }); - const App: React.FC = () => { return ( diff --git a/packages/react-router/test/base/src/index.tsx b/packages/react-router/test/base/src/index.tsx index de6c73b77f4..3c49e97c7a9 100644 --- a/packages/react-router/test/base/src/index.tsx +++ b/packages/react-router/test/base/src/index.tsx @@ -2,11 +2,17 @@ import React from 'react'; import { createRoot } from 'react-dom/client'; import App from './App'; +import RootSplatSiblingApp from './root-splat-sibling/RootSplatSiblingApp'; +import { ROOT_SPLAT_SIBLING_BASENAME } from './root-splat-sibling/basename'; + +/** + * A root-level splat route swallows every pathname in its outlet, so it can't share App's + * route tree. It gets its own root, picked here by pathname before anything renders. + */ +const { pathname } = window.location; +const isRootSplatSibling = + pathname === ROOT_SPLAT_SIBLING_BASENAME || pathname.startsWith(`${ROOT_SPLAT_SIBLING_BASENAME}/`); const container = document.getElementById('root'); const root = createRoot(container!); -root.render( - - - -); +root.render({isRootSplatSibling ? : }); diff --git a/packages/react-router/test/base/src/ionic-setup.ts b/packages/react-router/test/base/src/ionic-setup.ts new file mode 100644 index 00000000000..bd9f4918b32 --- /dev/null +++ b/packages/react-router/test/base/src/ionic-setup.ts @@ -0,0 +1,28 @@ +/** + * Ionic CSS and runtime config. Both App and RootSplatSiblingApp import this because + * App.test.tsx renders App with no index.tsx in the graph. + * + * Debug logging is on so a failing spec includes the navigation diagnostics. + */ +import { setupIonicReact, LogLevel } from '@ionic/react'; + +/* Core CSS required for Ionic components to work properly */ +import '@ionic/react/css/core.css'; + +/* Basic CSS for apps built with Ionic */ +import '@ionic/react/css/normalize.css'; +import '@ionic/react/css/structure.css'; +import '@ionic/react/css/typography.css'; + +/* Optional CSS utils that can be commented out */ +import '@ionic/react/css/display.css'; +import '@ionic/react/css/flex-utils.css'; +import '@ionic/react/css/float-elements.css'; +import '@ionic/react/css/padding.css'; +import '@ionic/react/css/text-alignment.css'; +import '@ionic/react/css/text-transformation.css'; + +/* Theme variables */ +import './theme/variables.css'; + +setupIonicReact({ logLevel: LogLevel.DEBUG }); diff --git a/packages/react-router/test/base/src/pages/Main.tsx b/packages/react-router/test/base/src/pages/Main.tsx index 4e9d66f5f7d..3dc6709f531 100644 --- a/packages/react-router/test/base/src/pages/Main.tsx +++ b/packages/react-router/test/base/src/pages/Main.tsx @@ -10,6 +10,7 @@ import { IonLabel, } from '@ionic/react'; import React from 'react'; +import { ROOT_SPLAT_SIBLING_BASENAME } from '../root-splat-sibling/basename'; const Main: React.FC = () => { return ( @@ -162,6 +163,10 @@ const Main: React.FC = () => { Empty Path Sibling + {/* A separate React root, so a plain href rather than a routerLink. */} + + Root Splat Sibling + diff --git a/packages/react-router/test/base/src/root-splat-sibling/RootSplatSiblingApp.tsx b/packages/react-router/test/base/src/root-splat-sibling/RootSplatSiblingApp.tsx new file mode 100644 index 00000000000..d0641cdd113 --- /dev/null +++ b/packages/react-router/test/base/src/root-splat-sibling/RootSplatSiblingApp.tsx @@ -0,0 +1,152 @@ +import { + IonApp, + IonBackButton, + IonButtons, + IonContent, + IonHeader, + IonIcon, + IonItem, + IonLabel, + IonList, + IonPage, + IonRouterOutlet, + IonTabBar, + IonTabButton, + IonTabs, + IonTitle, + IonToolbar, +} from '@ionic/react'; +import { IonReactRouter } from '@ionic/react-router'; +import { ellipse, triangle } from 'ionicons/icons'; +import React, { useState } from 'react'; +import { Navigate, Route, useParams } from 'react-router-dom'; + +/* Ionic CSS and setupIonicReact */ +import '../ionic-setup'; +import { ROOT_SPLAT_SIBLING_BASENAME } from './basename'; +import TestDescription from '../components/TestDescription'; + +/** + * A splat container page holding tabs, next to a more specific sibling that gets pushed over + * the whole tab bar, in the root outlet. + * + * A root catch-all can't live in the shared route tree in App.tsx without swallowing every + * other spec's pathname, so this app gets its own basename and index.tsx mounts it instead + * of App for that prefix. + */ + +/** + * The two spellings of a root splat fail differently, so "?splat=bare" runs the same flows + * against a bare "*" instead of "/*". Stamped onto the tabs page as data-splat so a test can + * confirm which spelling is live. + */ +const splatPath = new URLSearchParams(window.location.search).get('splat') === 'bare' ? '*' : '/*'; + +/** A fresh mount gets a fresh instance id, which tells a remount apart from a reveal. */ +let instanceCounter = 0; +const nextInstanceId = () => `tabs-${++instanceCounter}`; + +const Feed: React.FC = () => { + const [count, setCount] = useState(0); + + return ( + + + setCount((c) => c + 1)}> + Increment + + + Open item 12 + + +
{count}
+ + Increment the counter, then open item 12. The detail page should push over the whole tab bar, leaving these tabs + mounted behind it. Going back should reveal the same tabs with the counter unchanged. + + + ); +}; + +const FeedTab: React.FC = () => ( + + + + Feed + + + + +); + +const ProfileTab: React.FC = () => ( + + + + Profile + + + +
Profile
+ + +); + +/** The container page the splat route renders. */ +const Tabs: React.FC = () => { + const [instanceId] = useState(nextInstanceId); + + return ( + // These land on both the .ion-page div and the ion-tabs element, so select with div.ion-page. + + + } /> + } /> + } /> + + + + + Feed + + + + Profile + + + + ); +}; + +const Detail: React.FC = () => { + const { id } = useParams<{ id: string }>(); + + return ( + + + + + + + Detail + + + +
{id}
+ + + ); +}; + +const RootSplatSiblingApp: React.FC = () => ( + + + + } /> + } /> + + + +); + +export default RootSplatSiblingApp; diff --git a/packages/react-router/test/base/src/root-splat-sibling/basename.ts b/packages/react-router/test/base/src/root-splat-sibling/basename.ts new file mode 100644 index 00000000000..3b4afe66ab5 --- /dev/null +++ b/packages/react-router/test/base/src/root-splat-sibling/basename.ts @@ -0,0 +1,8 @@ +/** + * The prefix the root splat sibling app is served under, appended to Vite's base so + * previews keep working. + * + * Kept apart from the app itself so Main.tsx can link to it without pulling a second + * IonReactRouter into App's module graph. + */ +export const ROOT_SPLAT_SIBLING_BASENAME = `${import.meta.env?.BASE_URL?.replace(/\/$/, '') || ''}/root-splat-sibling`; diff --git a/packages/react-router/test/base/tests/e2e/playwright/root-splat-sibling.spec.ts b/packages/react-router/test/base/tests/e2e/playwright/root-splat-sibling.spec.ts new file mode 100644 index 00000000000..ab3ab73f6d5 --- /dev/null +++ b/packages/react-router/test/base/tests/e2e/playwright/root-splat-sibling.spec.ts @@ -0,0 +1,151 @@ +import { test, expect } from '@playwright/test'; + +import { ionSwipeToGoBack } from './utils/drag-utils'; +import { ionBackClick, ionPageHidden, ionPageVisible, withTestingMode } from './utils/test-utils'; + +/** + * A splat route can be the outlet's container page rather than a 404. When a more specific + * sibling is pushed over it, the container must stay mounted behind so back reveals the same + * page with its state intact. splat-sibling.spec.ts covers that one level down; this covers + * it in the root outlet, which is served under its own basename. + * + * https://github.com/ionic-team/ionic-framework/issues/31477 + */ +test.describe('root splat route with a more specific sibling', () => { + test('keeps the root splat tabs mounted behind a pushed sibling', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31477', + }); + + await page.goto(withTestingMode('/root-splat-sibling/feed')); + await ionPageVisible(page, 'root-splat-sibling-tabs'); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + await page.locator('[data-testid="open-detail"]').click(); + + await ionPageVisible(page, 'root-splat-sibling-detail'); + await expect(page.locator('[data-testid="detail-id"]')).toHaveText('12'); + + // The tabs are the page underneath, so they stay in the DOM and are just hidden. + await ionPageHidden(page, 'root-splat-sibling-tabs'); + await expect(page.locator('ion-tabs')).toHaveCount(1); + }); + + test('reveals the same tabs page with its state on back', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31477', + }); + + await page.goto(withTestingMode('/root-splat-sibling/feed')); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + const tabsPage = page.locator('div.ion-page[data-pageid="root-splat-sibling-tabs"]'); + // Pins the default spelling, so inverting the ternary fails here and not only in the bare test. + await expect(tabsPage).toHaveAttribute('data-splat', '/*'); + const originalInstance = await tabsPage.getAttribute('data-instance'); + + await page.locator('[data-testid="increment"]').click(); + await page.locator('[data-testid="increment"]').click(); + await expect(page.locator('[data-testid="count"]')).toHaveText('2'); + + await page.locator('[data-testid="open-detail"]').click(); + await ionPageVisible(page, 'root-splat-sibling-detail'); + + await ionBackClick(page, 'root-splat-sibling-detail'); + + await ionPageVisible(page, 'root-splat-sibling-tabs'); + await ionPageVisible(page, 'root-splat-sibling-feed'); + // A replacement page would carry a fresh instance id and a counter back at 0. + await expect(tabsPage).toHaveAttribute('data-instance', originalInstance!); + await expect(page.locator('[data-testid="count"]')).toHaveText('2'); + }); + + // Animations stay on here, so the gesture reveals the page underneath rather than a + // commit doing it. + test('reveals the tabs page while swiping back', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31477', + }); + + await page.goto('/root-splat-sibling/feed?ionic:mode=ios'); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + await page.locator('[data-testid="increment"]').click(); + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + + await page.locator('[data-testid="open-detail"]').click(); + await ionPageVisible(page, 'root-splat-sibling-detail'); + // ionPageHidden resolves early here, because the deactivation scan applies + // ion-page-hidden at render time rather than on commit, so wait out the push + // transition before starting the gesture. + await ionPageHidden(page, 'root-splat-sibling-tabs'); + await page.waitForTimeout(600); + + await ionSwipeToGoBack(page, true, 'ion-router-outlet#root-splat-sibling-outlet'); + + await ionPageVisible(page, 'root-splat-sibling-tabs'); + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + }); + + // A bare "*" fails differently from "/*", so run the same flows against both spellings. + test('keeps a bare "*" container mounted and restores it on back', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31477', + }); + + await page.goto(withTestingMode('/root-splat-sibling/feed?splat=bare')); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + const tabsPage = page.locator('div.ion-page[data-pageid="root-splat-sibling-tabs"]'); + // The query param is the only thing selecting the bare spelling, and navigation drops it. + await expect(tabsPage).toHaveAttribute('data-splat', '*'); + const originalInstance = await tabsPage.getAttribute('data-instance'); + + await page.locator('[data-testid="increment"]').click(); + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + + await page.locator('[data-testid="open-detail"]').click(); + await ionPageVisible(page, 'root-splat-sibling-detail'); + await ionPageHidden(page, 'root-splat-sibling-tabs'); + + await ionBackClick(page, 'root-splat-sibling-detail'); + + await ionPageVisible(page, 'root-splat-sibling-tabs'); + await expect(tabsPage).toHaveAttribute('data-instance', originalInstance!); + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + }); + + // Revealing the container isn't enough on its own, the tabs underneath have to still + // route after back. + test('leaves the revealed container routable through its tab bar', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31477', + }); + + await page.goto(withTestingMode('/root-splat-sibling/feed')); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + await page.locator('[data-testid="increment"]').click(); + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + + await page.locator('[data-testid="open-detail"]').click(); + await ionPageVisible(page, 'root-splat-sibling-detail'); + + await ionBackClick(page, 'root-splat-sibling-detail'); + await ionPageVisible(page, 'root-splat-sibling-feed'); + + await page.locator('[data-testid="tab-profile"]').click(); + await ionPageVisible(page, 'root-splat-sibling-profile'); + await expect(page.locator('[data-testid="profile-content"]')).toBeVisible(); + + await page.locator('[data-testid="tab-feed"]').click(); + await ionPageVisible(page, 'root-splat-sibling-feed'); + // Tabs keep their pages mounted, so the counter survives the round trip too. + await expect(page.locator('[data-testid="count"]')).toHaveText('1'); + }); +});