authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-09-15 11:08:06-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-15 11:08:06-07:00
log3efdcc5296f1754e0f3bf7fee5fc2ada510e0438
tree23baf83cb20152340b6313122673ba36525d81aa
parent7235701de052658cd78cb029b0df71af4b8dc21f
signature Signed by PGP key B5690EEEBB952194

Preserve diff tree order and collapsed folders (#11931)

4 files changed, 107 insertions(+), 4 deletions(-)

apps/web/src/components/diffs/DiffFileTree.test.tsx+17
...@@ -178,6 +178,23 @@ describe("diff tree file activation", () => {...@@ -178,6 +178,23 @@ describe("diff tree file activation", () => {
178 expect(targets).toEqual([]);178 expect(targets).toEqual([]);
179 });179 });
180180
181 it("reorders refreshed files without reopening a collapsed folder", async () => {
182 const files: DiffFileTreeEntry[] = [
183 { path: "src/state/shell.ts", status: "modified" },
184 { path: "src/features/route.ts", status: "added" },
185 ];
186 await mount({ files });
187 const initialFolder = model().getItem("src/features/")!;
188 if (!("collapse" in initialFolder)) throw new Error("Expected the directory handle");
189 await act(async () => initialFolder.collapse());
190 await act(async () => {
191 renderer!.update(<Panel files={[files[1]!, files[0]!]} />);
192 });
193 const folder = model().getItem("src/features/")!;
194 if (!("isExpanded" in folder)) throw new Error("Expected the directory handle");
195 expect(folder.isExpanded()).toBe(false);
196 });
197
181 it("does not echo controlled selection, but lets the reader activate it", async () => {198 it("does not echo controlled selection, but lets the reader activate it", async () => {
182 await mount({ selectedPath: "02-short.ts" });199 await mount({ selectedPath: "02-short.ts" });
183 expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);200 expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);
apps/web/src/components/diffs/DiffFileTree.tsx+30-3
...@@ -1,7 +1,7 @@...@@ -1,7 +1,7 @@
1import type { GitStatusEntry } from "@pierre/trees";1import type { GitStatusEntry } from "@pierre/trees";
2import { FileTree, useFileTree, useFileTreeSelector } from "@pierre/trees/react";2import { FileTree, useFileTree, useFileTreeSelector } from "@pierre/trees/react";
3import { ChevronsDownUpIcon, ChevronsUpDownIcon } from "lucide-react";3import { ChevronsDownUpIcon, ChevronsUpDownIcon } from "lucide-react";
4import { useEffect, useMemo, useRef, type ReactNode } from "react";4import { useEffect, useMemo, useRef, useState, type ReactNode } from "react";
55
6import { useTheme } from "~/hooks/useTheme";6import { useTheme } from "~/hooks/useTheme";
7import { cn } from "~/lib/utils";7import { cn } from "~/lib/utils";
...@@ -13,7 +13,9 @@ import { Button } from "../ui/button";...@@ -13,7 +13,9 @@ import { Button } from "../ui/button";
13import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";13import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
14import {14import {
15 buildDiffFileTreeUpdates,15 buildDiffFileTreeUpdates,
16 compareDiffFileTreeEntries,
16 collectDirectoryPaths,17 collectDirectoryPaths,
18 diffFileTreePositions,
17 type DiffFileTreeEntry,19 type DiffFileTreeEntry,
18} from "./diffFileTree.logic";20} from "./diffFileTree.logic";
1921
...@@ -54,6 +56,16 @@ export function DiffFileTree({...@@ -54,6 +56,16 @@ export function DiffFileTree({
54 const { resolvedTheme } = useTheme();56 const { resolvedTheme } = useTheme();
55 const paths = useMemo(() => entries.map((entry) => entry.path), [entries]);57 const paths = useMemo(() => entries.map((entry) => entry.path), [entries]);
56 const directoryPaths = useMemo(() => collectDirectoryPaths(paths), [paths]);58 const directoryPaths = useMemo(() => collectDirectoryPaths(paths), [paths]);
59 const positions = useMemo(() => diffFileTreePositions(paths), [paths]);
60 const [ordering] = useState(() => {
61 let currentPositions: ReadonlyMap<string, number> = new Map();
62 return {
63 sort: compareDiffFileTreeEntries(() => currentPositions),
64 update: (nextPositions: ReadonlyMap<string, number>) => {
65 currentPositions = nextPositions;
66 },
67 };
68 });
57 const gitStatus = useMemo<ReadonlyArray<GitStatusEntry>>(69 const gitStatus = useMemo<ReadonlyArray<GitStatusEntry>>(
58 () => entries.map((entry) => ({ path: entry.path, status: entry.status })),70 () => entries.map((entry) => ({ path: entry.path, status: entry.status })),
59 [entries],71 [entries],
...@@ -83,6 +95,7 @@ export function DiffFileTree({...@@ -83,6 +95,7 @@ export function DiffFileTree({
83 },95 },
84 paths: [],96 paths: [],
85 search: false,97 search: false,
98 sort: ordering.sort,
86 unsafeCSS: PIERRE_TREE_UNSAFE_CSS,99 unsafeCSS: PIERRE_TREE_UNSAFE_CSS,
87 });100 });
88 const allDirectoriesExpanded = useFileTreeSelector(model, (currentModel) =>101 const allDirectoriesExpanded = useFileTreeSelector(model, (currentModel) =>
...@@ -90,17 +103,31 @@ export function DiffFileTree({...@@ -90,17 +103,31 @@ export function DiffFileTree({
90 );103 );
91104
92 useEffect(() => {105 useEffect(() => {
106 ordering.update(positions);
93 const mountedPaths = mountedPathsRef.current;107 const mountedPaths = mountedPathsRef.current;
94 if (mountedPaths === paths) return;108 if (mountedPaths === paths) return;
95 mountedPathsRef.current = paths;109 mountedPathsRef.current = paths;
96 if (mountedPaths === null) {110 if (mountedPaths === null) {
97 model.resetPaths(paths);111 model.resetPaths(paths);
98 } else {112 } else if (mountedPaths.every((path, index) => paths[index] === path)) {
113 // PR slices only append files, so keep the existing tree and its open folders.
99 const updates = buildDiffFileTreeUpdates(mountedPaths, paths);114 const updates = buildDiffFileTreeUpdates(mountedPaths, paths);
100 if (updates.length > 0) model.batch(updates);115 if (updates.length > 0) model.batch(updates);
116 } else {
117 // A refreshed diff can change the rank of existing siblings. Mutations do not reorder
118 // those rows, so rebuild while carrying the reader's folder expansion forward.
119 const collapsedDirectories = directoryPaths.filter((path) => {
120 const directory = model.getItem(path);
121 return directory !== null && "isExpanded" in directory && !directory.isExpanded();
122 });
123 model.resetPaths(paths);
124 for (const path of collapsedDirectories) {
125 const directory = model.getItem(path);
126 if (directory !== null && "collapse" in directory) directory.collapse();
127 }
101 }128 }
102 model.setGitStatus(gitStatus);129 model.setGitStatus(gitStatus);
103 }, [gitStatus, model, paths]);130 }, [directoryPaths, gitStatus, model, ordering, paths, positions]);
104131
105 useEffect(() => {132 useEffect(() => {
106 if (selectedPath === null) {133 if (selectedPath === null) {
apps/web/src/components/diffs/diffFileTree.logic.test.ts+31
...@@ -1,9 +1,12 @@...@@ -1,9 +1,12 @@
1import type { FileDiffMetadata } from "@pierre/diffs";1import type { FileDiffMetadata } from "@pierre/diffs";
2import { preloadFileTree } from "@pierre/trees";
2import { describe, expect, it } from "vite-plus/test";3import { describe, expect, it } from "vite-plus/test";
34
4import {5import {
5 buildDiffFileTreeUpdates,6 buildDiffFileTreeUpdates,
7 compareDiffFileTreeEntries,
6 collectDirectoryPaths,8 collectDirectoryPaths,
9 diffFileTreePositions,
7 diffFileTreeEntries,10 diffFileTreeEntries,
8} from "./diffFileTree.logic";11} from "./diffFileTree.logic";
912
...@@ -41,6 +44,34 @@ describe("collectDirectoryPaths", () => {...@@ -41,6 +44,34 @@ describe("collectDirectoryPaths", () => {
41 });44 });
42});45});
4346
47describe("diff tree reading order", () => {
48 it("places folders and files where their first diff appears", () => {
49 const paths = [
50 "apps/mobile/src/state/shell.ts",
51 "apps/mobile/src/features/threads/route.ts",
52 "apps/mobile/src/features/threads/screen.tsx",
53 ];
54 const positions = diffFileTreePositions(paths);
55 const tree = preloadFileTree({
56 paths,
57 initialExpansion: "open",
58 flattenEmptyDirectories: true,
59 sort: compareDiffFileTreeEntries(() => positions),
60 });
61 const rows = [...tree.shadowHtml.matchAll(/data-item-path="([^"]+)"/g)].map(
62 (match) => match[1],
63 );
64 expect(rows).toEqual([
65 "apps/mobile/src/",
66 "apps/mobile/src/state/",
67 "apps/mobile/src/state/shell.ts",
68 "apps/mobile/src/features/threads/",
69 "apps/mobile/src/features/threads/route.ts",
70 "apps/mobile/src/features/threads/screen.tsx",
71 ]);
72 });
73});
74
44describe("buildDiffFileTreeUpdates", () => {75describe("buildDiffFileTreeUpdates", () => {
45 it("adds a new file's directories before the file", () => {76 it("adds a new file's directories before the file", () => {
46 expect(buildDiffFileTreeUpdates(["README.md"], ["README.md", "src/lib/a.ts"])).toEqual([77 expect(buildDiffFileTreeUpdates(["README.md"], ["README.md", "src/lib/a.ts"])).toEqual([
apps/web/src/components/diffs/diffFileTree.logic.ts+29-1
...@@ -1,5 +1,5 @@...@@ -1,5 +1,5 @@
1import type { FileDiffMetadata } from "@pierre/diffs";1import type { FileDiffMetadata } from "@pierre/diffs";
2import type { FileTreeBatchOperation, GitStatus } from "@pierre/trees";2import type { FileTreeBatchOperation, FileTreeSortComparator, GitStatus } from "@pierre/trees";
33
4import { resolveFileDiffPath } from "~/lib/diffRendering";4import { resolveFileDiffPath } from "~/lib/diffRendering";
55
...@@ -47,6 +47,34 @@ export function collectDirectoryPaths(paths: ReadonlyArray<string>): ReadonlyArr...@@ -47,6 +47,34 @@ export function collectDirectoryPaths(paths: ReadonlyArray<string>): ReadonlyArr
47 return [...directories];47 return [...directories];
48}48}
4949
50/** A folder takes the position of its first file in the diff. */
51export function diffFileTreePositions(paths: ReadonlyArray<string>): ReadonlyMap<string, number> {
52 const positions = new Map<string, number>();
53 paths.forEach((path, index) => {
54 positions.set(path, index);
55 let directory = "";
56 for (const segment of path.split("/").slice(0, -1)) {
57 directory += `${segment}/`;
58 if (!positions.has(directory)) positions.set(directory, index);
59 }
60 });
61 return positions;
62}
63
64export function compareDiffFileTreeEntries(
65 getPositions: () => ReadonlyMap<string, number>,
66): FileTreeSortComparator {
67 return (left, right) => {
68 const positions = getPositions();
69 return (
70 (positions.get(left.path) ?? Number.MAX_SAFE_INTEGER) -
71 (positions.get(right.path) ?? Number.MAX_SAFE_INTEGER) ||
72 left.depth - right.depth ||
73 left.path.localeCompare(right.path)
74 );
75 };
76}
77
50function pathDepth(path: string): number {78function pathDepth(path: string): number {
51 return path.split("/").filter(Boolean).length;79 return path.split("/").filter(Boolean).length;
52}80}