Skip to content

Commit 28286d5

Browse files
refactor(mobile): opening Review or Files shows the inspector in the first commit (#25817)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 737eab2 commit 28286d5

8 files changed

Lines changed: 162 additions & 75 deletions

‎apps/mobile/src/Stack.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,9 +482,12 @@ export function workspaceLocationFromState(state: NavigationState) {
482482
? ({ ...state, routes, index: routes.length - 1 } as NavigationState)
483483
: state;
484484
const path = getPathFromState(effectiveState, navigationPathConfig);
485+
const route = effectiveState.routes[effectiveState.index];
485486
return {
486487
pathname: path.startsWith("/") ? path : `/${path}`,
487-
routeKey: effectiveState.routes[effectiveState.index]?.key,
488+
routeKey: route?.key,
489+
routeName: route?.name,
490+
routeKeys: effectiveState.routes.map((stackRoute) => stackRoute.key),
488491
};
489492
}
490493

@@ -518,6 +521,8 @@ function RootStackLayout(props: {
518521
<AdaptiveWorkspaceLayout
519522
pathname={workspaceLocation.pathname}
520523
workspaceRouteKey={workspaceLocation.routeKey}
524+
workspaceRouteName={workspaceLocation.routeName}
525+
workspaceRouteKeys={workspaceLocation.routeKeys}
521526
selectedPullRequestKey={selectedPullRequestKey(props.state)}
522527
>
523528
{props.children}

‎apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { resolveFilesystemReadAccess } from "@t3tools/client-runtime/state/files
44
import { environmentSession } from "../../state/session";
55
import { NativeStackScreenOptions } from "../../native/StackHeader";
66
import { StackActions, useNavigation, type StaticScreenProps } from "@react-navigation/native";
7-
import { useEffect, useId, useRef, useState } from "react";
7+
import { useId, useState } from "react";
88
import { Platform, View } from "react-native";
99
import { useSafeAreaInsets } from "react-native-safe-area-context";
1010
import Svg, { Defs, LinearGradient, Rect, Stop } from "react-native-svg";
@@ -431,14 +431,13 @@ function FilesToolbarBottomFade() {
431431
export function ThreadFilesTreeScreen(props: ThreadFilesRouteScreenProps) {
432432
useAdaptiveWorkspacePaneRole("inspector");
433433
const navigation = useNavigation();
434-
const { fileInspector, layout, showAuxiliaryPane } = useAdaptiveWorkspaceLayout();
434+
const { fileInspector, layout } = useAdaptiveWorkspaceLayout();
435435
const [searchQuery, setSearchQuery] = useState("");
436436
const { themeAppearance: highlightTheme } = useAppearancePreferences();
437437
const { cwd, environmentId, projectName, selectedThread, threadId } = useThreadFilesWorkspace(
438438
props.route.params,
439439
);
440440
const hasFileInspector = fileInspector.supported && threadId !== null;
441-
const revealedInspectorRef = useRef(false);
442441
const fileAccessSession = useEnvironmentQuery(
443442
environmentId !== null ? environmentSession.sessionStateAtom(environmentId) : null,
444443
);
@@ -511,13 +510,6 @@ export function ThreadFilesTreeScreen(props: ThreadFilesRouteScreenProps) {
511510
theme: highlightTheme,
512511
});
513512
};
514-
useEffect(() => {
515-
if (hasFileInspector && cwd !== null && !revealedInspectorRef.current) {
516-
revealedInspectorRef.current = true;
517-
showAuxiliaryPane("inspector");
518-
}
519-
}, [cwd, hasFileInspector, showAuxiliaryPane]);
520-
521513
if (environmentId === null || (threadId !== null && selectedThread === null)) {
522514
if (hasFileInspector) {
523515
return (

‎apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx‎

Lines changed: 49 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,7 @@ import { EnvironmentId, ThreadId, type SidebarProjectGroupingMode } from "@t3too
1010
import { useAtomValue } from "@effect/atom-react";
1111
import { useFocusEffect } from "@react-navigation/native";
1212
import { NavigationContext, NavigationRouteContext, useNavigation } from "@react-navigation/native";
13-
import {
14-
use,
15-
useDeferredValue,
16-
useEffect,
17-
useEffectEvent,
18-
useMemo,
19-
useRef,
20-
useState,
21-
type ReactNode,
22-
} from "react";
13+
import { use, useDeferredValue, useEffect, useMemo, useRef, useState, type ReactNode } from "react";
2314
import { Keyboard, Platform, useWindowDimensions, View } from "react-native";
2415
import { KeyboardController, useKeyboardState } from "react-native-keyboard-controller";
2516
import Animated, {
@@ -45,6 +36,7 @@ import { scopedThreadKey } from "../../lib/scopedEntities";
4536
import type { ThreadListFilters } from "../../persistence/mobile-preferences";
4637
import { appAtomRegistry } from "../../state/atom-registry";
4738
import { useProjects } from "../../state/entities";
39+
import { useThreadCwd } from "../../state/use-selected-thread-worktree";
4840
import { environmentThreadShells } from "../../state/threads";
4941
import { mobilePreferencesAtom } from "../../state/preferences";
5042
import {
@@ -128,46 +120,20 @@ export function useRegisterWorkspaceInspector(render: (() => ReactNode) | undefi
128120
);
129121
})();
130122

131-
const focusedRef = useRef(false);
132-
const deactivateRef = useRef<(() => void) | null>(null);
133-
134-
const syncRegistration = useEffectEvent(() => {
135-
if (!focusedRef.current || wrappedRender === undefined) {
136-
deactivateRef.current?.();
137-
return;
138-
}
139-
deactivateRef.current = registerWorkspaceInspector(wrappedRender);
140-
});
141-
142-
useFocusEffect(() => {
143-
focusedRef.current = true;
144-
syncRegistration();
145-
return () => {
146-
focusedRef.current = false;
147-
syncRegistration();
148-
};
149-
});
150-
151-
useEffect(() => {
152-
if (focusedRef.current) {
153-
syncRegistration();
154-
}
155-
// oxlint-disable-next-line react/exhaustive-effect-dependencies -- Changed pane content replaces its existing native workspace registration.
156-
}, [wrappedRender]);
157-
158-
useEffect(
159-
() => () => {
160-
deactivateRef.current?.();
161-
deactivateRef.current = null;
162-
},
163-
[],
123+
// useFocusEffect re-runs when the renderer changes while focused, so the
124+
// pane always shows this render's content, and its cleanup deactivates on
125+
// blur and unmount.
126+
useFocusEffect(() =>
127+
wrappedRender === undefined ? undefined : registerWorkspaceInspector(wrappedRender),
164128
);
165129
}
166130

167131
export function AdaptiveWorkspaceLayout(props: {
168132
readonly children: ReactNode;
169133
readonly pathname: string;
170134
readonly workspaceRouteKey: string | undefined;
135+
readonly workspaceRouteName: string | undefined;
136+
readonly workspaceRouteKeys: ReadonlyArray<string>;
171137
readonly selectedPullRequestKey?: string | null;
172138
}) {
173139
const preferencesResult = useAtomValue(mobilePreferencesAtom);
@@ -194,6 +160,8 @@ function AdaptiveWorkspaceLayoutContent(
194160
readonly children: ReactNode;
195161
readonly pathname: string;
196162
readonly workspaceRouteKey: string | undefined;
163+
readonly workspaceRouteName: string | undefined;
164+
readonly workspaceRouteKeys: ReadonlyArray<string>;
197165
readonly selectedPullRequestKey?: string | null;
198166
} & {
199167
readonly projectGroupingMode: SidebarProjectGroupingMode;
@@ -348,16 +316,52 @@ function AdaptiveWorkspaceLayoutContent(
348316
const activeThread = parseActiveThreadPath(pathname);
349317
const environmentId = activeThread?.environmentId ?? null;
350318
const threadId = activeThread?.threadId ?? null;
351-
const selectedThreadKey = (() => {
319+
const selectedThreadRef = (() => {
352320
if (environmentId === null || threadId === null) {
353321
return null;
354322
}
355323
try {
356-
return scopedThreadKey(EnvironmentId.make(environmentId), ThreadId.make(threadId));
324+
return {
325+
environmentId: EnvironmentId.make(environmentId),
326+
threadId: ThreadId.make(threadId),
327+
};
357328
} catch {
358329
return null;
359330
}
360331
})();
332+
const selectedThreadKey =
333+
selectedThreadRef === null
334+
? null
335+
: scopedThreadKey(selectedThreadRef.environmentId, selectedThreadRef.threadId);
336+
const isFilesRoute = props.workspaceRouteName === "ThreadFiles";
337+
// Same resolution the Files screen uses: without a cwd it shows "Files unavailable" and
338+
// registers no inspector, so revealing would only hide the sidebar beside that screen.
339+
const filesCwd = useThreadCwd(isFilesRoute && fileInspector.supported ? selectedThreadRef : null);
340+
// Opening a review or the files route reveals its inspector once per route and thread.
341+
// Routes still on the stack remember their reveal, so going back to one keeps the user's choice.
342+
const revealsInspector =
343+
props.workspaceRouteName === "ThreadReview" || (isFilesRoute && filesCwd !== null);
344+
const [revealedInspectors, setRevealedInspectors] = useState<ReadonlyMap<string, string>>(
345+
() => new Map(),
346+
);
347+
if (
348+
revealsInspector &&
349+
props.workspaceRouteKey !== undefined &&
350+
selectedThreadKey !== null &&
351+
revealedInspectors.get(props.workspaceRouteKey) !== selectedThreadKey
352+
) {
353+
const next = new Map<string, string>();
354+
for (const routeKey of props.workspaceRouteKeys) {
355+
const revealedThreadKey = revealedInspectors.get(routeKey);
356+
if (revealedThreadKey !== undefined) {
357+
next.set(routeKey, revealedThreadKey);
358+
}
359+
}
360+
next.set(props.workspaceRouteKey, selectedThreadKey);
361+
setRevealedInspectors(next);
362+
setFocusedAuxiliaryPaneRole("inspector");
363+
setFileInspectorPreferredVisible(true);
364+
}
361365
const [workspaceInspector, setWorkspaceInspector] = useState<{
362366
readonly render: () => ReactNode;
363367
readonly active: boolean;

‎apps/mobile/src/features/review/ReviewSheet.tsx‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,7 @@ import type { AppBarMenuItem } from "../../components/ui/AppBar.types";
1010
import type { ReviewSectionItem } from "./reviewModel";
1111
import { useReviewHeaderPresentation } from "./useReviewHeaderPresentation";
1212
import { Screen, ScreenStack, ScreenStackHeaderConfig } from "react-native-screens";
13-
import {
14-
type Ref,
15-
type ReactElement,
16-
useEffect,
17-
useImperativeHandle,
18-
useRef,
19-
useState,
20-
} from "react";
13+
import { type Ref, type ReactElement, useImperativeHandle, useRef, useState } from "react";
2114
import {
2215
FlatList,
2316
Platform,
@@ -432,7 +425,7 @@ type ReviewSheetProps = StaticScreenProps<{
432425
export function ReviewSheet(props: ReviewSheetProps) {
433426
const { nativeReviewDiffStyle } = useAppearanceCodeSurface();
434427
useAdaptiveWorkspacePaneRole("inspector");
435-
const { panes, showAuxiliaryPane } = useAdaptiveWorkspaceLayout();
428+
const { panes } = useAdaptiveWorkspaceLayout();
436429
const navigation = useNavigation();
437430
const selectDetail = useSelectWorkspaceDetail();
438431
const insets = useSafeAreaInsets();
@@ -451,10 +444,6 @@ export function ReviewSheet(props: ReviewSheetProps) {
451444
const bottomContentInset =
452445
Platform.OS === "ios" ? safeArea.bottom + 8 : Math.max(insets.bottom, 18) + 18;
453446

454-
useEffect(() => {
455-
showAuxiliaryPane("inspector");
456-
// oxlint-disable-next-line react/exhaustive-effect-dependencies -- Opening review for another thread reselects its inspector pane.
457-
}, [environmentId, showAuxiliaryPane, threadId]);
458447
const {
459448
error,
460449
reviewSections,

‎apps/mobile/src/reactCompilerCoverage.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,16 @@ describe("React Compiler coverage", () => {
182182
).toEqual([true]);
183183
});
184184

185+
// An uncached registration would re-register the workspace inspector on every render.
186+
it("registers the workspace inspector only when its renderer changes", () => {
187+
expect(
188+
compilerCachedCalls(
189+
"src/features/layout/AdaptiveWorkspaceLayout.tsx",
190+
"registerWorkspaceInspector",
191+
),
192+
).toEqual([true]);
193+
});
194+
185195
it("lists only files that exist", () => {
186196
expect(Object.keys(KNOWN_BAILOUTS).filter((file) => !sourceFiles.includes(file))).toEqual([]);
187197
});
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { EnvironmentId, ProjectId, ThreadId } from "@t3tools/contracts";
2+
import { describe, expect, it, vi } from "vite-plus/test";
3+
4+
const state = vi.hoisted(() => ({
5+
shell: null as { projectId: string; worktreePath: string | null } | null,
6+
detail: null as { thread: { projectId: string } } | null,
7+
detailWorktreePath: null as string | null,
8+
projects: new Map<string, { workspaceRoot: string }>(),
9+
}));
10+
11+
vi.mock("./entities", () => ({
12+
useQuietThreadShell: (ref: unknown) => (ref === null ? null : state.shell),
13+
useProject: (ref: { projectId: string } | null) =>
14+
ref === null ? null : (state.projects.get(ref.projectId) ?? null),
15+
}));
16+
vi.mock("./threads", async () => {
17+
const { some, none } = await import("effect/Option");
18+
return {
19+
useEnvironmentThread: (environmentId: unknown, threadId: unknown) => ({
20+
data:
21+
environmentId !== null && threadId !== null && state.detail ? some(state.detail) : none(),
22+
}),
23+
};
24+
});
25+
vi.mock("./use-thread-detail", () => ({
26+
useThreadWorktreePath: (ref: unknown) => (ref === null ? null : state.detailWorktreePath),
27+
useSelectedThreadWorktreePath: () => null,
28+
}));
29+
vi.mock("./use-thread-selection", () => ({ useThreadSelection: () => ({}) }));
30+
31+
const { useThreadCwd } = await import("./use-selected-thread-worktree");
32+
33+
const ref = { environmentId: EnvironmentId.make("env"), threadId: ThreadId.make("thread") };
34+
35+
function setup(input: Partial<Omit<typeof state, "projects">>) {
36+
Object.assign(state, { shell: null, detail: null, detailWorktreePath: null }, input);
37+
state.projects = new Map([[ProjectId.make("project"), { workspaceRoot: "/repo" }]]);
38+
}
39+
40+
describe("useThreadCwd", () => {
41+
it("falls back to the project root from the loaded detail when the shell is missing", () => {
42+
setup({ detail: { thread: { projectId: "project" } } });
43+
expect(useThreadCwd(ref)).toBe("/repo");
44+
});
45+
46+
it("prefers the thread worktree over the project root", () => {
47+
setup({ shell: { projectId: "project", worktreePath: "/repo/.worktrees/a" } });
48+
expect(useThreadCwd(ref)).toBe("/repo/.worktrees/a");
49+
});
50+
51+
it("is null without a thread", () => {
52+
setup({ detail: { thread: { projectId: "project" } } });
53+
expect(useThreadCwd(null)).toBeNull();
54+
});
55+
});

‎apps/mobile/src/state/use-selected-thread-worktree.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1+
import type { ScopedThreadRef } from "@t3tools/contracts";
2+
import * as Option from "effect/Option";
13
import { useMemo } from "react";
24

3-
import { useSelectedThreadWorktreePath } from "./use-thread-detail";
5+
import { useProject, useQuietThreadShell } from "./entities";
6+
import { useEnvironmentThread } from "./threads";
7+
import { useSelectedThreadWorktreePath, useThreadWorktreePath } from "./use-thread-detail";
48
import { useThreadSelection } from "./use-thread-selection";
59
import { resolvePreferredThreadWorktreePath } from "../features/terminal/terminalLaunchContext";
610

@@ -22,3 +26,29 @@ export function useSelectedThreadWorktree() {
2226
selectedThreadCwd: selectedThreadWorktreePath ?? selectedThreadProject?.workspaceRoot ?? null,
2327
};
2428
}
29+
30+
/**
31+
* Resolves a thread's cwd the way the selected thread's does, for a thread that is not the
32+
* current route's: the shell, or the loaded detail when the shell is missing, picks the project.
33+
*/
34+
export function useThreadCwd(ref: ScopedThreadRef | null): string | null {
35+
const shell = useQuietThreadShell(ref);
36+
const detailState = useEnvironmentThread(
37+
shell === null ? (ref?.environmentId ?? null) : null,
38+
shell === null ? (ref?.threadId ?? null) : null,
39+
);
40+
const detailThread = Option.getOrNull(detailState.data)?.thread ?? null;
41+
const detailWorktreePath = useThreadWorktreePath(ref);
42+
const projectId = shell?.projectId ?? detailThread?.projectId ?? null;
43+
const project = useProject(
44+
ref === null || projectId === null ? null : { environmentId: ref.environmentId, projectId },
45+
);
46+
return (
47+
resolvePreferredThreadWorktreePath({
48+
threadShellWorktreePath: shell?.worktreePath ?? null,
49+
threadDetailWorktreePath: detailWorktreePath,
50+
}) ??
51+
project?.workspaceRoot ??
52+
null
53+
);
54+
}

‎apps/mobile/src/state/use-thread-detail.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,17 @@ export function useSelectedThreadVisibleTurnItems(): OrchestrationV2ThreadProjec
9292
const EMPTY_WORKTREE_PATH_ATOM = Atom.make<string | null>(null);
9393
const EMPTY_PENDING_REQUESTS_ATOM = Atom.make<PendingThreadRequests | null>(null);
9494

95-
export function useSelectedThreadWorktreePath() {
96-
const { selectedThreadDetailRef } = useThreadSelection();
95+
export function useThreadWorktreePath(ref: ScopedThreadRef | null) {
9796
return useAtomValue(
98-
selectedThreadDetailRef === null
99-
? EMPTY_WORKTREE_PATH_ATOM
100-
: environmentThreadDetails.worktreePathAtom(selectedThreadDetailRef),
97+
ref === null ? EMPTY_WORKTREE_PATH_ATOM : environmentThreadDetails.worktreePathAtom(ref),
10198
);
10299
}
103100

101+
export function useSelectedThreadWorktreePath() {
102+
const { selectedThreadDetailRef } = useThreadSelection();
103+
return useThreadWorktreePath(selectedThreadDetailRef);
104+
}
105+
104106
export function useSelectedThreadPendingRequests() {
105107
const { selectedThreadDetailRef } = useThreadSelection();
106108
return useAtomValue(

0 commit comments

Comments
 (0)