Skip to content

Commit 62841b3

Browse files
juliusmarmingejuliusmarminge
committed
fix(web): reveal reselected diff files
1 parent e5a87e8 commit 62841b3

2 files changed

Lines changed: 216 additions & 0 deletions

File tree

‎apps/web/src/components/diffs/DiffFileTree.test.tsx‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import type { CodeViewScrollTarget } from "@pierre/diffs";
2+
import type { FileTree as FileTreeModel } from "@pierre/trees";
3+
import { FileTree } from "@pierre/trees/react";
4+
import { act, type MouseEvent, type ReactNode } from "react";
5+
import { create, type ReactTestRenderer } from "react-test-renderer";
6+
import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test";
7+
8+
import { DiffFileTree, type DiffFileTreeEntry } from "./DiffFileTree";
9+
import { useCodeViewFileReveal } from "./useCodeViewFileReveal";
10+
11+
vi.mock("../../hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) }));
12+
// Tooltip positioning is unrelated to the tree's actual model and activation path.
13+
vi.mock("../ui/tooltip", () => ({
14+
Tooltip: ({ children }: { children: ReactNode }) => children,
15+
TooltipTrigger: ({ render }: { render: ReactNode }) => render,
16+
TooltipPopup: () => null,
17+
}));
18+
19+
const entries: DiffFileTreeEntry[] = [
20+
{ path: "01-tall.ts", status: "modified" },
21+
{ path: "02-short.ts", status: "modified" },
22+
{ path: "03-medium.ts", status: "modified" },
23+
];
24+
25+
class TreeRow {
26+
constructor(readonly path: string) {}
27+
28+
getAttribute(name: string) {
29+
return name === "data-item-path" ? this.path : null;
30+
}
31+
}
32+
33+
describe("diff tree file activation", () => {
34+
let renderer: ReactTestRenderer | undefined;
35+
const targets: CodeViewScrollTarget[] = [];
36+
const viewer = {
37+
getInstance: () => viewer,
38+
scrollTo: (target: CodeViewScrollTarget) => targets.push(target),
39+
};
40+
41+
function Panel({
42+
files = entries,
43+
selectedPath = null,
44+
}: {
45+
files?: DiffFileTreeEntry[];
46+
selectedPath?: string | null;
47+
}) {
48+
const reveal = useCodeViewFileReveal(viewer, "working-tree");
49+
return (
50+
<DiffFileTree
51+
entries={files}
52+
ariaLabel="Working tree files"
53+
selectedPath={selectedPath}
54+
onSelectFile={(path) => reveal(`${path}\0${path}`)}
55+
/>
56+
);
57+
}
58+
59+
const model = (): FileTreeModel => renderer!.root.findByType(FileTree).props.model;
60+
61+
async function mount(props: Parameters<typeof Panel>[0] = {}) {
62+
await act(async () => {
63+
renderer = create(<Panel {...props} />);
64+
});
65+
}
66+
67+
// Exercise T3's capture handler before the real Pierre model's selection transition.
68+
// Only DOM hit testing is represented here; native pointer/keyboard dispatch and diff
69+
// geometry are verified separately in the integrated client.
70+
async function activate(path: string, modifiers: Partial<MouseEvent<HTMLElement>> = {}) {
71+
const event = {
72+
button: 0,
73+
ctrlKey: false,
74+
metaKey: false,
75+
shiftKey: false,
76+
altKey: false,
77+
defaultPrevented: false,
78+
nativeEvent: { composedPath: () => [{}, new TreeRow(path), {}] },
79+
...modifiers,
80+
} as MouseEvent<HTMLElement>;
81+
await act(async () => {
82+
renderer!.root
83+
.find((node) => String(node.type) === "file-tree-container")
84+
.props.onClickCapture?.(event);
85+
const tree = model();
86+
const item = tree.getItem(path)!;
87+
if (event.ctrlKey || event.metaKey) {
88+
item.toggleSelect();
89+
} else {
90+
for (const selected of tree.getSelectedPaths()) {
91+
if (selected !== path) tree.getItem(selected)?.deselect();
92+
}
93+
item.select();
94+
}
95+
item.focus();
96+
if ("toggle" in item && !event.ctrlKey && !event.metaKey && !event.shiftKey) item.toggle();
97+
});
98+
}
99+
100+
beforeEach(() => {
101+
targets.length = 0;
102+
vi.useFakeTimers();
103+
vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
104+
vi.stubGlobal("HTMLElement", TreeRow);
105+
});
106+
107+
afterEach(async () => {
108+
await act(async () => renderer?.unmount());
109+
renderer = undefined;
110+
vi.runOnlyPendingTimers();
111+
vi.useRealTimers();
112+
vi.unstubAllGlobals();
113+
});
114+
115+
it("reissues the reveal when the sole selected file is activated again", async () => {
116+
await mount();
117+
await activate("02-short.ts");
118+
expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);
119+
await activate("02-short.ts");
120+
expect(targets).toEqual([
121+
{ type: "item", id: "02-short.ts\u000002-short.ts", align: "start" },
122+
{ type: "item", id: "02-short.ts\u000002-short.ts", align: "start" },
123+
]);
124+
});
125+
126+
it("reveals newly selected files once in either direction", async () => {
127+
await mount();
128+
await activate("02-short.ts");
129+
await activate("01-tall.ts");
130+
await activate("03-medium.ts");
131+
expect(targets.map((target) => ("id" in target ? target.id : null))).toEqual(
132+
["02-short.ts", "01-tall.ts", "03-medium.ts"].map((path) => `${path}\0${path}`),
133+
);
134+
});
135+
136+
it("keeps focus-only navigation separate from button activation", async () => {
137+
await mount();
138+
await activate("02-short.ts");
139+
await act(async () => model().getItem("01-tall.ts")!.focus());
140+
expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);
141+
expect(targets).toHaveLength(1);
142+
await activate("01-tall.ts", { detail: 0 });
143+
await activate("01-tall.ts", { detail: 0 });
144+
expect(targets).toHaveLength(3);
145+
});
146+
147+
it.each(["ctrlKey", "metaKey"] as const)(
148+
"does not reveal a selected file that a %s click deselects",
149+
async (modifier) => {
150+
await mount();
151+
await activate("02-short.ts");
152+
await activate("02-short.ts", { [modifier]: true });
153+
expect(model().getSelectedPaths()).toEqual([]);
154+
expect(targets).toHaveLength(1);
155+
},
156+
);
157+
158+
it("lets a click narrow multiple selected files without a second reveal", async () => {
159+
await mount();
160+
await activate("02-short.ts");
161+
await act(async () => model().getItem("01-tall.ts")!.select());
162+
expect(model().getSelectedPaths()).toHaveLength(2);
163+
targets.length = 0;
164+
await activate("02-short.ts");
165+
expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);
166+
expect(targets).toHaveLength(1);
167+
});
168+
169+
it("leaves directory selection and expansion to the tree", async () => {
170+
await mount({ files: [{ path: "src/app.ts", status: "modified" }] });
171+
const directory = model().getItem("src/")!;
172+
if (!("isExpanded" in directory)) throw new Error("Expected the directory handle");
173+
expect(directory.isExpanded()).toBe(true);
174+
await activate("src/");
175+
expect(directory.isExpanded()).toBe(false);
176+
await activate("src/");
177+
expect(directory.isExpanded()).toBe(true);
178+
expect(targets).toEqual([]);
179+
});
180+
181+
it("does not echo controlled selection, but lets the reader activate it", async () => {
182+
await mount({ selectedPath: "02-short.ts" });
183+
expect(model().getSelectedPaths()).toEqual(["02-short.ts"]);
184+
expect(targets).toEqual([]);
185+
await activate("02-short.ts");
186+
expect(targets).toHaveLength(1);
187+
await act(async () => {
188+
renderer!.update(<Panel selectedPath="03-medium.ts" />);
189+
});
190+
expect(model().getSelectedPaths()).toEqual(["03-medium.ts"]);
191+
expect(targets).toHaveLength(1);
192+
});
193+
});

‎apps/web/src/components/diffs/DiffFileTree.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,29 @@ export function DiffFileTree({
177177
<FileTree
178178
model={model}
179179
aria-label={ariaLabel}
180+
onClickCapture={(event) => {
181+
if (
182+
event.defaultPrevented ||
183+
event.button !== 0 ||
184+
event.ctrlKey ||
185+
event.metaKey ||
186+
event.shiftKey ||
187+
event.altKey
188+
) {
189+
return;
190+
}
191+
// Pierre does not emit a selection change for its sole selected row.
192+
// Read selection before the row handles the click so new selections reveal only once.
193+
const selected = model.getSelectedPaths();
194+
const path = selected.length === 1 ? selected[0] : undefined;
195+
if (!path || !filePathsRef.current.has(path)) return;
196+
const clickedSelectedRow = event.nativeEvent
197+
.composedPath()
198+
.some(
199+
(node) => node instanceof HTMLElement && node.getAttribute("data-item-path") === path,
200+
);
201+
if (clickedSelectedRow) onSelectFileRef.current(path);
202+
}}
180203
className="min-h-0 flex-1 overflow-hidden"
181204
style={pierreTreeStyle(resolvedTheme)}
182205
/>

0 commit comments

Comments
 (0)