diff --git a/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts b/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts index e1d9d87151069..1418c7f8cb455 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts @@ -19,8 +19,6 @@ import { formatComposerContextReference } from "@t3tools/shared/composerContextR import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; import { normalizeThreadPullRequestKey } from "@t3tools/client-runtime/thread-pull-requests"; import { useRef, useState } from "react"; -import { Platform } from "react-native"; -import { useSelectWorkspaceDetail } from "../workspace/useSelectWorkspaceDetail"; import { pullRequestComposerContext } from "../../lib/composerContext"; import { scopedThreadKey } from "../../lib/scopedEntities"; import { uuidv4 } from "../../lib/uuid"; @@ -76,16 +74,13 @@ export function usePullRequestThreads( const canAsk = !connected || canOperate; const canCheckout = useEnvironmentScope(target.environmentId, AuthSourceControlWriteScope); const navigation = useNavigation(); - const selectDetail = useSelectWorkspaceDetail(); const pending = useRef(false); const previousPrompts = useRef(new Map()); const openThread = (thread: LinkedThread) => { openThreadRoute({ environmentId: params.environmentId, threadId: thread.id }); }; - // Android shows one page at a time; iOS opens the thread over the pull request, so Back returns. const openThreadRoute = (destination: ReactNavigation.RootParamList["Thread"]) => { - if (Platform.OS === "android") selectDetail({ name: "Thread", params: destination }); - else navigation.dispatch(StackActions.push("Thread", destination)); + navigation.dispatch(StackActions.push("Thread", destination)); }; const [busy, setBusy] = useState(false); const ask = async (thread?: LinkedThread, task: PullRequestAgentTask = "ask") => { diff --git a/apps/mobile/src/features/threads/ThreadPullRequestsSheet.tsx b/apps/mobile/src/features/threads/ThreadPullRequestsSheet.tsx index 6eb94d3c232b8..fb933771199a3 100644 --- a/apps/mobile/src/features/threads/ThreadPullRequestsSheet.tsx +++ b/apps/mobile/src/features/threads/ThreadPullRequestsSheet.tsx @@ -37,6 +37,7 @@ export function ThreadPullRequestsSheet( ); const openPullRequest = useOpenPullRequest({ replace: true, + mode: "workspace", context: selectedThread ?? undefined, }); const actions = useLinkedPullRequestActions({ diff --git a/apps/mobile/src/features/threads/thread-route-state.test.ts b/apps/mobile/src/features/threads/thread-route-state.test.ts index 31b684bfaec31..e56175e2bed36 100644 --- a/apps/mobile/src/features/threads/thread-route-state.test.ts +++ b/apps/mobile/src/features/threads/thread-route-state.test.ts @@ -18,13 +18,26 @@ const { StackRouter, CommonActions, StackActions } = requireNavigation( }; describe("draft thread handoff", () => { - it("submits in the existing route and stops suppressing the live thread selection", () => { + it.each(["Home", "PullRequest"])("submits in place and preserves Back to %s", (origin) => { const router = StackRouter({}); - const options = { routeNames: ["Home", "Thread"], routeParamList: {}, routeGetIdList: {} }; + const options = { + routeNames: ["Home", "Thread", "PullRequest"], + routeParamList: {}, + routeGetIdList: {}, + }; const initial = router.getInitialState(options); + const parent = + origin === "Home" + ? initial + : router.getStateForAction( + initial, + StackActions.push("PullRequest", { number: 42 }), + options, + )!; + if (parent.stale !== false) throw new Error("Parent route was not initialized"); const draft = router.getStateForAction( - initial, - CommonActions.navigate("Thread", { draft: { projectId: "project" } }), + parent, + StackActions.push("Thread", { draft: { projectId: "project" } }), options, )!; if (!draft || draft.stale !== false) throw new Error("Draft route was not initialized"); @@ -34,6 +47,7 @@ describe("draft thread handoff", () => { CommonActions.setParams(submittedThreadParams("environment", "thread")), options, )!; + if (submitted.stale !== false) throw new Error("Submitted route was not initialized"); expect(submitted.routes.map((route) => route.key)).toEqual( draft.routes.map((route) => route.key), ); @@ -44,6 +58,9 @@ describe("draft thread handoff", () => { threadId: "thread", }); expect(isDraftThreadRoute(submitted.routes.at(-1))).toBe(false); + expect(router.getStateForAction(submitted, CommonActions.goBack(), options)?.routes).toEqual( + parent.routes, + ); }); }); diff --git a/apps/mobile/src/features/workspace/useAndroidWorkspaceNavigation.ts b/apps/mobile/src/features/workspace/useAndroidWorkspaceNavigation.ts index db50839c12add..84f5aed2867a9 100644 --- a/apps/mobile/src/features/workspace/useAndroidWorkspaceNavigation.ts +++ b/apps/mobile/src/features/workspace/useAndroidWorkspaceNavigation.ts @@ -32,19 +32,21 @@ export function useWorkspaceBackNavigation(tab: WorkspaceTab) { } export function useAndroidWorkspaceBack(tab: WorkspaceTab) { + const navigation = useNavigation(); const dispatch = useWorkspaceTabs()?.dispatch; const goBack = useWorkspaceBackNavigation(tab); useFocusEffect( useCallback(() => { if (Platform.OS !== "android") return; - dispatch?.({ type: "select", tab }); + // A chat opened from a PR, or a PR from a chat, keeps the tab it was opened from. + if ((navigation.getState()?.index ?? 0) <= 1) dispatch?.({ type: "select", tab }); const subscription = BackHandler.addEventListener("hardwareBackPress", () => { if (Keyboard.isVisible()) Keyboard.dismiss(); else goBack(); return true; }); return () => subscription.remove(); - }, [goBack, dispatch, tab]), + }, [navigation, goBack, dispatch, tab]), ); return goBack; } diff --git a/apps/mobile/src/lib/android-workspace-navigation.test.ts b/apps/mobile/src/lib/android-workspace-navigation.test.ts index 8dc91a0180f51..6c0a6f0de8eb0 100644 --- a/apps/mobile/src/lib/android-workspace-navigation.test.ts +++ b/apps/mobile/src/lib/android-workspace-navigation.test.ts @@ -1,5 +1,6 @@ import * as NodeModule from "node:module"; -import { describe, expect, it } from "vite-plus/test"; +import type { NavigationState } from "@react-navigation/native"; +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { androidWorkspaceTabFromNavigationState, androidWorkspaceTabNavigationState, @@ -13,18 +14,72 @@ import { } from "./android-workspace-navigation"; import type { RootStackResetAction } from "./root-stack-reset"; -const require = NodeModule.createRequire(import.meta.url); -const nativePackage = require.resolve("@react-navigation/native/package.json"); -const requireFromNative = NodeModule.createRequire(nativePackage); -const corePackage = requireFromNative.resolve("@react-navigation/core/package.json"); -const requireFromCore = NodeModule.createRequire(corePackage); -const { CommonActions, StackActions, StackRouter } = requireFromCore( - "@react-navigation/routers", -) as { - readonly CommonActions: typeof import("@react-navigation/native").CommonActions; - readonly StackActions: typeof import("@react-navigation/native").StackActions; - readonly StackRouter: typeof import("@react-navigation/native").StackRouter; -}; +function loadRouters() { + const require = NodeModule.createRequire(import.meta.url); + const nativePackage = require.resolve("@react-navigation/native/package.json"); + const corePackage = NodeModule.createRequire(nativePackage).resolve( + "@react-navigation/core/package.json", + ); + return NodeModule.createRequire(corePackage)("@react-navigation/routers") as { + readonly CommonActions: typeof import("@react-navigation/native").CommonActions; + readonly StackActions: typeof import("@react-navigation/native").StackActions; + readonly StackRouter: typeof import("@react-navigation/native").StackRouter; + }; +} + +vi.mock("@react-navigation/native", () => ({ + ...loadRouters(), + useNavigation: () => navigation, + useRoute: () => current.routes[current.index], + useFocusEffect: (callback: () => unknown) => callback(), +})); +vi.mock("react", () => ({ useCallback: (callback: unknown) => callback })); +vi.mock("react-native", () => ({ + Platform: { OS: "android" }, + Keyboard: { isVisible: () => false }, + BackHandler: { + addEventListener: (_event: string, callback: () => boolean) => { + hardwareBack = callback; + return { remove: () => {} }; + }, + }, +})); +vi.mock("../features/workspace/WorkspaceTabsContext", () => ({ + useWorkspaceTabs: () => ({ dispatch: selectTab }), +})); +vi.mock("../state/entities", () => ({ + useThreadShell: () => ({ projectId: "project-a" }), + useProjects: () => [], +})); +vi.mock("../features/workspace/useSelectWorkspaceDetail", () => ({ + useSelectWorkspaceDetail: () => (detail: typeof pullRequest) => { + current = apply( + current, + CommonActions.reset({ + stale: true, + index: 1, + routes: [{ name: "Home", key: current.routes[0]?.key }, detail], + }), + ); + }, +})); +vi.mock("../state/atom-registry", () => ({ appAtomRegistry: { get: () => new Map() } })); +vi.mock("../state/server", () => ({ environmentServerConfigsAtom: {} })); +vi.mock("./pullRequestNavigation", () => ({ + nativePullRequestDestination: (_url: string, context: object) => ({ + ...pullRequest.params, + ...context, + }), +})); +vi.mock("./openExternalUrl", () => ({ tryOpenExternalUrl: vi.fn() })); + +import { useOpenPullRequest } from "../native/useOpenPullRequest"; +import { + useAndroidWorkspaceBack, + useWorkspaceListNavigation, +} from "../features/workspace/useAndroidWorkspaceNavigation"; + +const { CommonActions, StackActions, StackRouter } = loadRouters(); const router = StackRouter({}); const options = { routeNames: [ @@ -82,6 +137,31 @@ function names(state: ReturnType) { return state.routes.map((route) => route.name); } +let current = router.getInitialState(options); +let hardwareBack: (() => boolean) | undefined; +const selectTab = vi.fn(); +type RouterAction = Parameters[1]; +const navigation = { + getState: () => current, + getParent: () => undefined, + canGoBack: () => router.getStateForAction(current, CommonActions.goBack(), options) !== null, + goBack: () => { + current = apply(current, CommonActions.goBack()); + }, + navigate: (name: string, params: object) => { + current = apply(current, CommonActions.navigate(name, params)); + }, + dispatch: (action: RouterAction | ((state: NavigationState) => RouterAction)) => { + current = apply(current, typeof action === "function" ? action(current) : action); + }, +}; + +beforeEach(() => { + current = router.getInitialState(options); + hardwareBack = undefined; + selectTab.mockClear(); +}); + describe("Android workspace navigation", () => { it("returns to a distinct previous thread, otherwise to the workspace list", () => { const parent = withThread(); @@ -253,3 +333,78 @@ describe("Android Settings tab", () => { expect(selectedTab(account, "usage")).toBe("usage"); }); }); + +describe("Android contextual navigation", () => { + it.each([false, true])( + "restores the original chat history after a PR link (replace sheet: %s)", + async (replace) => { + current = apply( + current, + StackActions.push("Thread", { ...thread.params, environmentId: "environment-b" }), + ); + current = apply(current, StackActions.push(thread.name, thread.params)); + const previous = current.routes; + if (replace) current = apply(current, StackActions.push("GitOverview", thread.params)); + await useOpenPullRequest({ replace })("https://github.com/owner/repository/pull/42"); + expect(current.routes.slice(0, -1)).toEqual(previous); + expect(current.routes.at(-1)).toMatchObject(pullRequest); + const back = useAndroidWorkspaceBack("pullRequests"); + if (replace) { + if (!hardwareBack?.()) navigation.goBack(); + } else back(); + expect(current.routes).toEqual(previous); + expect(selectTab).not.toHaveBeenCalled(); + }, + ); + + it("returns through detail history on Back but clears it on an explicit list switch", () => { + const previous = legacyHistory(); + current = apply( + previous, + StackActions.push("Thread", { ...thread.params, environmentId: "environment-b" }), + ); + useAndroidWorkspaceBack("threads")(); + expect(current.routes).toEqual(previous.routes); + expect(selectTab).not.toHaveBeenCalled(); + useWorkspaceListNavigation("threads")(); + expect(current.routes).toEqual([previous.routes[0]]); + expect(selectTab).toHaveBeenLastCalledWith({ type: "select", tab: "threads" }); + expect(navigation.canGoBack()).toBe(false); + }); +}); + +it.each([false, true])( + "returns to the PR diff after opening a thread (first send: %s)", + (draft) => { + current = legacyHistory(); + current = apply( + current, + StackActions.push("PullRequestFile", { ...pullRequest.params, path: "file.ts" }), + ); + const previous = current.routes; + current = apply( + current, + StackActions.push("Thread", draft ? { draft: { draftId: "pr-draft" } } : thread.params), + ); + if (draft) + current = apply(current, CommonActions.setParams({ draft: undefined, ...thread.params })); + const back = useAndroidWorkspaceBack("threads"); + if (draft) hardwareBack?.(); + else back(); + expect(current.routes).toEqual(previous); + expect(selectTab).not.toHaveBeenCalled(); + }, +); + +it("keeps chat history when a PR links to itself", async () => { + current = withThread(); + const chat = current.routes; + await useOpenPullRequest()("https://github.com/owner/repository/pull/42"); + const previous = current.routes; + await useOpenPullRequest()("https://github.com/owner/repository/pull/42"); + useAndroidWorkspaceBack("pullRequests")(); + expect(current.routes).toEqual(previous); + useAndroidWorkspaceBack("pullRequests")(); + expect(current.routes).toEqual(chat); + expect(selectTab).not.toHaveBeenCalled(); +}); diff --git a/apps/mobile/src/lib/android-workspace-navigation.ts b/apps/mobile/src/lib/android-workspace-navigation.ts index 3c2a995e41a68..accded2af1689 100644 --- a/apps/mobile/src/lib/android-workspace-navigation.ts +++ b/apps/mobile/src/lib/android-workspace-navigation.ts @@ -1,15 +1,16 @@ import type { NavigationState } from "@react-navigation/native"; import { resetRootStack, type RootStackNavigation } from "./root-stack-reset"; -function androidWorkspaceBackReturnsToThread(state: NavigationState) { +function androidWorkspaceBackReturnsToDetail(state: NavigationState) { const current = state.routes[state.index]; - const params = current?.params as ReactNavigation.RootParamList["Thread"] | undefined; - return ( - current?.name === "Thread" && - state.routes[state.index - 1]?.name === "Thread" && - params !== undefined && - !parentThreadIsBelow(state, params) - ); + const previous = state.routes[state.index - 1]; + if (current?.name !== "Thread" && current?.name !== "PullRequest") return false; + if (previous?.name === "PullRequest" || previous?.name === "PullRequestFile") return true; + if (previous?.name !== "Thread") return false; + if (current.name === "PullRequest") return true; + // Git actions can leave a duplicate Thread route that should return to its list. + const params = current.params as ReactNavigation.RootParamList["Thread"] | undefined; + return params !== undefined && !parentThreadIsBelow(state, params); } /** The root stack with only the workspace list. */ @@ -62,7 +63,7 @@ export function returnToAndroidWorkspaceList( selectTab: () => void, ) { const state = navigation.getState(); - if (state && androidWorkspaceBackReturnsToThread(state)) return navigation.goBack(); + if (state && androidWorkspaceBackReturnsToDetail(state)) return navigation.goBack(); selectTab(); resetAndroidWorkspace(navigation); } diff --git a/apps/mobile/src/native/useOpenPullRequest.ts b/apps/mobile/src/native/useOpenPullRequest.ts index 4ce5b4e876aa4..d514e5d3d2948 100644 --- a/apps/mobile/src/native/useOpenPullRequest.ts +++ b/apps/mobile/src/native/useOpenPullRequest.ts @@ -11,12 +11,17 @@ import { environmentServerConfigsAtom } from "../state/server"; type PullRequestContext = { readonly environmentId: string; readonly projectId: string }; -// Avoid useRoute so layouts without route context can open any thread's pull requests. -export function useNavigateToPullRequest(options?: { readonly replace?: boolean }) { +// Route-free so navigator layouts such as the workspace sidebar, which have no +// route context for useRoute, can open pull requests for any thread. +export function useNavigateToPullRequest(options?: { + readonly replace?: boolean; + readonly mode?: "contextual" | "workspace"; +}) { const navigation = useNavigation(); const selectDetail = useSelectWorkspaceDetail(); const projects = useProjects(); const replace = options?.replace; + const mode = options?.mode ?? "workspace"; return useCallback( async (url: string, context: PullRequestContext | null) => { // Read at press time so a capability push does not hand every link a new handler. @@ -28,18 +33,19 @@ export function useNavigateToPullRequest(options?: { readonly replace?: boolean (environmentId) => serverConfigs.get(environmentId)?.environment.capabilities, ); if (!params) return tryOpenExternalUrl(url, "pull-request"); - // Android shows one page at a time; iOS opens it over the current page, so Back returns. - if (Platform.OS === "android") selectDetail({ name: "PullRequest", params }); + if (Platform.OS === "android" && mode === "workspace") + selectDetail({ name: "PullRequest", params }); else if (replace) navigation.dispatch(StackActions.replace("PullRequest", params)); else navigation.dispatch(StackActions.push("PullRequest", params)); return true; }, - [navigation, projects, replace, selectDetail], + [navigation, projects, replace, mode, selectDetail], ); } export function useOpenPullRequest(options?: { readonly replace?: boolean; + readonly mode?: "contextual" | "workspace"; readonly context?: PullRequestContext; }) { const route = useRoute(); @@ -63,7 +69,10 @@ export function useOpenPullRequest(options?: { ); const projectId = options?.context?.projectId ?? first(routeContext?.projectId) ?? thread?.projectId; - const navigateToPullRequest = useNavigateToPullRequest({ replace: options?.replace }); + const navigateToPullRequest = useNavigateToPullRequest({ + replace: options?.replace, + mode: options?.mode ?? "contextual", + }); return useCallback( (url: string) => navigateToPullRequest(