History

fix(web): reveal reselected diff files (#9951)

Julius Marminge committed 2 months ago

Commit 6b87ce31 parent f530d7b2 files · +216 −0Browse files

Unified · Split

2 files
  1. apps/web/src/components/diffs/DiffFileTree.test.tsx +193 −0
  2. apps/web/src/components/diffs/DiffFileTree.tsx +23 −0
apps/web/src/components/diffs/DiffFileTree.test.tsx+193−0
@@ -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+23−0
@@ -177,6 +177,29 @@ export function DiffFileTree({ · 176 unmodified lines
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 />