Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/mobile/src/Stack.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,12 @@ export function workspaceLocationFromState(state: NavigationState) {
? ({ ...state, routes, index: routes.length - 1 } as NavigationState)
: state;
const path = getPathFromState(effectiveState, navigationPathConfig);
const route = effectiveState.routes[effectiveState.index];
return {
pathname: path.startsWith("/") ? path : `/${path}`,
routeKey: effectiveState.routes[effectiveState.index]?.key,
routeKey: route?.key,
routeName: route?.name,
routeKeys: effectiveState.routes.map((stackRoute) => stackRoute.key),
};
}

Expand Down Expand Up @@ -518,6 +521,8 @@ function RootStackLayout(props: {
<AdaptiveWorkspaceLayout
pathname={workspaceLocation.pathname}
workspaceRouteKey={workspaceLocation.routeKey}
workspaceRouteName={workspaceLocation.routeName}
workspaceRouteKeys={workspaceLocation.routeKeys}
selectedPullRequestKey={selectedPullRequestKey(props.state)}
>
{props.children}
Expand Down
12 changes: 2 additions & 10 deletions apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { resolveFilesystemReadAccess } from "@t3tools/client-runtime/state/files
import { environmentSession } from "../../state/session";
import { NativeStackScreenOptions } from "../../native/StackHeader";
import { StackActions, useNavigation, type StaticScreenProps } from "@react-navigation/native";
import { useEffect, useId, useRef, useState } from "react";
import { useId, useState } from "react";
import { Platform, View } from "react-native";
import { useSafeAreaInsets } from "react-native-safe-area-context";
import Svg, { Defs, LinearGradient, Rect, Stop } from "react-native-svg";
Expand Down Expand Up @@ -431,14 +431,13 @@ function FilesToolbarBottomFade() {
export function ThreadFilesTreeScreen(props: ThreadFilesRouteScreenProps) {
useAdaptiveWorkspacePaneRole("inspector");
const navigation = useNavigation();
const { fileInspector, layout, showAuxiliaryPane } = useAdaptiveWorkspaceLayout();
const { fileInspector, layout } = useAdaptiveWorkspaceLayout();
const [searchQuery, setSearchQuery] = useState("");
const { themeAppearance: highlightTheme } = useAppearancePreferences();
const { cwd, environmentId, projectName, selectedThread, threadId } = useThreadFilesWorkspace(
props.route.params,
);
const hasFileInspector = fileInspector.supported && threadId !== null;
const revealedInspectorRef = useRef(false);
const fileAccessSession = useEnvironmentQuery(
environmentId !== null ? environmentSession.sessionStateAtom(environmentId) : null,
);
Expand Down Expand Up @@ -511,13 +510,6 @@ export function ThreadFilesTreeScreen(props: ThreadFilesRouteScreenProps) {
theme: highlightTheme,
});
};
useEffect(() => {
if (hasFileInspector && cwd !== null && !revealedInspectorRef.current) {
revealedInspectorRef.current = true;
showAuxiliaryPane("inspector");
}
}, [cwd, hasFileInspector, showAuxiliaryPane]);

if (environmentId === null || (threadId !== null && selectedThread === null)) {
if (hasFileInspector) {
return (
Expand Down
94 changes: 49 additions & 45 deletions apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,7 @@ import { EnvironmentId, ThreadId, type SidebarProjectGroupingMode } from "@t3too
import { useAtomValue } from "@effect/atom-react";
import { useFocusEffect } from "@react-navigation/native";
import { NavigationContext, NavigationRouteContext, useNavigation } from "@react-navigation/native";
import {
use,
useDeferredValue,
useEffect,
useEffectEvent,
useMemo,
useRef,
useState,
type ReactNode,
} from "react";
import { use, useDeferredValue, useEffect, useMemo, useRef, useState, type ReactNode } from "react";
import { Keyboard, Platform, useWindowDimensions, View } from "react-native";
import { KeyboardController, useKeyboardState } from "react-native-keyboard-controller";
import Animated, {
Expand All @@ -45,6 +36,7 @@ import { scopedThreadKey } from "../../lib/scopedEntities";
import type { ThreadListFilters } from "../../persistence/mobile-preferences";
import { appAtomRegistry } from "../../state/atom-registry";
import { useProjects } from "../../state/entities";
import { useThreadCwd } from "../../state/use-selected-thread-worktree";
import { environmentThreadShells } from "../../state/threads";
import { mobilePreferencesAtom } from "../../state/preferences";
import {
Expand Down Expand Up @@ -124,46 +116,20 @@ export function useRegisterWorkspaceInspector(render: (() => ReactNode) | undefi
);
})();

const focusedRef = useRef(false);
const deactivateRef = useRef<(() => void) | null>(null);

const syncRegistration = useEffectEvent(() => {
if (!focusedRef.current || wrappedRender === undefined) {
deactivateRef.current?.();
return;
}
deactivateRef.current = registerWorkspaceInspector(wrappedRender);
});

useFocusEffect(() => {
focusedRef.current = true;
syncRegistration();
return () => {
focusedRef.current = false;
syncRegistration();
};
});

useEffect(() => {
if (focusedRef.current) {
syncRegistration();
}
// oxlint-disable-next-line react/exhaustive-effect-dependencies -- Changed pane content replaces its existing native workspace registration.
}, [wrappedRender]);

useEffect(
() => () => {
deactivateRef.current?.();
deactivateRef.current = null;
},
[],
// useFocusEffect re-runs when the renderer changes while focused, so the
// pane always shows this render's content, and its cleanup deactivates on
// blur and unmount.
useFocusEffect(() =>
wrappedRender === undefined ? undefined : registerWorkspaceInspector(wrappedRender),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);
}

export function AdaptiveWorkspaceLayout(props: {
readonly children: ReactNode;
readonly pathname: string;
readonly workspaceRouteKey: string | undefined;
readonly workspaceRouteName: string | undefined;
readonly workspaceRouteKeys: ReadonlyArray<string>;
readonly selectedPullRequestKey?: string | null;
}) {
const preferencesResult = useAtomValue(mobilePreferencesAtom);
Expand All @@ -190,6 +156,8 @@ function AdaptiveWorkspaceLayoutContent(
readonly children: ReactNode;
readonly pathname: string;
readonly workspaceRouteKey: string | undefined;
readonly workspaceRouteName: string | undefined;
readonly workspaceRouteKeys: ReadonlyArray<string>;
readonly selectedPullRequestKey?: string | null;
} & {
readonly projectGroupingMode: SidebarProjectGroupingMode;
Expand Down Expand Up @@ -344,16 +312,52 @@ function AdaptiveWorkspaceLayoutContent(
const activeThread = parseActiveThreadPath(pathname);
const environmentId = activeThread?.environmentId ?? null;
const threadId = activeThread?.threadId ?? null;
const selectedThreadKey = (() => {
const selectedThreadRef = (() => {
if (environmentId === null || threadId === null) {
return null;
}
try {
return scopedThreadKey(EnvironmentId.make(environmentId), ThreadId.make(threadId));
return {
environmentId: EnvironmentId.make(environmentId),
threadId: ThreadId.make(threadId),
};
} catch {
return null;
}
})();
const selectedThreadKey =
selectedThreadRef === null
? null
: scopedThreadKey(selectedThreadRef.environmentId, selectedThreadRef.threadId);
const isFilesRoute = props.workspaceRouteName === "ThreadFiles";
// Same resolution the Files screen uses: without a cwd it shows "Files unavailable" and
// registers no inspector, so revealing would only hide the sidebar beside that screen.
const filesCwd = useThreadCwd(isFilesRoute && fileInspector.supported ? selectedThreadRef : null);
Comment thread
juliusmarminge marked this conversation as resolved.
// Opening a review or the files route reveals its inspector once per route and thread.
// Routes still on the stack remember their reveal, so going back to one keeps the user's choice.
const revealsInspector =
props.workspaceRouteName === "ThreadReview" || (isFilesRoute && filesCwd !== null);
const [revealedInspectors, setRevealedInspectors] = useState<ReadonlyMap<string, string>>(
() => new Map(),
);
if (
revealsInspector &&
props.workspaceRouteKey !== undefined &&
selectedThreadKey !== null &&
revealedInspectors.get(props.workspaceRouteKey) !== selectedThreadKey
) {
const next = new Map<string, string>();
for (const routeKey of props.workspaceRouteKeys) {
const revealedThreadKey = revealedInspectors.get(routeKey);
if (revealedThreadKey !== undefined) {
next.set(routeKey, revealedThreadKey);
}
}
next.set(props.workspaceRouteKey, selectedThreadKey);
setRevealedInspectors(next);
setFocusedAuxiliaryPaneRole("inspector");
setFileInspectorPreferredVisible(true);
}
const [workspaceInspector, setWorkspaceInspector] = useState<{
readonly render: () => ReactNode;
readonly active: boolean;
Expand Down
15 changes: 2 additions & 13 deletions apps/mobile/src/features/review/ReviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@ import type { AppBarMenuItem } from "../../components/ui/AppBar.types";
import type { ReviewSectionItem } from "./reviewModel";
import { useReviewHeaderPresentation } from "./useReviewHeaderPresentation";
import { Screen, ScreenStack, ScreenStackHeaderConfig } from "react-native-screens";
import {
type Ref,
type ReactElement,
useEffect,
useImperativeHandle,
useRef,
useState,
} from "react";
import { type Ref, type ReactElement, useImperativeHandle, useRef, useState } from "react";
import {
FlatList,
Platform,
Expand Down Expand Up @@ -432,7 +425,7 @@ type ReviewSheetProps = StaticScreenProps<{
export function ReviewSheet(props: ReviewSheetProps) {
const { nativeReviewDiffStyle } = useAppearanceCodeSurface();
useAdaptiveWorkspacePaneRole("inspector");
const { panes, showAuxiliaryPane } = useAdaptiveWorkspaceLayout();
const { panes } = useAdaptiveWorkspaceLayout();
const navigation = useNavigation();
const selectDetail = useSelectWorkspaceDetail();
const insets = useSafeAreaInsets();
Expand All @@ -451,10 +444,6 @@ export function ReviewSheet(props: ReviewSheetProps) {
const bottomContentInset =
Platform.OS === "ios" ? safeArea.bottom + 8 : Math.max(insets.bottom, 18) + 18;

useEffect(() => {
showAuxiliaryPane("inspector");
// oxlint-disable-next-line react/exhaustive-effect-dependencies -- Opening review for another thread reselects its inspector pane.
}, [environmentId, showAuxiliaryPane, threadId]);
const {
error,
reviewSections,
Expand Down
10 changes: 10 additions & 0 deletions apps/mobile/src/reactCompilerCoverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ describe("React Compiler coverage", () => {
).toEqual([true]);
});

// An uncached registration would re-register the workspace inspector on every render.
it("registers the workspace inspector only when its renderer changes", () => {
expect(
compilerCachedCalls(
"src/features/layout/AdaptiveWorkspaceLayout.tsx",
"registerWorkspaceInspector",
),
).toEqual([true]);
});

it("lists only files that exist", () => {
expect(Object.keys(KNOWN_BAILOUTS).filter((file) => !sourceFiles.includes(file))).toEqual([]);
});
Expand Down
55 changes: 55 additions & 0 deletions apps/mobile/src/state/use-selected-thread-worktree.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { EnvironmentId, ProjectId, ThreadId } from "@t3tools/contracts";
import { describe, expect, it, vi } from "vite-plus/test";

const state = vi.hoisted(() => ({
shell: null as { projectId: string; worktreePath: string | null } | null,
detail: null as { thread: { projectId: string } } | null,
detailWorktreePath: null as string | null,
projects: new Map<string, { workspaceRoot: string }>(),
}));

vi.mock("./entities", () => ({
useQuietThreadShell: (ref: unknown) => (ref === null ? null : state.shell),
useProject: (ref: { projectId: string } | null) =>
ref === null ? null : (state.projects.get(ref.projectId) ?? null),
}));
vi.mock("./threads", async () => {
const { some, none } = await import("effect/Option");
return {
useEnvironmentThread: (environmentId: unknown, threadId: unknown) => ({
data:
environmentId !== null && threadId !== null && state.detail ? some(state.detail) : none(),
}),
};
});
vi.mock("./use-thread-detail", () => ({
useThreadWorktreePath: (ref: unknown) => (ref === null ? null : state.detailWorktreePath),
useSelectedThreadWorktreePath: () => null,
}));
vi.mock("./use-thread-selection", () => ({ useThreadSelection: () => ({}) }));

const { useThreadCwd } = await import("./use-selected-thread-worktree");

const ref = { environmentId: EnvironmentId.make("env"), threadId: ThreadId.make("thread") };

function setup(input: Partial<Omit<typeof state, "projects">>) {
Object.assign(state, { shell: null, detail: null, detailWorktreePath: null }, input);
state.projects = new Map([[ProjectId.make("project"), { workspaceRoot: "/repo" }]]);
}

describe("useThreadCwd", () => {
it("falls back to the project root from the loaded detail when the shell is missing", () => {
setup({ detail: { thread: { projectId: "project" } } });
expect(useThreadCwd(ref)).toBe("/repo");
});

it("prefers the thread worktree over the project root", () => {
setup({ shell: { projectId: "project", worktreePath: "/repo/.worktrees/a" } });
expect(useThreadCwd(ref)).toBe("/repo/.worktrees/a");
});

it("is null without a thread", () => {
setup({ detail: { thread: { projectId: "project" } } });
expect(useThreadCwd(null)).toBeNull();
});
});
32 changes: 31 additions & 1 deletion apps/mobile/src/state/use-selected-thread-worktree.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import type { ScopedThreadRef } from "@t3tools/contracts";
import * as Option from "effect/Option";
import { useMemo } from "react";

import { useSelectedThreadWorktreePath } from "./use-thread-detail";
import { useProject, useQuietThreadShell } from "./entities";
import { useEnvironmentThread } from "./threads";
import { useSelectedThreadWorktreePath, useThreadWorktreePath } from "./use-thread-detail";
import { useThreadSelection } from "./use-thread-selection";
import { resolvePreferredThreadWorktreePath } from "../features/terminal/terminalLaunchContext";

Expand All @@ -22,3 +26,29 @@ export function useSelectedThreadWorktree() {
selectedThreadCwd: selectedThreadWorktreePath ?? selectedThreadProject?.workspaceRoot ?? null,
};
}

/**
* Resolves a thread's cwd the way the selected thread's does, for a thread that is not the
* current route's: the shell, or the loaded detail when the shell is missing, picks the project.
*/
export function useThreadCwd(ref: ScopedThreadRef | null): string | null {
const shell = useQuietThreadShell(ref);
const detailState = useEnvironmentThread(
shell === null ? (ref?.environmentId ?? null) : null,
shell === null ? (ref?.threadId ?? null) : null,
);
const detailThread = Option.getOrNull(detailState.data)?.thread ?? null;
const detailWorktreePath = useThreadWorktreePath(ref);
const projectId = shell?.projectId ?? detailThread?.projectId ?? null;
const project = useProject(
ref === null || projectId === null ? null : { environmentId: ref.environmentId, projectId },
);
return (
resolvePreferredThreadWorktreePath({
threadShellWorktreePath: shell?.worktreePath ?? null,
threadDetailWorktreePath: detailWorktreePath,
}) ??
project?.workspaceRoot ??
null
);
}
12 changes: 7 additions & 5 deletions apps/mobile/src/state/use-thread-detail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,17 @@ export function useSelectedThreadVisibleTurnItems(): OrchestrationV2ThreadProjec
const EMPTY_WORKTREE_PATH_ATOM = Atom.make<string | null>(null);
const EMPTY_PENDING_REQUESTS_ATOM = Atom.make<PendingThreadRequests | null>(null);

export function useSelectedThreadWorktreePath() {
const { selectedThreadDetailRef } = useThreadSelection();
export function useThreadWorktreePath(ref: ScopedThreadRef | null) {
return useAtomValue(
selectedThreadDetailRef === null
? EMPTY_WORKTREE_PATH_ATOM
: environmentThreadDetails.worktreePathAtom(selectedThreadDetailRef),
ref === null ? EMPTY_WORKTREE_PATH_ATOM : environmentThreadDetails.worktreePathAtom(ref),
);
}

export function useSelectedThreadWorktreePath() {
const { selectedThreadDetailRef } = useThreadSelection();
return useThreadWorktreePath(selectedThreadDetailRef);
}

export function useSelectedThreadPendingRequests() {
const { selectedThreadDetailRef } = useThreadSelection();
return useAtomValue(
Expand Down
Loading