diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index c50689c654b..e3793e64b08 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -1,6 +1,6 @@ import { useEffect, useState, type ReactNode } from 'react' import { Linking, Pressable, StyleSheet, Text, View } from 'react-native' -import { useNavigation, useRouter } from 'expo-router' +import { useNavigation, useNavigationContainerRef, useRouter } from 'expo-router' import { useSafeAreaInsets } from 'react-native-safe-area-context' import { OrcaMobileWebShellView, @@ -30,6 +30,7 @@ import { useMobileWebShellSession } from './use-mobile-web-shell-session' import { usePageHostSnapshot } from './use-page-host-snapshot' import { useReportedHostAreaServing } from './host-area-serving' import { isHostRouteOf } from './page-route-policy' +import { nativeStackReturn } from './native-stack-return' import { PageRouteUnavailableScreen } from './PageRouteUnavailableScreen' import { useResponsiveLayout } from '../layout/responsive-layout' import { storageReadParamsSchema, type BridgeNativeVerb } from './bridge/bridge-native-verbs' @@ -142,6 +143,7 @@ export function MobileWebShellScreen({ const overlay = onOverlayLoad !== undefined const router = useRouter() const navigation = useNavigation() + const navigationContainer = useNavigationContainerRef() const popShellStack = useShellStackPop() const { isWideLayout } = useResponsiveLayout() const { droppedBinaryFrames, reportDroppedBinaryFrames } = useMobileWebShellDroppedFrames() @@ -252,15 +254,32 @@ export function MobileWebShellScreen({ console.warn('[web-shell] refused to open this screen', issue) reportShellFailure('document-load-failed') }, - // Pushed, never replaced: the page stays mounted underneath, so Back reveals it with no - // download and no second `init`. + // A new screen is pushed, never replaced: the page stays mounted underneath, so Back reveals it + // with no download and no second `init`. onNavigate: (href: string) => { // An overlay hands its place to where the page went: Back there returns to the screen beneath. if (overlay) { return void router.replace(href) } - // Wide, this host's route is the area-owning session below: back to it, or in place of this - // one from a cold deep link, never a second on top. `dismissTo` takes the href's params. + // A screen already on the stack is returned to, as system Back would: the page's own exits + // ask for the screen beneath it, and a push would stack a second copy over a live one. + const rootState = navigationContainer.getRootState() + const onStack = rootState ? nativeStackReturn(rootState, href) : null + if (onStack === 'current') { + return + } + if (onStack) { + navigation.dispatch({ + type: 'POP', + payload: { count: onStack.count }, + target: onStack.target, + source: onStack.source + }) + return + } + // Wide, this host's route is the area-owning session even when it is not on the stack as + // asked: `dismissTo` pops to it by name with the href's params, or replaces this one after a + // cold deep link, so it is never a second on top. if (isWideLayout && isHostRouteOf(href, hostId)) { router.dismissTo(href) return diff --git a/mobile/src/mobile-web-shell/catch-all-page-route-states.test.tsx b/mobile/src/mobile-web-shell/catch-all-page-route-states.test.tsx index fa42ad9b426..5719cd76600 100644 --- a/mobile/src/mobile-web-shell/catch-all-page-route-states.test.tsx +++ b/mobile/src/mobile-web-shell/catch-all-page-route-states.test.tsx @@ -102,6 +102,7 @@ vi.mock('expo-router', () => ({ }), usePathname: () => '/h/host-1', useNavigation: () => ({ setOptions: vi.fn() }), + useNavigationContainerRef: () => ({ getRootState: () => undefined }), useFocusEffect: () => {} })) vi.mock('../../modules/orca-mobile-web-shell/src', async () => { diff --git a/mobile/src/mobile-web-shell/host-area-leave-session.test.tsx b/mobile/src/mobile-web-shell/host-area-leave-session.test.tsx index def7f3e679e..e6372993c95 100644 --- a/mobile/src/mobile-web-shell/host-area-leave-session.test.tsx +++ b/mobile/src/mobile-web-shell/host-area-leave-session.test.tsx @@ -1,8 +1,8 @@ /** - * Leaving a detail session for the host route on a wide layout: the page's back chevron - * (`leaveSession`, which replaces to `/h/` when nothing is below it) and the missing-worktree - * bounce (`/h/?notice=…`). Under a page that owns the host area that hop reaches the shell, - * which must land on one host route rather than stack a second host-area session on the session. + * Leaving a detail session for the host route: the page's back chevron (`leaveSession`, which + * replaces to `/h/` when nothing is below it) and the missing-worktree bounce + * (`/h/?notice=…`). That hop reaches the shell, which must land on one host route rather than + * stack a second host-area session on the session. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -44,6 +44,7 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { clientFrame, createFakeRpcClient } from './bridge-host-test-fakes' import { MobileWebShellScreen } from './MobileWebShellScreen' +import type { NativeStackState } from './native-stack-return' const HOST = '/h/host-1' const DETAIL = '/h/host-1/session/wt-1' @@ -54,16 +55,38 @@ const PHONE = { width: 390, height: 844 } /** A stack entry: the route's pathname and its search, as the native stack holds them. */ type Entry = string +/** The entries as the navigation container holds them: the host stack inside the root one. */ +function rootStateOf(stack: readonly Entry[]): NativeStackState { + const routes = stack.map((entry, index) => { + const [path = '', search = ''] = entry.split('?') + const [, , hostId = '', area, worktreeId] = path.split('/') + const query = Object.fromEntries(new URLSearchParams(search)) + return area === undefined + ? { key: `entry-${index}`, name: '[hostId]/index', params: { hostId, ...query } } + : { + key: `entry-${index}`, + name: `[hostId]/${area}/[worktreeId]`, + params: { hostId, worktreeId: worktreeId ?? '', ...query } + } + }) + const hostStack = { key: 'host-stack', index: routes.length - 1, routes } + return { key: 'root-stack', index: 0, routes: [{ key: 'h', name: 'h', state: hostStack }] } +} + /** - * The native stack the screen's calls act on, with expo-router 55's semantics: `push` adds, - * `dismissTo` is react-navigation's POP_TO — pop back to the nearest entry of that route and take - * the new params, or replace the current entry when there is none below it. + * The native stack the screen's calls act on, with expo-router 55's semantics: `push` adds, a + * dispatched `POP` removes that many, and `dismissTo` is react-navigation's POP_TO — pop back to + * the nearest entry of that route and take the new params, or replace the current entry when there + * is none below it. */ function applied(stack: readonly Entry[]): Entry[] { const next = [...stack] for (const [href] of dependencies.push.mock.calls) { next.push(String(href)) } + for (const [action] of dependencies.dispatch.mock.calls) { + next.splice(next.length - action.payload.count) + } for (const [href] of device.dismissTo.mock.calls) { const target = String(href) const route = target.split('?')[0] @@ -78,9 +101,14 @@ function applied(stack: readonly Entry[]): Entry[] { } /** The session page on screen, asking the shell for `href`. */ -async function hopFromSession(viewport: { width: number; height: number }, href: string) { +async function hopFromSession( + viewport: { width: number; height: number }, + stack: readonly Entry[], + href: string +) { device.width = viewport.width device.height = viewport.height + dependencies.rootState = rootStateOf(stack) dependencies.client = createFakeRpcClient() dependencies.state = harness.readyState('session-one') dependencies.pageRoutes = ['/h/[hostId]', '/h/[hostId]/session/[worktreeId]'] @@ -114,25 +142,31 @@ afterEach(() => { describe('leaving a wide detail session for its host route', () => { it('returns to the host-area session below it, so Back does not reopen the session', async () => { - await hopFromSession(IPAD, HOST) + await hopFromSession(IPAD, [HOST, DETAIL], HOST) expect(applied([HOST, DETAIL])).toEqual([HOST]) }) it('carries the bounce notice to that host route, and still stacks nothing', async () => { - await hopFromSession(IPAD, BOUNCE) + await hopFromSession(IPAD, [HOST, DETAIL], BOUNCE) expect(applied([HOST, DETAIL])).toEqual([BOUNCE]) }) it('lands on one host route from a cold deep link, with nothing below the session', async () => { - await hopFromSession(IPAD, BOUNCE) + await hopFromSession(IPAD, [DETAIL], BOUNCE) expect(applied([DETAIL])).toEqual([BOUNCE]) }) }) describe('the same hop on a phone', () => { - it('pushes, as it always has', async () => { - await hopFromSession(PHONE, HOST) + it('returns to the host route below rather than stacking a second one', async () => { + await hopFromSession(PHONE, [HOST, DETAIL], HOST) expect(device.dismissTo).not.toHaveBeenCalled() - expect(applied([HOST, DETAIL])).toEqual([HOST, DETAIL, HOST]) + expect(applied([HOST, DETAIL])).toEqual([HOST]) + }) + + it('pushes a route the stack does not hold, such as another worktree', async () => { + const other = '/h/host-1/session/wt-2' + await hopFromSession(PHONE, [HOST, DETAIL], other) + expect(applied([HOST, DETAIL])).toEqual([HOST, DETAIL, other]) }) }) diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx index 46013be226d..7bd0a28f136 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx +++ b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx @@ -12,6 +12,7 @@ import type { MobileWebShellUpdateNotice } from './mobile-web-shell-session-contract' import type { ShellPageFrame } from './shell-page-frame' +import type { NativeStackState } from './native-stack-return' /** * Everything the screen's cases mock away, as one mutable record, plus the helpers that mount it. @@ -33,6 +34,10 @@ export type ScreenDependencies = { back: Mock /** What the native stack answers: false is a page opened as the first screen on it. */ canGoBack: boolean + /** The navigation container's whole state; undefined before it is ready. */ + rootState: NativeStackState | undefined + /** Actions the screen dispatched onto its own place on the stack. */ + dispatch: Mock pathname: string pageRoutes: readonly string[] routeGrants: readonly string[] @@ -103,6 +108,8 @@ export function createScreenDependencies(): ScreenDependencies { push: vi.fn(), back: vi.fn(), canGoBack: true, + rootState: undefined, + dispatch: vi.fn(), pathname: '/h/host-1', pageRoutes: ['/h/[hostId]'], routeGrants: DEFAULT_ROUTE_GRANTS, @@ -154,6 +161,8 @@ export function resetScreenDependencies(dependencies: ScreenDependencies): void dependencies.openUrl.mockReset() dependencies.openUrl.mockImplementation(() => Promise.resolve(true)) dependencies.canGoBack = true + dependencies.rootState = undefined + dependencies.dispatch.mockReset() dependencies.pathname = '/h/host-1' dependencies.updateNotice = null dependencies.backHandlers.clear() diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-mocks.tsx b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-mocks.tsx index 02b85754ba5..b13db66e613 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-mocks.tsx +++ b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-mocks.tsx @@ -117,7 +117,11 @@ export function screenModuleMocks(dependencies: ScreenDependencies) { // Read by the pop latch, which clears on the route this shell is mounted at changing. usePathname: () => dependencies.pathname, // The screen's own place on the stack, which is where the iOS swipe-back is taken away. - useNavigation: () => ({ setOptions: dependencies.setScreenOptions }), + useNavigation: () => ({ + setOptions: dependencies.setScreenOptions, + dispatch: dependencies.dispatch + }), + useNavigationContainerRef: () => ({ getRootState: () => dependencies.rootState }), useFocusEffect: (effect: () => undefined | (() => void)) => React.useEffect(effect, [effect]) }), // A component rather than a host string: the React key is what makes a retry a rebuilt WebView, diff --git a/mobile/src/mobile-web-shell/native-stack-return.test.ts b/mobile/src/mobile-web-shell/native-stack-return.test.ts new file mode 100644 index 00000000000..3d1d80e6005 --- /dev/null +++ b/mobile/src/mobile-web-shell/native-stack-return.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest' +import { nativeStackReturn, type NativeStackState } from './native-stack-return' + +type Route = NativeStackState['routes'][number] + +function stack(key: string, routes: readonly Route[]): NativeStackState { + return { + key, + index: routes.length - 1, + routes: routes.map((route, index) => ({ ...route, key: `${key}/${index}` })) + } +} + +function hostStack(...routes: Route[]): Route { + return { name: 'h', state: stack('host-stack', routes) } +} + +// The root layout is a screen of expo-router's own navigator, so the walk starts one level up. +function rootStack(...routes: Route[]): NativeStackState { + return stack('expo-root', [{ name: '__root', state: stack('root-stack', routes) }]) +} + +const HOME = { name: 'index' } +const HOST = { name: '[hostId]/index', params: { hostId: 'mac/1' } } +const SESSION = { + name: '[hostId]/session/[worktreeId]', + params: { hostId: 'mac/1', worktreeId: 'wt-a' } +} + +describe('nativeStackReturn', () => { + it('pops back to Home under the host instead of stacking a second Home', () => { + expect(nativeStackReturn(rootStack(HOME, hostStack(HOST)), '/')).toEqual({ + target: 'root-stack', + source: 'root-stack/1', + count: 1 + }) + }) + + it('finds nothing when the host is the root, as after a cold deep link', () => { + expect(nativeStackReturn(rootStack(hostStack(HOST)), '/')).toBeNull() + }) + + it('returns to an exact path and query, however the href spells it', () => { + const files = { name: '[hostId]/files/[worktreeId]', params: SESSION.params } + const tabbed = { ...SESSION, params: { ...SESSION.params, tab: 'b', pane: '1' } } + const state = rootStack(HOME, hostStack(HOST, tabbed, files)) + expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a?pane=1&tab=b')).toEqual({ + target: 'host-stack', + source: 'host-stack/2', + count: 1 + }) + expect(nativeStackReturn(state, '/h/mac%2F1')).toMatchObject({ target: 'host-stack', count: 2 }) + expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a?pane=1&tab=c')).toBeNull() + expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-b?pane=1&tab=b')).toBeNull() + }) + + it('treats a param cleared to empty as absent', () => { + const cleared = { ...SESSION, params: { ...SESSION.params, paneKey: '' } } + const state = rootStack( + HOME, + hostStack(HOST, cleared, { name: '[hostId]/files/[worktreeId]', params: SESSION.params }) + ) + expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a')).toMatchObject({ count: 1 }) + }) + + it('names the focused screen as current rather than a pop', () => { + const state = rootStack(HOME, hostStack(HOST, SESSION)) + expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a')).toBe('current') + }) +}) diff --git a/mobile/src/mobile-web-shell/native-stack-return.ts b/mobile/src/mobile-web-shell/native-stack-return.ts new file mode 100644 index 00000000000..e9d3a879469 --- /dev/null +++ b/mobile/src/mobile-web-shell/native-stack-return.ts @@ -0,0 +1,116 @@ +import { stringifyRouteHref } from '../navigation/route-href' + +export type NativeStackState = Readonly<{ + key?: string + index?: number + routes: readonly NativeStackRoute[] +}> + +type NativeStackRoute = Readonly<{ + key?: string + name: string + params?: object + state?: NativeStackState +}> + +/** A pop of `count` screens off the navigator keyed `target`, counted from its route `source`. + * `source` is explicit because a screen's `dispatch` stamps its own key there otherwise, and a parent + * navigator that cannot find that key among its routes drops the pop. */ +export type NativeStackReturn = Readonly<{ target: string; source: string; count: number }> + +/** Expo-router file segments that name no part of the URL: `index`, `(group)` and its own `__root`. */ +function isUrlSegment(segment: string): boolean { + return segment !== 'index' && !/^\(.*\)$/.test(segment) && !segment.startsWith('__') +} + +/** A state not yet rehydrated may omit `index`; React Navigation reads that as its last route. */ +function focusedIndex(state: NativeStackState): number { + return state.index ?? state.routes.length - 1 +} + +function decodeSegment(segment: string): string { + try { + return decodeURIComponent(segment) + } catch { + return segment + } +} + +/** One spelling per URL: decoded path segments and the query sorted, as expo-router parses both. + * An empty value is dropped because screens clear a param with `''` (session `paneKey`), and the + * fragment because expo-router keeps it as a param no href spells the same way. */ +function canonicalHref(href: string): string { + const [path = '', search = ''] = href.split('#')[0].split('?') + const segments = path.split('/').filter(Boolean).map(decodeSegment) + const query = [...new URLSearchParams(search)] + .filter(([, value]) => value !== '') + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + return `/${segments.join('/')}?${new URLSearchParams(query).toString()}` +} + +/** The URL a route on the stack shows, or null when its params hold something no URL carries. */ +function routeHref(route: NativeStackRoute, parentNames: readonly string[]): string | null { + const names = [...parentNames, route.name] + const nested = route.state?.routes[focusedIndex(route.state)] + if (nested) { + return routeHref(nested, names) + } + const params: Record = {} + for (const [key, value] of Object.entries(route.params ?? {})) { + if (typeof value === 'string') { + params[key] = value + } else if (Array.isArray(value) && value.every((one) => typeof one === 'string')) { + params[key] = value + } else if (value !== undefined) { + return null + } + } + const pathname = `/${names + .flatMap((name) => name.split('/')) + .filter(isUrlSegment) + .join('/')}` + return canonicalHref(stringifyRouteHref({ pathname, params })) +} + +function returnWithin( + level: NativeStackState, + parentNames: readonly string[], + wanted: string +): NativeStackReturn | 'current' | null { + const current = focusedIndex(level) + const focused = level.routes[current] + if (!focused) { + return null + } + if (focused.state) { + // The deeper navigator first: its screens sit nearer the top than anything at this level. + const deeper = returnWithin(focused.state, [...parentNames, focused.name], wanted) + if (deeper) { + return deeper + } + } else if (routeHref(focused, parentNames) === wanted) { + return 'current' + } + if (!level.key || !focused.key) { + return null + } + for (let index = current - 1; index >= 0; index--) { + if (routeHref(level.routes[index], parentNames) === wanted) { + return { target: level.key, source: focused.key, count: current - index } + } + } + return null +} + +/** + * Where `href` already sits on the native stack: `'current'` when it is the focused screen, the pop + * that returns to the nearest screen below showing it, or null when it is nowhere on the stack. + * + * Path and query both count, so another worktree or tab under the same file is a different screen. + */ +export function nativeStackReturn( + state: NativeStackState, + href: string +): NativeStackReturn | 'current' | null { + return returnWithin(state, [], canonicalHref(href)) +}