From 32579db52518b83282f89749d3ee6f78519bbfac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sun, 17 May 2026 21:26:13 +0100 Subject: [PATCH] refactor(web): unify file-tree components on a useTree headless hook (#919) --- .../settings/export/export-file-tree.test.tsx | 146 ++++++++ .../settings/export/export-file-tree.tsx | 220 +++++------- .../settings/export/export-preview.tsx | 1 - .../review/review-file-tree.test.tsx | 172 +++++++++ .../components/review/review-file-tree.tsx | 226 ++++++------ apps/web/components/review/types.ts | 18 +- apps/web/components/shared/file-tree.test.tsx | 200 +++++++++++ apps/web/components/shared/file-tree.tsx | 235 ++++++------- .../web/components/task/file-browser-hooks.ts | 56 ++- .../components/task/file-browser-parts.tsx | 146 ++++---- apps/web/components/task/file-browser.tsx | 2 +- .../components/task/file-tree-utils.test.ts | 33 ++ apps/web/components/task/file-tree-utils.ts | 13 +- .../e2e/tests/task/file-tree-create.spec.ts | 188 ++++++++++ .../tests/task/file-tree-drag-drop.spec.ts | 157 +++++++++ .../tests/task/file-tree-lazy-load.spec.ts | 114 ++++++ .../e2e/tests/task/file-tree-rename.spec.ts | 196 +++++++++++ .../e2e/tests/task/file-tree-search.spec.ts | 147 ++++++++ apps/web/hooks/use-tree-keyboard-nav.test.ts | 172 +++++++++ apps/web/hooks/use-tree-keyboard-nav.ts | 144 ++++++++ apps/web/hooks/use-tree.test.ts | 275 +++++++++++++++ apps/web/hooks/use-tree.ts | 325 ++++++++++++++++++ 22 files changed, 2689 insertions(+), 497 deletions(-) create mode 100644 apps/web/app/office/workspace/settings/export/export-file-tree.test.tsx create mode 100644 apps/web/components/review/review-file-tree.test.tsx create mode 100644 apps/web/components/shared/file-tree.test.tsx create mode 100644 apps/web/e2e/tests/task/file-tree-create.spec.ts create mode 100644 apps/web/e2e/tests/task/file-tree-drag-drop.spec.ts create mode 100644 apps/web/e2e/tests/task/file-tree-lazy-load.spec.ts create mode 100644 apps/web/e2e/tests/task/file-tree-rename.spec.ts create mode 100644 apps/web/e2e/tests/task/file-tree-search.spec.ts create mode 100644 apps/web/hooks/use-tree-keyboard-nav.test.ts create mode 100644 apps/web/hooks/use-tree-keyboard-nav.ts create mode 100644 apps/web/hooks/use-tree.test.ts create mode 100644 apps/web/hooks/use-tree.ts diff --git a/apps/web/app/office/workspace/settings/export/export-file-tree.test.tsx b/apps/web/app/office/workspace/settings/export/export-file-tree.test.tsx new file mode 100644 index 0000000000..73708d2607 --- /dev/null +++ b/apps/web/app/office/workspace/settings/export/export-file-tree.test.tsx @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup, fireEvent } from "@testing-library/react"; +import { ExportFileTree } from "./export-file-tree"; +import type { ExportFile } from "./export-types"; +import { buildFileTree } from "./export-utils"; + +afterEach(cleanup); + +const ALPHA_PATH = "agents/alpha.yml"; +const BETA_PATH = "agents/beta.yml"; +const FOO_PATH = "skills/foo.yml"; +const ROOT_PATH = "kandev.yml"; + +const FILES: ExportFile[] = [ + { path: ROOT_PATH, content: "name: root" }, + { path: ALPHA_PATH, content: "name: alpha" }, + { path: BETA_PATH, content: "name: beta" }, + { path: FOO_PATH, content: "name: foo" }, +]; + +const ALPHA_NAME = "alpha.yml"; +const CHECKBOX_SELECTOR = "[role='checkbox']"; + +type RenderOpts = Partial[0]>; + +function renderExport(overrides: RenderOpts = {}) { + const tree = buildFileTree(FILES); + const onSelectedPathsChange = vi.fn(); + const onPreviewPathChange = vi.fn(); + render( + f.path))} + onSelectedPathsChange={onSelectedPathsChange} + previewPath={null} + onPreviewPathChange={onPreviewPathChange} + {...overrides} + />, + ); + return { onSelectedPathsChange, onPreviewPathChange }; +} + +describe("ExportFileTree", () => { + it("renders all directories expanded by default", () => { + renderExport(); + // All dirs render expanded on mount, so every leaf file is visible. + expect(screen.getByText(ALPHA_NAME)).toBeTruthy(); + expect(screen.getByText("beta.yml")).toBeTruthy(); + expect(screen.getByText("foo.yml")).toBeTruthy(); + expect(screen.getByText(ROOT_PATH)).toBeTruthy(); + }); + + it("clicking a file row fires onPreviewPathChange with its path", () => { + const { onPreviewPathChange } = renderExport(); + fireEvent.click(screen.getByText(ALPHA_NAME)); + expect(onPreviewPathChange).toHaveBeenCalledWith(ALPHA_PATH); + }); + + it("clicking a directory row collapses it and hides children", () => { + renderExport(); + fireEvent.click(screen.getByText("agents")); + expect(screen.queryByText(ALPHA_NAME)).toBeNull(); + expect(screen.queryByText("beta.yml")).toBeNull(); + // Sibling dir's children remain visible. + expect(screen.getByText("foo.yml")).toBeTruthy(); + }); + + it("typing into the search input hides non-matching rows", () => { + renderExport(); + const input = screen.getByPlaceholderText("Search files..."); + fireEvent.change(input, { target: { value: "alpha" } }); + expect(screen.getByText(ALPHA_NAME)).toBeTruthy(); + expect(screen.queryByText("beta.yml")).toBeNull(); + expect(screen.queryByText("foo.yml")).toBeNull(); + expect(screen.queryByText(ROOT_PATH)).toBeNull(); + }); + + it("search keeps an ancestor visible when a descendant matches", () => { + renderExport(); + const input = screen.getByPlaceholderText("Search files..."); + fireEvent.change(input, { target: { value: "alpha" } }); + // The "agents" dir is shown because alpha.yml lives under it. + expect(screen.getByText("agents")).toBeTruthy(); + // The unrelated "skills" dir is hidden because none of its descendants match. + expect(screen.queryByText("skills")).toBeNull(); + }); + + it("toggling an unchecked file checkbox adds its path via onSelectedPathsChange", () => { + const { onSelectedPathsChange } = renderExport({ + selectedPaths: new Set(), + }); + // alpha.yml row's checkbox is rendered alongside the row. + const row = screen.getByText(ALPHA_NAME).parentElement!; + const box = row.querySelector(CHECKBOX_SELECTOR) as HTMLElement; + fireEvent.click(box); + expect(onSelectedPathsChange).toHaveBeenCalledTimes(1); + const next = onSelectedPathsChange.mock.calls[0][0] as Set; + expect(next.has(ALPHA_PATH)).toBe(true); + }); + + it("toggling a directory checkbox propagates to every descendant", () => { + const { onSelectedPathsChange } = renderExport({ + selectedPaths: new Set(), + }); + const row = screen.getByText("agents").parentElement!; + const box = row.querySelector(CHECKBOX_SELECTOR) as HTMLElement; + fireEvent.click(box); + const next = onSelectedPathsChange.mock.calls[0][0] as Set; + expect(next.has(ALPHA_PATH)).toBe(true); + expect(next.has(BETA_PATH)).toBe(true); + // Unrelated branch is untouched. + expect(next.has(FOO_PATH)).toBe(false); + }); + + it("dir checkbox renders indeterminate when only some descendants are selected", () => { + renderExport({ selectedPaths: new Set([ALPHA_PATH]) }); + const row = screen.getByText("agents").parentElement!; + const box = row.querySelector(CHECKBOX_SELECTOR) as HTMLElement; + expect(box.getAttribute("data-state")).toBe("indeterminate"); + }); + + it("dir checkbox renders fully checked when every descendant is selected", () => { + renderExport({ + selectedPaths: new Set([ALPHA_PATH, BETA_PATH]), + }); + const row = screen.getByText("agents").parentElement!; + const box = row.querySelector(CHECKBOX_SELECTOR) as HTMLElement; + expect(box.getAttribute("data-state")).toBe("checked"); + }); + + it("checkbox click does not bubble to the row's preview handler", () => { + const { onPreviewPathChange } = renderExport({ + selectedPaths: new Set(), + }); + const row = screen.getByText(ALPHA_NAME).parentElement!; + const box = row.querySelector(CHECKBOX_SELECTOR) as HTMLElement; + fireEvent.click(box); + expect(onPreviewPathChange).not.toHaveBeenCalled(); + }); + + it("previewPath highlights the matching file row", () => { + renderExport({ previewPath: ALPHA_PATH }); + const row = screen.getByText(ALPHA_NAME).parentElement!; + expect(row.className).toContain("bg-accent"); + }); +}); diff --git a/apps/web/app/office/workspace/settings/export/export-file-tree.tsx b/apps/web/app/office/workspace/settings/export/export-file-tree.tsx index 4416328518..2c15f44259 100644 --- a/apps/web/app/office/workspace/settings/export/export-file-tree.tsx +++ b/apps/web/app/office/workspace/settings/export/export-file-tree.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useState } from "react"; +import { useState } from "react"; import { IconFile, IconFolder, @@ -12,70 +12,63 @@ import { import { Input } from "@kandev/ui/input"; import { Checkbox } from "@kandev/ui/checkbox"; import { cn } from "@/lib/utils"; -import type { ExportFile, FileTreeNode } from "./export-types"; +import { useTree, type VisibleRow } from "@/hooks/use-tree"; +import type { FileTreeNode } from "./export-types"; import { getDescendantFilePaths } from "./export-utils"; interface ExportFileTreeProps { tree: FileTreeNode[]; - files: ExportFile[]; selectedPaths: Set; onSelectedPathsChange: (next: Set) => void; previewPath: string | null; onPreviewPathChange: (path: string) => void; } +// Stable adapter identities so useTree's visibleRows memoisation is not +// invalidated on every parent render. +const EXPORT_GET_PATH = (n: FileTreeNode) => n.path; +const EXPORT_GET_CHILDREN = (n: FileTreeNode) => n.children; +const EXPORT_IS_DIR = (n: FileTreeNode) => n.isDir; + +type CheckState = boolean | "indeterminate"; + +function getCheckState(node: FileTreeNode, selectedPaths: Set): CheckState { + const descendants = getDescendantFilePaths(node); + // Empty descendants would make .every() vacuously true. + if (descendants.length === 0) return false; + if (descendants.every((p) => selectedPaths.has(p))) return true; + if (descendants.some((p) => selectedPaths.has(p))) return "indeterminate"; + return false; +} + export function ExportFileTree({ tree, - files, selectedPaths, onSelectedPathsChange, previewPath, onPreviewPathChange, }: ExportFileTreeProps) { const [search, setSearch] = useState(""); - const [expanded, setExpanded] = useState>(() => { - // Expand all directories by default - const dirs = new Set(); - function walk(nodes: FileTreeNode[]) { - for (const n of nodes) { - if (n.isDir) { - dirs.add(n.path); - walk(n.children); - } - } - } - walk(tree); - return dirs; + + const { visibleRows, toggle } = useTree({ + nodes: tree, + getPath: EXPORT_GET_PATH, + getChildren: EXPORT_GET_CHILDREN, + isDir: EXPORT_IS_DIR, + defaultExpanded: "all", + search, + searchMode: "hide", }); - const toggleExpand = useCallback((path: string) => { - setExpanded((prev) => { - const next = new Set(prev); - if (next.has(path)) next.delete(path); - else next.add(path); - return next; - }); - }, []); - - const toggleSelection = useCallback( - (node: FileTreeNode) => { - const paths = getDescendantFilePaths(node); - const allSelected = paths.every((p) => selectedPaths.has(p)); - const next = new Set(selectedPaths); - for (const p of paths) { - if (allSelected) next.delete(p); - else next.add(p); - } - onSelectedPathsChange(next); - }, - [selectedPaths, onSelectedPathsChange], - ); - - const lowerSearch = search.toLowerCase(); - const matchesSearch = (node: FileTreeNode): boolean => { - if (!lowerSearch) return true; - if (node.name.toLowerCase().includes(lowerSearch)) return true; - return node.children.some(matchesSearch); + const toggleSelection = (node: FileTreeNode) => { + const paths = getDescendantFilePaths(node); + const allSelected = paths.every((p) => selectedPaths.has(p)); + const next = new Set(selectedPaths); + for (const p of paths) { + if (allSelected) next.delete(p); + else next.add(p); + } + onSelectedPathsChange(next); }; return ( @@ -92,19 +85,17 @@ export function ExportFileTree({
- {tree.filter(matchesSearch).map((node) => ( - ( + { + if (row.isDir) toggle(row.path); + else onPreviewPathChange(row.path); + }} + onToggleCheck={() => toggleSelection(row.node)} /> ))}
@@ -112,94 +103,37 @@ export function ExportFileTree({ ); } -interface TreeRowProps { - node: FileTreeNode; - depth: number; - expanded: Set; - selectedPaths: Set; - previewPath: string | null; - onToggleExpand: (path: string) => void; - onToggleSelection: (node: FileTreeNode) => void; - onPreview: (path: string) => void; - matchesSearch: (node: FileTreeNode) => boolean; - allFiles: ExportFile[]; +interface ExportTreeRowProps { + row: VisibleRow; + isActive: boolean; + checkState: CheckState; + onClick: () => void; + onToggleCheck: () => void; } -function TreeRow({ - node, - depth, - expanded, - selectedPaths, - previewPath, - onToggleExpand, - onToggleSelection, - onPreview, - matchesSearch, - allFiles, -}: TreeRowProps) { - const isExpanded = expanded.has(node.path); - const descendants = getDescendantFilePaths(node); - const allSelected = descendants.every((p) => selectedPaths.has(p)); - const someSelected = !allSelected && descendants.some((p) => selectedPaths.has(p)); - const isActive = !node.isDir && previewPath === node.path; - - const handleClick = () => { - if (node.isDir) { - onToggleExpand(node.path); - } else { - onPreview(node.path); - } - }; - +function ExportTreeRow({ row, isActive, checkState, onClick, onToggleCheck }: ExportTreeRowProps) { + let FileIconComp = IconFile; + if (row.isDir) FileIconComp = row.isExpanded ? IconFolderOpen : IconFolder; let ChevronIcon: typeof IconChevronDown | null = null; - if (node.isDir) ChevronIcon = isExpanded ? IconChevronDown : IconChevronRight; - - let FileIcon = IconFile; - if (node.isDir) FileIcon = isExpanded ? IconFolderOpen : IconFolder; - - let checkedState: boolean | "indeterminate" = false; - if (allSelected) checkedState = true; - else if (someSelected) checkedState = "indeterminate"; - + if (row.isDir) ChevronIcon = row.isExpanded ? IconChevronDown : IconChevronRight; return ( - <> -
- onToggleSelection(node)} - onClick={(e) => e.stopPropagation()} - className="cursor-pointer shrink-0" - /> - {ChevronIcon && } - - {node.name} -
- {node.isDir && - isExpanded && - node.children - .filter(matchesSearch) - .map((child) => ( - - ))} - +
+ e.stopPropagation()} + className="cursor-pointer shrink-0" + /> + {ChevronIcon && } + + {row.displayName} +
); } diff --git a/apps/web/app/office/workspace/settings/export/export-preview.tsx b/apps/web/app/office/workspace/settings/export/export-preview.tsx index 37768b8807..a9fbf4ccb0 100644 --- a/apps/web/app/office/workspace/settings/export/export-preview.tsx +++ b/apps/web/app/office/workspace/settings/export/export-preview.tsx @@ -93,7 +93,6 @@ export function ExportPreview() {
): ReviewFile { + return { + path: APP_PATH, + diff: "", + status: "modified", + additions: 1, + deletions: 0, + staged: false, + source: "uncommitted", + ...overrides, + }; +} + +type RenderOpts = Partial[0]>; + +function renderTree(files: ReviewFile[], overrides: RenderOpts = {}) { + const onFilterChange = vi.fn(); + const onSelectFile = vi.fn(); + const onToggleReviewed = vi.fn(); + const props = { + files, + reviewedFiles: new Set(), + staleFiles: new Set(), + commentCountByFile: {} as Record, + selectedFile: null, + filter: "", + onFilterChange, + onSelectFile, + onToggleReviewed, + ...overrides, + }; + render(); + return { onFilterChange, onSelectFile, onToggleReviewed }; +} + +describe("ReviewFileTree", () => { + it("renders one row per file", () => { + renderTree([file({ path: "src/a.ts" }), file({ path: "src/b.ts" })]); + expect(screen.getByText("a.ts")).toBeTruthy(); + expect(screen.getByText("b.ts")).toBeTruthy(); + }); + + it("clicking a file row fires onSelectFile with the composite key", () => { + const f = file({ path: APP_PATH, repository_name: "frontend", repository_id: "f" }); + const { onSelectFile } = renderTree([f]); + const row = screen.getByText("app.tsx"); + fireEvent.click(row); + expect(onSelectFile).toHaveBeenCalledWith(reviewFileKey(f)); + }); + + it("toggling the reviewed checkbox fires onToggleReviewed with the composite key", () => { + const f = file({ path: APP_PATH, repository_name: "frontend", repository_id: "f" }); + const { onToggleReviewed, onSelectFile } = renderTree([f]); + // The checkbox is the only role=checkbox in the tree. + const checkbox = screen.getByRole("checkbox"); + fireEvent.click(checkbox); + expect(onToggleReviewed).toHaveBeenCalledTimes(1); + expect(onToggleReviewed).toHaveBeenCalledWith(reviewFileKey(f), true); + // Checkbox click must NOT also fire onSelectFile - the row's onClick is + // stopped by the checkbox's stopPropagation handler. This invariant + // matters because clicking "I reviewed this file" should not navigate + // to it. + expect(onSelectFile).not.toHaveBeenCalled(); + }); + + it("reviewed checkbox renders unchecked when the file is in staleFiles", () => { + const f = file({ path: APP_PATH, repository_name: "x", repository_id: "x" }); + const key = reviewFileKey(f); + renderTree([f], { + reviewedFiles: new Set([key]), + staleFiles: new Set([key]), + }); + // Stale takes precedence over reviewed: checkbox is unchecked. + const checkbox = screen.getByRole("checkbox") as HTMLInputElement; + // Radix Checkbox surfaces the state via aria-checked / data-state, not + // the underlying input's `checked` attribute. + expect(checkbox.getAttribute("aria-checked")).not.toBe("true"); + }); + + it("reviewed checkbox renders checked when reviewed and not stale", () => { + const f = file({ path: APP_PATH, repository_name: "x", repository_id: "x" }); + const key = reviewFileKey(f); + renderTree([f], { + reviewedFiles: new Set([key]), + staleFiles: new Set(), + }); + const checkbox = screen.getByRole("checkbox"); + expect(checkbox.getAttribute("aria-checked")).toBe("true"); + }); + + it("renders comment count badge for files with comments", () => { + const f = file({ path: APP_PATH, repository_name: "x", repository_id: "x" }); + const key = reviewFileKey(f); + renderTree([f], { commentCountByFile: { [key]: 3 } }); + expect(screen.getByText("3")).toBeTruthy(); + }); + + it("filter input change fires onFilterChange", () => { + const { onFilterChange } = renderTree([file({ path: "a.ts" })]); + const input = screen.getByPlaceholderText("Filter changed files") as HTMLInputElement; + fireEvent.change(input, { target: { value: "abc" } }); + expect(onFilterChange).toHaveBeenCalledWith("abc"); + }); + + it("X button on the filter input clears the filter via onFilterChange", () => { + const { onFilterChange } = renderTree([file({ path: "a.ts" })], { filter: "abc" }); + // The X button has no accessible name; locate it via the input's sibling + // button. With filter set, the clear button is rendered. + const input = screen.getByPlaceholderText("Filter changed files"); + const container = input.parentElement!; + const clear = within(container).getByRole("button"); + fireEvent.click(clear); + expect(onFilterChange).toHaveBeenCalledWith(""); + }); + + it("groups files under a repo-root header when 2+ distinct repositories are present", () => { + renderTree([ + file({ path: "src/a.ts", repository_name: "frontend", repository_id: "f" }), + file({ path: "handlers/task.go", repository_name: "backend", repository_id: "b" }), + ]); + const repoHeaders = screen.getAllByTestId("repo-root-node"); + const names = repoHeaders.map((h) => h.textContent ?? ""); + expect(names.some((n) => n.includes("frontend"))).toBe(true); + expect(names.some((n) => n.includes("backend"))).toBe(true); + }); + + it("collapsing a directory header hides its children", () => { + renderTree([ + file({ path: "src/a.ts", repository_name: "frontend", repository_id: "f" }), + file({ path: "handlers/task.go", repository_name: "backend", repository_id: "b" }), + ]); + // Both files visible by default (TreeNode expanded=true initially). + expect(screen.getByText("a.ts")).toBeTruthy(); + expect(screen.getByText("task.go")).toBeTruthy(); + + // Click the "frontend" repo root header to collapse. + const repoHeaders = screen.getAllByTestId("repo-root-node"); + const frontend = repoHeaders.find((h) => h.textContent?.includes("frontend"))!; + const toggle = within(frontend).getAllByRole("button")[0]; + fireEvent.click(toggle); + // a.ts no longer rendered. + expect(screen.queryByText("a.ts")).toBeNull(); + // backend's child is still visible. + expect(screen.getByText("task.go")).toBeTruthy(); + }); + + it("disambiguates same-named files in different repos via composite key", () => { + const fA = file({ path: "README.md", repository_name: "frontend", repository_id: "f" }); + const fB = file({ path: "README.md", repository_name: "backend", repository_id: "b" }); + const { onSelectFile } = renderTree([fA, fB]); + // There are two rows with the same display name; clicking each must + // dispatch a distinct key. + const rows = screen.getAllByText("README.md"); + expect(rows).toHaveLength(2); + fireEvent.click(rows[0]); + fireEvent.click(rows[1]); + const calls = onSelectFile.mock.calls.map((c) => c[0] as string); + expect(new Set(calls).size).toBe(2); + expect(calls).toContain(reviewFileKey(fA)); + expect(calls).toContain(reviewFileKey(fB)); + }); +}); diff --git a/apps/web/components/review/review-file-tree.tsx b/apps/web/components/review/review-file-tree.tsx index 914d49fc0d..218d5eba0d 100644 --- a/apps/web/components/review/review-file-tree.tsx +++ b/apps/web/components/review/review-file-tree.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo, useState, useCallback, useMemo, useRef } from "react"; +import { memo, useMemo, useRef } from "react"; import { IconChevronDown, IconChevronRight, @@ -12,6 +12,7 @@ import { import { Checkbox } from "@kandev/ui/checkbox"; import { cn } from "@kandev/ui/lib/utils"; import { FileIcon } from "@/components/ui/file-icon"; +import { useTree, type VisibleRow } from "@/hooks/use-tree"; import type { ReviewFile, FileTreeNode } from "./types"; import { buildFileTree, reviewFileKey } from "./types"; @@ -27,6 +28,12 @@ type ReviewFileTreeProps = { onToggleReviewed: (path: string, reviewed: boolean) => void; }; +// Stable adapter identities so useTree's visibleRows memoisation is not +// invalidated on every parent render. +const REVIEW_GET_PATH = (n: FileTreeNode) => n.path; +const REVIEW_GET_CHILDREN = (n: FileTreeNode) => n.children; +const REVIEW_IS_DIR = (n: FileTreeNode) => Boolean(n.isDir); + export const ReviewFileTree = memo(function ReviewFileTree({ files, reviewedFiles, @@ -41,43 +48,29 @@ export const ReviewFileTree = memo(function ReviewFileTree({ const inputRef = useRef(null); const tree = useMemo(() => buildFileTree(files), [files]); + const { visibleRows, toggle } = useTree({ + nodes: tree, + getPath: REVIEW_GET_PATH, + getChildren: REVIEW_GET_CHILDREN, + isDir: REVIEW_IS_DIR, + defaultExpanded: "all", + }); + return (
-
-
- - onFilterChange(e.target.value)} - placeholder="Filter changed files" - className="flex-1 bg-transparent text-[13px] text-foreground placeholder:text-muted-foreground outline-none min-w-0" - /> - {filter && ( - - )} -
-
+
- {tree.map((node) => ( - ( + toggle(row.path)} /> ))}
@@ -85,30 +78,103 @@ export const ReviewFileTree = memo(function ReviewFileTree({ ); }); -type TreeNodeProps = { - node: FileTreeNode; +interface ReviewFilterInputProps { + ref: React.RefObject; + value: string; + onChange: (v: string) => void; +} + +function ReviewFilterInput({ ref, value, onChange }: ReviewFilterInputProps) { + return ( +
+
+ + onChange(e.target.value)} + placeholder="Filter changed files" + className="flex-1 bg-transparent text-[13px] text-foreground placeholder:text-muted-foreground outline-none min-w-0" + /> + {value && ( + + )} +
+
+ ); +} + +interface ReviewTreeRowProps { + row: VisibleRow; reviewedFiles: Set; staleFiles: Set; commentCountByFile: Record; selectedFile: string | null; onSelectFile: (path: string) => void; onToggleReviewed: (path: string, reviewed: boolean) => void; - depth: number; -}; + onToggleDir: () => void; +} -function FileNode({ - node, +function ReviewTreeRow(props: ReviewTreeRowProps) { + if (props.row.isDir) return ; + return ; +} + +function ReviewDirRow({ row, onToggleDir }: ReviewTreeRowProps) { + const isRepoRoot = Boolean(row.node.isRepoRoot); + const fileCount = isRepoRoot ? countLeafFiles(row.node) : 0; + return ( +
+ +
+ ); +} + +function ReviewFileRow({ + row, reviewedFiles, staleFiles, commentCountByFile, selectedFile, onSelectFile, onToggleReviewed, - depth, -}: Omit & { node: FileTreeNode }) { - const file = node.file!; - // Composite key from reviewFileKey() so two same-name files in - // different repos don't share reviewed/stale/comment-count slots. +}: ReviewTreeRowProps) { + const file = row.node.file as ReviewFile; + // Composite key from reviewFileKey() so two same-name files in different + // repos don't share reviewed/stale/comment-count slots. const key = reviewFileKey(file); const isReviewed = reviewedFiles.has(key); const isStale = staleFiles.has(key); @@ -120,19 +186,17 @@ function FileNode({ "flex items-center gap-1.5 px-2 py-1 cursor-pointer transition-colors group", isSelected ? "bg-accent/50" : "hover:bg-muted/50", )} - style={{ paddingLeft: `${depth * 12 + 8}px` }} + style={{ paddingLeft: `${row.depth * 12 + 8}px` }} onClick={() => onSelectFile(key)} > { - onToggleReviewed(key, checked === true); - }} + onCheckedChange={(checked) => onToggleReviewed(key, checked === true)} onClick={(e) => e.stopPropagation()} className="h-3.5 w-3.5" /> - - {node.name} + + {row.node.name} {isStale && } {commentCount > 0 && ( @@ -144,76 +208,6 @@ function FileNode({ ); } -function TreeNode({ - node, - reviewedFiles, - staleFiles, - commentCountByFile, - selectedFile, - onSelectFile, - onToggleReviewed, - depth, -}: TreeNodeProps) { - const [expanded, setExpanded] = useState(true); - const handleToggle = useCallback(() => { - setExpanded((prev) => !prev); - }, []); - const sharedProps = { - reviewedFiles, - staleFiles, - commentCountByFile, - selectedFile, - onSelectFile, - onToggleReviewed, - }; - - if (node.isDir) { - // Repo roots get a stronger label so the user sees the multi-repo grouping. - const fileCount = node.isRepoRoot ? countLeafFiles(node) : 0; - return ( -
- - {expanded && node.children && ( -
- {node.children.map((child) => ( - - ))} -
- )} -
- ); - } - - return ; -} - -/** Counts leaf (file) nodes anywhere under the given node. Used for the - * "N files" badge on multi-repo repo-root headers. */ function countLeafFiles(node: FileTreeNode): number { if (!node.isDir) return 1; let total = 0; diff --git a/apps/web/components/review/types.ts b/apps/web/components/review/types.ts index 25917a60fe..b7479b5c23 100644 --- a/apps/web/components/review/types.ts +++ b/apps/web/components/review/types.ts @@ -131,20 +131,34 @@ function buildMultiRepoTree(files: ReviewFile[]): FileTreeNode[] { } const repoRoots: FileTreeNode[] = []; for (const [, entry] of byRepo) { + const repoPath = `__repo__:${entry.id}`; const subtree = buildFlatTree(entry.files); + // Two repos can contain a same-named path ("src/foo.ts" in both + // frontend and backend). Prefix every node path under this repo with + // the repo root path so tree-node paths are globally unique — required + // for useTree's expansion Set to track them independently. + const prefixedSubtree = subtree.map((n) => prefixPaths(n, repoPath)); repoRoots.push({ name: entry.name, - path: `__repo__:${entry.id}`, + path: repoPath, isDir: true, isRepoRoot: true, repositoryId: entry.id, - children: subtree, + children: prefixedSubtree, }); } // Stable alphabetical order by repo name. return repoRoots.sort((a, b) => a.name.localeCompare(b.name)); } +function prefixPaths(node: FileTreeNode, prefix: string): FileTreeNode { + return { + ...node, + path: `${prefix}/${node.path}`, + children: node.children?.map((c) => prefixPaths(c, prefix)), + }; +} + function buildFlatTree(files: ReviewFile[]): FileTreeNode[] { const root: FileTreeNode = { name: "", path: "", isDir: true, children: [] }; diff --git a/apps/web/components/shared/file-tree.test.tsx b/apps/web/components/shared/file-tree.test.tsx new file mode 100644 index 0000000000..92515fb59e --- /dev/null +++ b/apps/web/components/shared/file-tree.test.tsx @@ -0,0 +1,200 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup, fireEvent } from "@testing-library/react"; +import { FileTree, type FileTreeNode } from "./file-tree"; + +afterEach(cleanup); + +function file(name: string, parentPath = ""): FileTreeNode { + return { + name, + path: parentPath ? `${parentPath}/${name}` : name, + isDir: false, + children: [], + }; +} + +type RenderOpts = Partial[0]>; + +function renderTree(nodes: FileTreeNode[], overrides: RenderOpts = {}) { + const onSelectPath = vi.fn(); + const onCheckedPathsChange = vi.fn(); + render( + , + ); + return { onSelectPath, onCheckedPathsChange }; +} + +describe("FileTree rendering and selection", () => { + it("renders top-level nodes by default with dirs collapsed", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [file("a.ts", "src"), file("b.ts", "src")], + }, + file("README.md"), + ]; + renderTree(tree); + expect(screen.getByText("src")).toBeTruthy(); + expect(screen.getByText("README.md")).toBeTruthy(); + // Children of "src" are hidden until expanded. + expect(screen.queryByText("a.ts")).toBeNull(); + expect(screen.queryByText("b.ts")).toBeNull(); + }); + + it("clicking a directory expands it and reveals children", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [file("a.ts", "src")], + }, + ]; + renderTree(tree); + fireEvent.click(screen.getByText("src")); + expect(screen.getByText("a.ts")).toBeTruthy(); + }); + + it("clicking a file fires onSelectPath with its path", () => { + const tree: FileTreeNode[] = [file("README.md")]; + const { onSelectPath } = renderTree(tree); + fireEvent.click(screen.getByText("README.md")); + expect(onSelectPath).toHaveBeenCalledWith("README.md"); + }); + + it("defaultExpanded=true expands all directories on mount", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [ + { + name: "ui", + path: "src/ui", + isDir: true, + children: [file("Button.tsx", "src/ui")], + }, + ], + }, + ]; + renderTree(tree, { defaultExpanded: true }); + // The src/ui chain collapses to a single display row "src/ui", + // and the deepest dir's children are visible because all dirs + // are pre-expanded. + expect(screen.getByText("Button.tsx")).toBeTruthy(); + }); + + it("collapses single-child directory chains into 'a/b' display names", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [ + { + name: "ui", + path: "src/ui", + isDir: true, + children: [file("Button.tsx", "src/ui")], + }, + ], + }, + ]; + renderTree(tree); + // Initial render: "src/ui" appears as one row (chain collapse), no + // separate "src" or "ui" rows. + expect(screen.getByText("src/ui")).toBeTruthy(); + expect(screen.queryByText("src")).toBeNull(); + expect(screen.queryByText("ui")).toBeNull(); + }); + + it("selectedPath highlights the matching file row", () => { + const tree: FileTreeNode[] = [file("a.ts")]; + renderTree(tree, { selectedPath: "a.ts" }); + const row = screen.getByText("a.ts").parentElement!; + expect(row.className).toContain("bg-accent"); + }); + + it("renderExtra slot is invoked per row and its output appears in the DOM", () => { + const tree: FileTreeNode[] = [file("a.ts"), file("b.ts")]; + const renderExtra = vi.fn((n: FileTreeNode) => {`x:${n.name}`}); + renderTree(tree, { renderExtra }); + expect(screen.getByText("x:a.ts")).toBeTruthy(); + expect(screen.getByText("x:b.ts")).toBeTruthy(); + expect(renderExtra).toHaveBeenCalledTimes(2); + }); +}); + +describe("FileTree checkboxes", () => { + it("does not render checkboxes when showCheckboxes is false", () => { + const tree: FileTreeNode[] = [file("a.ts")]; + renderTree(tree); + expect(screen.queryByRole("checkbox")).toBeNull(); + }); + + it("renders a checkbox per node when showCheckboxes and checkedPaths are set", () => { + const tree: FileTreeNode[] = [file("a.ts"), file("b.ts")]; + renderTree(tree, { showCheckboxes: true, checkedPaths: new Set() }); + expect(screen.getAllByRole("checkbox")).toHaveLength(2); + }); + + it("clicking a directory checkbox propagates the toggle to every leaf under it", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [file("a.ts", "src"), file("b.ts", "src")], + }, + ]; + const { onCheckedPathsChange } = renderTree(tree, { + showCheckboxes: true, + checkedPaths: new Set(), + defaultExpanded: true, + }); + // There are three checkboxes: src, a.ts, b.ts. The first one is the dir. + const boxes = screen.getAllByRole("checkbox"); + fireEvent.click(boxes[0]); + expect(onCheckedPathsChange).toHaveBeenCalledTimes(1); + const next = onCheckedPathsChange.mock.calls[0][0] as Set; + expect(next.has("src/a.ts")).toBe(true); + expect(next.has("src/b.ts")).toBe(true); + }); + + it("dir checkbox renders indeterminate when only some descendants are checked", () => { + const tree: FileTreeNode[] = [ + { + name: "src", + path: "src", + isDir: true, + children: [file("a.ts", "src"), file("b.ts", "src")], + }, + ]; + renderTree(tree, { + showCheckboxes: true, + checkedPaths: new Set(["src/a.ts"]), + defaultExpanded: true, + }); + const [dirBox] = screen.getAllByRole("checkbox"); + expect(dirBox.getAttribute("data-state")).toBe("indeterminate"); + }); + + it("checkbox click does not bubble to the row's onSelectPath", () => { + const tree: FileTreeNode[] = [file("a.ts")]; + const { onSelectPath } = renderTree(tree, { + showCheckboxes: true, + checkedPaths: new Set(), + }); + const box = screen.getByRole("checkbox"); + fireEvent.click(box); + expect(onSelectPath).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/components/shared/file-tree.tsx b/apps/web/components/shared/file-tree.tsx index 978036fc44..3da4c86dc5 100644 --- a/apps/web/components/shared/file-tree.tsx +++ b/apps/web/components/shared/file-tree.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useState, type ReactNode } from "react"; +import { useCallback, type ReactNode } from "react"; import { IconChevronRight, IconChevronDown, @@ -10,6 +10,8 @@ import { } from "@tabler/icons-react"; import { Checkbox } from "@kandev/ui/checkbox"; import { cn } from "@/lib/utils"; +import { useTree, type VisibleRow } from "@/hooks/use-tree"; +import { useTreeKeyboardNav } from "@/hooks/use-tree-keyboard-nav"; export interface FileTreeNode { name: string; @@ -19,6 +21,13 @@ export interface FileTreeNode { content?: string; } +// Module-level adapter constants so they keep a stable identity across +// renders. Without this, useTree's visibleRows useMemo would recompute on +// every parent render — the adapters land in its dependency array. +const FILE_TREE_GET_PATH = (n: FileTreeNode) => n.path; +const FILE_TREE_GET_CHILDREN = (n: FileTreeNode) => n.children; +const FILE_TREE_IS_DIR = (n: FileTreeNode) => n.isDir; + interface FileTreeProps { nodes: FileTreeNode[]; selectedPath?: string | null; @@ -30,21 +39,22 @@ interface FileTreeProps { defaultExpanded?: boolean; } -/** Collect all leaf (file) paths under a node */ +/** Collect all leaf (file) paths under a node. */ function getLeafPaths(node: FileTreeNode): string[] { if (!node.isDir) return [node.path]; return node.children.flatMap(getLeafPaths); } -/** Collapse single-child directory chains into "a/b/c" display names */ -function collapseChain(node: FileTreeNode): { displayName: string; node: FileTreeNode } { - let current = node; - let name = current.name; - while (current.isDir && current.children.length === 1 && current.children[0].isDir) { - current = current.children[0]; - name = `${name}/${current.name}`; - } - return { displayName: name, node: current }; +type CheckState = boolean | "indeterminate"; + +function getCheckState(node: FileTreeNode, checkedPaths: Set): CheckState { + const leaves = getLeafPaths(node); + // An empty leaves array would make .every() vacuously true and the dir + // would render as checked. A dir with no file leaves can't be "all checked". + if (leaves.length === 0) return false; + if (leaves.every((p) => checkedPaths.has(p))) return true; + if (leaves.some((p) => checkedPaths.has(p))) return "indeterminate"; + return false; } export function FileTree({ @@ -57,31 +67,25 @@ export function FileTree({ renderExtra, defaultExpanded = false, }: FileTreeProps) { - const [expanded, setExpanded] = useState>(() => { - if (!defaultExpanded) return new Set(); - const dirs = new Set(); - function walk(list: FileTreeNode[]) { - for (const n of list) { - if (n.isDir) { - dirs.add(n.path); - walk(n.children); - } - } - } - walk(nodes); - return dirs; + const tree = useTree({ + nodes, + getPath: FILE_TREE_GET_PATH, + getChildren: FILE_TREE_GET_CHILDREN, + isDir: FILE_TREE_IS_DIR, + chainCollapse: true, + defaultExpanded: defaultExpanded ? "all" : undefined, + }); + const { visibleRows, toggle } = tree; + const nav = useTreeKeyboardNav({ + visibleRows, + toggle, + expand: tree.expand, + collapse: tree.collapse, + isExpanded: tree.isExpanded, + onActivate: (row) => onSelectPath?.(row.path), }); - const toggleExpand = useCallback((path: string) => { - setExpanded((prev) => { - const next = new Set(prev); - if (next.has(path)) next.delete(path); - else next.add(path); - return next; - }); - }, []); - - const toggleCheck = useCallback( + const handleToggleCheck = useCallback( (node: FileTreeNode) => { if (!checkedPaths || !onCheckedPathsChange) return; const paths = getLeafPaths(node); @@ -97,19 +101,28 @@ export function FileTree({ ); return ( -
- {nodes.map((node) => ( - + {visibleRows.map((row) => ( + { + nav.setFocusedPath(row.path); + if (row.isDir) toggle(row.path); + else onSelectPath?.(row.path); + }} + onToggleCheck={() => handleToggleCheck(row.node)} renderExtra={renderExtra} /> ))} @@ -117,102 +130,56 @@ export function FileTree({ ); } -interface TreeNodeRowProps { - node: FileTreeNode; - depth: number; - expanded: Set; - onToggleExpand: (path: string) => void; - selectedPath?: string | null; - onSelectPath?: (path: string) => void; - checkedPaths?: Set; +interface FileTreeRowProps { + row: VisibleRow; + isActive: boolean; + isFocused: boolean; + checkState?: CheckState; showCheckboxes: boolean; - onToggleCheck: (node: FileTreeNode) => void; + onClick: () => void; + onToggleCheck: () => void; renderExtra?: (node: FileTreeNode) => ReactNode; } -function TreeNodeRow({ - node, - depth, - expanded, - onToggleExpand, - selectedPath, - onSelectPath, - checkedPaths, +function FileTreeRow({ + row, + isActive, + isFocused, + checkState, showCheckboxes, + onClick, onToggleCheck, renderExtra, -}: TreeNodeRowProps) { - const { displayName, node: effectiveNode } = collapseChain(node); - const isExpanded = expanded.has(effectiveNode.path); - const isActive = !effectiveNode.isDir && selectedPath === effectiveNode.path; - - const handleClick = () => { - if (effectiveNode.isDir) { - onToggleExpand(effectiveNode.path); - } else { - onSelectPath?.(effectiveNode.path); - } - }; - - const checkState = (() => { - if (!showCheckboxes || !checkedPaths) return undefined; - const leaves = getLeafPaths(effectiveNode); - const allChecked = leaves.every((p) => checkedPaths.has(p)); - if (allChecked) return true; - const someChecked = leaves.some((p) => checkedPaths.has(p)); - if (someChecked) return "indeterminate" as const; - return false; - })(); - - const FolderIcon = isExpanded ? IconFolderOpen : IconFolder; - const ChevronIcon = isExpanded ? IconChevronDown : IconChevronRight; +}: FileTreeRowProps) { + const FolderIcon = row.isExpanded ? IconFolderOpen : IconFolder; + const ChevronIcon = row.isExpanded ? IconChevronDown : IconChevronRight; return ( - <> -
- {showCheckboxes && checkState !== undefined && ( - onToggleCheck(effectiveNode)} - onClick={(e) => e.stopPropagation()} - className="cursor-pointer shrink-0" - /> - )} - {effectiveNode.isDir && ( - - )} - {effectiveNode.isDir ? ( - - ) : ( - - )} - {displayName} - {renderExtra?.(effectiveNode)} -
- {effectiveNode.isDir && - isExpanded && - effectiveNode.children.map((child) => ( - - ))} - +
+ {showCheckboxes && checkState !== undefined && ( + e.stopPropagation()} + className="cursor-pointer shrink-0" + /> + )} + {row.isDir && } + {row.isDir ? ( + + ) : ( + + )} + {row.displayName} + {renderExtra?.(row.node)} +
); } diff --git a/apps/web/components/task/file-browser-hooks.ts b/apps/web/components/task/file-browser-hooks.ts index e93df8eb72..736e2b2e1f 100644 --- a/apps/web/components/task/file-browser-hooks.ts +++ b/apps/web/components/task/file-browser-hooks.ts @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useState, useCallback, useRef } from "react"; +import { useEffect, useState, useCallback, useRef, useMemo } from "react"; import { getWebSocketClient } from "@/lib/ws/connection"; import { requestFileTree, @@ -16,7 +16,20 @@ import { setFilesPanelScrollPosition, } from "@/lib/local-storage"; import type { useToast } from "@/components/toast-provider"; +import { useTree, type VisibleRow } from "@/hooks/use-tree"; import { mergeTreeNodes } from "./file-browser-parts"; +import { compareTreeNodes, sortRootChildren } from "./file-tree-utils"; + +const FB_GET_PATH = (n: FileTreeNode) => n.path; +// Children are sorted (dirs first, then files, alphabetically) on every +// access so the flat visibleRows list comes out in the same order the +// recursive render produced. The backend doesn't guarantee order and tree +// mutations (rename / create) can insert arbitrarily. +const FB_GET_CHILDREN = (n: FileTreeNode) => + n.children ? [...n.children].sort(compareTreeNodes) : undefined; +const FB_IS_DIR = (n: FileTreeNode) => n.is_dir; + +export type FileBrowserRow = VisibleRow; const MAX_RETRY_ATTEMPTS = 4; const RETRY_DELAYS_MS = [1000, 2000, 5000, 10000]; @@ -108,7 +121,7 @@ export function useFileBrowserSearch(sessionId: string) { function applyFileChanges(ctx: { client: ReturnType; sessionId: string; - expandedPaths: Set; + expandedPaths: ReadonlySet; changes: Array<{ path: string }>; setTree: React.Dispatch>; setLoadState: React.Dispatch>; @@ -279,15 +292,18 @@ function useTreeLoader(ctx: TreeLoaderContext) { function useFileChangeSubscription({ sessionIdRef, - expandedPaths, + expandedPathsRef, setTree, setLoadState, }: { sessionIdRef: React.MutableRefObject; - expandedPaths: Set; + expandedPathsRef: React.MutableRefObject>; setTree: React.Dispatch>; setLoadState: React.Dispatch>; }) { + // expandedPaths is read via ref so that toggling a directory (which mints a + // new Set) does not tear down and re-attach this WS listener — any event + // arriving during the swap would otherwise be silently dropped. useEffect(() => { const client = getWebSocketClient(); if (!client) return; @@ -297,14 +313,13 @@ function useFileChangeSubscription({ applyFileChanges({ client, sessionId: sessionIdRef.current, - expandedPaths, + expandedPaths: expandedPathsRef.current, changes, setTree, setLoadState, }); }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [expandedPaths]); + }, [sessionIdRef, expandedPathsRef, setTree, setLoadState]); } /** @@ -317,7 +332,23 @@ function useFileChangeSubscription({ export function useFileBrowserTree(sessionId: string, resetKey?: string) { const effectiveResetKey = resetKey ?? sessionId; const [tree, setTree] = useState(null); - const [expandedPaths, setExpandedPaths] = useState>(new Set()); + // Expansion + flat visibleRows come from the shared useTree hook. tree is + // a single root node with children; useTree wants an array, so feed it the + // root's children (pre-sorted — see sortRootChildren). + const treeNodes = useMemo(() => sortRootChildren(tree), [tree]); + const treeApi = useTree({ + nodes: treeNodes, + getPath: FB_GET_PATH, + getChildren: FB_GET_CHILDREN, + isDir: FB_IS_DIR, + }); + const expandedPaths = treeApi.expanded; + const setExpandedPaths = treeApi.setExpanded; + const visibleRows = treeApi.visibleRows; + // Stable ref over `expandedPaths` so the WS file-change subscription does + // not re-attach on every toggle (each toggle mints a new Set reference). + const expandedPathsRef = useRef>(expandedPaths); + expandedPathsRef.current = expandedPaths; const [isLoadingTree, setIsLoadingTree] = useState(true); const [loadState, setLoadState] = useState("loading"); const [loadError, setLoadError] = useState(null); @@ -373,7 +404,7 @@ export function useFileBrowserTree(sessionId: string, resetKey?: string) { return () => { clearRetryTimer(); }; - }, [clearRetryTimer, loadTree, effectiveResetKey, sessionId]); + }, [clearRetryTimer, loadTree, effectiveResetKey, sessionId, setExpandedPaths]); // Fire the initial load on the waiting → ready transition. `loadState` is // read via a ref so a failed load (which sets state back to "waiting") does @@ -400,17 +431,16 @@ export function useFileBrowserTree(sessionId: string, resetKey?: string) { } }, [expandedPaths, effectiveResetKey]); - useFileChangeSubscription({ sessionIdRef, expandedPaths, setTree, setLoadState }); + useFileChangeSubscription({ sessionIdRef, expandedPathsRef, setTree, setLoadState }); - const collapseAll = useCallback(() => { - setExpandedPaths(new Set()); - }, []); + const collapseAll = treeApi.collapseAll; return { tree, setTree, expandedPaths, setExpandedPaths, + visibleRows, visibleLoadingPaths, isLoadingTree, loadState, diff --git a/apps/web/components/task/file-browser-parts.tsx b/apps/web/components/task/file-browser-parts.tsx index 47bf031516..34bc2d6404 100644 --- a/apps/web/components/task/file-browser-parts.tsx +++ b/apps/web/components/task/file-browser-parts.tsx @@ -12,9 +12,9 @@ import { cn } from "@/lib/utils"; import { FileIcon } from "@/components/ui/file-icon"; import type { FileTreeNode } from "@/lib/types/backend"; import type { FileInfo } from "@/lib/state/store"; +import type { FileBrowserRow } from "./file-browser-hooks"; import { InlineFileInput } from "./inline-file-input"; import { renderSessionOrLoadState } from "./file-browser-load-state"; -import { compareTreeNodes } from "./file-tree-utils"; import { FileContextMenu, useFileRename, @@ -32,22 +32,17 @@ export { type GitFileStatus = FileInfo["status"] | undefined; -type TreeNodeItemProps = { - node: FileTreeNode; - depth: number; - expandedPaths: Set; +type TreeNodeRowProps = { + row: FileBrowserRow; activeFolderPath: string; activeFilePath?: string | null; visibleLoadingPaths: Set; - creatingInPath: string | null; fileStatuses: Map; tree: FileTreeNode | null; onToggleExpand: (node: FileTreeNode) => void; onOpenFile: (path: string) => void; onDeleteFile?: (path: string) => Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; - onCreateFileSubmit: (parentPath: string, name: string) => void; - onCancelCreate: () => void; setTree: React.Dispatch>; isSelectedFn?: (path: string) => boolean; onSelect?: (path: string, e: React.MouseEvent) => boolean; @@ -119,28 +114,6 @@ function TreeNodeFileIcon({ ); } -/** Expanded directory children */ -function TreeNodeChildren({ props, depth }: { props: TreeNodeItemProps; depth: number }) { - const { node, creatingInPath, onCreateFileSubmit, onCancelCreate } = props; - return ( -
- {creatingInPath === node.path && ( - onCreateFileSubmit(node.path, name)} - onCancel={onCancelCreate} - /> - )} - {node.children && - [...node.children] - .sort(compareTreeNodes) - .map((child) => ( - - ))} -
- ); -} - function getTreeNodeRowClass( isActive: boolean, isActiveFolder: boolean, @@ -160,13 +133,13 @@ function getTreeNodeRowClass( ); } -export function TreeNodeItem(props: TreeNodeItemProps) { - const { node, depth, expandedPaths, activeFolderPath, activeFilePath, visibleLoadingPaths } = - props; +export function TreeNodeItem(props: TreeNodeRowProps) { + const { row, activeFolderPath, activeFilePath, visibleLoadingPaths } = props; const { fileStatuses, tree, onToggleExpand, onOpenFile, onDeleteFile, onRenameFile, setTree } = props; + const node = row.node; - const isExpanded = expandedPaths.has(node.path); + const isExpanded = row.isExpanded; const isActive = !node.is_dir && activeFilePath === node.path; const isActiveFolder = node.is_dir && activeFolderPath === node.path; const gitStatus = node.is_dir ? undefined : fileStatuses.get(node.path); @@ -198,7 +171,7 @@ export function TreeNodeItem(props: TreeNodeItemProps) { props.isDragging, isDropTarget, )} - style={{ paddingLeft: treeNodePaddingLeft(depth, node.is_dir) }} + style={{ paddingLeft: treeNodePaddingLeft(row.depth, node.is_dir) }} onClick={handleClick} draggable={!!props.onDragStart} onDragStart={(e) => props.onDragStart?.(node.path, e)} @@ -225,21 +198,18 @@ export function TreeNodeItem(props: TreeNodeItemProps) { ); return ( -
- - {rowContent} - - {node.is_dir && isExpanded && } -
+ + {rowContent} + ); } @@ -310,7 +280,7 @@ type FileBrowserContentAreaProps = { loadError: string | null; creatingInPath: string | null; fileStatuses: Map; - expandedPaths: Set; + visibleRows: FileBrowserRow[]; activeFolderPath: string; activeFilePath?: string | null; visibleLoadingPaths: Set; @@ -335,8 +305,35 @@ type FileBrowserContentAreaProps = { selectedPaths?: Set; }; +function rowToItemProps(props: FileBrowserContentAreaProps, row: FileBrowserRow): TreeNodeRowProps { + return { + row, + activeFolderPath: props.activeFolderPath, + activeFilePath: props.activeFilePath, + visibleLoadingPaths: props.visibleLoadingPaths, + fileStatuses: props.fileStatuses, + tree: props.tree, + onToggleExpand: props.onToggleExpand, + onOpenFile: props.onOpenFile, + onDeleteFile: props.onDeleteFile, + onRenameFile: props.onRenameFile, + setTree: props.setTree, + isSelectedFn: props.isSelectedFn, + onSelect: props.onSelect, + isDragging: props.isDragging, + dragOverPath: props.dragOverPath, + onDragStart: props.onDragStart, + onDragEnd: props.onDragEnd, + onDragOver: props.onDragOver, + onDragLeave: props.onDragLeave, + onDrop: props.onDrop, + selectedCount: props.selectedCount, + selectedPaths: props.selectedPaths, + }; +} + function FileTreeView(props: FileBrowserContentAreaProps) { - const { tree, creatingInPath, onCreateFileSubmit, onCancelCreate } = props; + const { tree, visibleRows, creatingInPath, onCreateFileSubmit, onCancelCreate } = props; if (!tree) return null; return (
@@ -347,41 +344,18 @@ function FileTreeView(props: FileBrowserContentAreaProps) { onCancel={onCancelCreate} /> )} - {tree.children && - [...tree.children] - .sort(compareTreeNodes) - .map((child) => ( - ( + + + {creatingInPath === row.path && row.isDir && row.isExpanded && ( + onCreateFileSubmit(row.path, name)} + onCancel={onCancelCreate} /> - ))} + )} + + ))}
); } diff --git a/apps/web/components/task/file-browser.tsx b/apps/web/components/task/file-browser.tsx index 5673e02c57..01b40a8de7 100644 --- a/apps/web/components/task/file-browser.tsx +++ b/apps/web/components/task/file-browser.tsx @@ -442,7 +442,7 @@ export function FileBrowser({ loadError={treeState.loadError} creatingInPath={handlers.creatingInPath} fileStatuses={fileStatuses} - expandedPaths={treeState.expandedPaths} + visibleRows={treeState.visibleRows} activeFolderPath={handlers.activeFolderPath} activeFilePath={activeFilePath} visibleLoadingPaths={treeState.visibleLoadingPaths} diff --git a/apps/web/components/task/file-tree-utils.test.ts b/apps/web/components/task/file-tree-utils.test.ts index cfde195681..14f1f08632 100644 --- a/apps/web/components/task/file-tree-utils.test.ts +++ b/apps/web/components/task/file-tree-utils.test.ts @@ -5,6 +5,7 @@ import { computeMoveTargets, findNodeByPath, getVisiblePaths, + sortRootChildren, } from "./file-tree-utils"; function file(name: string, parentPath = ""): FileTreeNode { @@ -87,6 +88,38 @@ describe("moveNodesInTree", () => { }); }); +describe("sortRootChildren", () => { + it("orders directories before files at the root, alpha within each group", () => { + // Backend can return root children in any order — interleaved dirs/files + // and not necessarily alphabetical. The file browser feeds this list + // straight into useTree, which never re-sorts its top-level `nodes`, so + // this helper is the only thing keeping dirs on top at depth 0. + const tree = root([ + file("README.md"), + dir("zeta", []), + file(".babelrc"), + dir("alpha", []), + file("package.json"), + dir("src", []), + ]); + // compareTreeNodes uses localeCompare, which is case-insensitive by + // default — "package" (p) sorts before "README" (R/r). + expect(sortRootChildren(tree).map((n) => n.name)).toEqual([ + "alpha", + "src", + "zeta", + ".babelrc", + "package.json", + "README.md", + ]); + }); + + it("returns an empty array for a null tree or a tree with no children", () => { + expect(sortRootChildren(null)).toEqual([]); + expect(sortRootChildren({ name: "", path: "", is_dir: true })).toEqual([]); + }); +}); + describe("getVisiblePaths", () => { it("returns all nodes in DFS order including directories", () => { const tree = root([dir("lib", [file("config.ts", "lib")], ""), file("readme.md")]); diff --git a/apps/web/components/task/file-tree-utils.ts b/apps/web/components/task/file-tree-utils.ts index 8b61b34b82..467d8f2194 100644 --- a/apps/web/components/task/file-tree-utils.ts +++ b/apps/web/components/task/file-tree-utils.ts @@ -6,6 +6,17 @@ export const compareTreeNodes = (a: FileTreeNode, b: FileTreeNode): number => { return a.name.localeCompare(b.name); }; +/** + * Return the tree's root children sorted dirs-first / alpha. useTree iterates + * its top-level `nodes` directly (it only invokes `getChildren` to descend), + * so the root list must be pre-sorted by the caller — otherwise backend order + * leaks through and dirs interleave with files at depth 0. + */ +export function sortRootChildren(tree: FileTreeNode | null): FileTreeNode[] { + if (!tree?.children) return []; + return [...tree.children].sort(compareTreeNodes); +} + /** * Merge a freshly-fetched tree node into an existing one, preserving * already-loaded children so expanded folders don't collapse. @@ -89,7 +100,7 @@ export function renameNodeInTree( } /** Collect visible (expanded) node paths in DFS order for multi-select range computation. */ -export function getVisiblePaths(tree: FileTreeNode, expandedPaths: Set): string[] { +export function getVisiblePaths(tree: FileTreeNode, expandedPaths: ReadonlySet): string[] { const result: string[] = []; function walk(node: FileTreeNode) { // Skip the root node itself (it represents the workspace root) diff --git a/apps/web/e2e/tests/task/file-tree-create.spec.ts b/apps/web/e2e/tests/task/file-tree-create.spec.ts new file mode 100644 index 0000000000..99a8692c6f --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-create.spec.ts @@ -0,0 +1,188 @@ +import { type Page } from "@playwright/test"; +import fs from "node:fs"; +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../../helpers/git-helper"; + +// File creation lives in file-browser-toolbar.tsx ("New file" button) + +// inline-file-input.tsx (InlineFileInput) + file-browser.tsx +// (handleStartCreate / handleCreateFileSubmit). +// Behaviour to lock in: +// - "New file" at the tree root creates a file at root +// - After expanding a folder, "New file" creates inside that folder +// - Typing a path-like name (subdir/file.ts) creates implicit parent folders +// - Escape cancels the input +// There is no separate "create folder" affordance today; users create folders +// implicitly by including a "/" in the new-file name. + +async function setupTask( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; workflowId: string; startStepId: string; repositoryId: string }, + profileName: string, + taskTitle: string, +) { + const profile = await createStandardProfile(apiClient, profileName); + await apiClient.createTaskWithAgent(seedData.workspaceId, taskTitle, profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, taskTitle); + await session.clickTab("Files"); + return session; +} + +async function startCreateAtRoot(testPage: Page) { + const btn = testPage.getByRole("button", { name: "New file" }); + await expect(btn).toBeVisible({ timeout: 15_000 }); + await btn.click(); + const input = testPage.getByPlaceholder("filename..."); + await expect(input).toBeVisible({ timeout: 5_000 }); + await expect(input).toBeFocused({ timeout: 2_000 }); + return input; +} + +test.describe("File tree create file", () => { + test("New file at root creates a file on disk and in the tree", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + // Seed at least one file so the tree loads. Without any files the tree + // shows "No files found" instead of the toolbar. + git.createFile("seed.ts", "seed"); + git.stageAll(); + git.commit("seed"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-create-root", + "FT Create Root", + ); + + await expect(session.fileTreeNode("seed.ts")).toBeVisible({ timeout: 15_000 }); + + const input = await startCreateAtRoot(testPage); + await input.fill("brand-new.ts"); + await input.press("Enter"); + + await expect(session.fileTreeNode("brand-new.ts")).toBeVisible({ timeout: 10_000 }); + await expect + .poll(() => fs.existsSync(path.join(repoDir, "brand-new.ts")), { timeout: 10_000 }) + .toBe(true); + }); + + test("New file inside expanded folder creates the file in that folder", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("scope/existing.ts", "x"); + git.stageAll(); + git.commit("seed scope"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-create-folder", + "FT Create In Folder", + ); + + // Expand the folder so it becomes the "active folder" for handleStartCreate. + const folder = session.fileTreeNode("scope"); + await expect(folder).toBeVisible({ timeout: 15_000 }); + await folder.click(); + await expect(session.fileTreeNode("scope/existing.ts")).toBeVisible({ timeout: 10_000 }); + + await testPage.getByRole("button", { name: "New file" }).click(); + const input = testPage.getByPlaceholder("filename..."); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.fill("inside.ts"); + await input.press("Enter"); + + await expect(session.fileTreeNode("scope/inside.ts")).toBeVisible({ timeout: 10_000 }); + await expect + .poll(() => fs.existsSync(path.join(repoDir, "scope", "inside.ts")), { timeout: 10_000 }) + .toBe(true); + }); + + test("typing path-like name creates implicit parent folder", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("seed.ts", "seed"); + git.stageAll(); + git.commit("seed"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-create-implicit", + "FT Create Implicit Folder", + ); + + await expect(session.fileTreeNode("seed.ts")).toBeVisible({ timeout: 15_000 }); + + const input = await startCreateAtRoot(testPage); + await input.fill("newdir/leaf.ts"); + await input.press("Enter"); + + // The file appears under the new folder on disk. + await expect + .poll(() => fs.existsSync(path.join(repoDir, "newdir", "leaf.ts")), { timeout: 10_000 }) + .toBe(true); + }); + + test("Escape cancels the inline input without creating a file", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("seed.ts", "seed"); + git.stageAll(); + git.commit("seed"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-create-cancel", + "FT Create Cancel", + ); + + await expect(session.fileTreeNode("seed.ts")).toBeVisible({ timeout: 15_000 }); + + const input = await startCreateAtRoot(testPage); + await input.fill("ghost.ts"); + await input.press("Escape"); + + await expect(testPage.getByPlaceholder("filename...")).toHaveCount(0, { timeout: 5_000 }); + await expect(session.fileTreeNode("ghost.ts")).toHaveCount(0); + expect(fs.existsSync(path.join(repoDir, "ghost.ts"))).toBe(false); + }); +}); diff --git a/apps/web/e2e/tests/task/file-tree-drag-drop.spec.ts b/apps/web/e2e/tests/task/file-tree-drag-drop.spec.ts new file mode 100644 index 0000000000..4eaa5f9663 --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-drag-drop.spec.ts @@ -0,0 +1,157 @@ +import { type Page, type Locator } from "@playwright/test"; +import fs from "node:fs"; +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../../helpers/git-helper"; + +// DnD in file-browser.tsx uses native HTML5 drag events (dragstart, dragover, +// drop) keyed off React's onDragStart/Over/Drop. Playwright's locator.dragTo() +// does not trigger native HTML5 DnD reliably in Chromium - the drop target +// must see dragover with preventDefault() and a drop event with the same +// DataTransfer that was set in dragstart. +// +// We dispatch the events manually via page.evaluate(), constructing a shared +// DataTransfer for the dragstart -> drop sequence. This is the established +// workaround for testing HTML5 DnD in Playwright and mirrors what the user +// would do. + +async function setupTask( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; workflowId: string; startStepId: string; repositoryId: string }, + profileName: string, + taskTitle: string, +) { + const profile = await createStandardProfile(apiClient, profileName); + await apiClient.createTaskWithAgent(seedData.workspaceId, taskTitle, profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, taskTitle); + await session.clickTab("Files"); + return session; +} + +async function dispatchHtmlDnd(testPage: Page, source: Locator, target: Locator) { + // Make sure both are attached and visible. + await source.scrollIntoViewIfNeeded(); + await target.scrollIntoViewIfNeeded(); + const sourceHandle = await source.elementHandle(); + const targetHandle = await target.elementHandle(); + if (!sourceHandle || !targetHandle) throw new Error("DnD: source/target missing"); + // Real browsers reuse the same DataTransfer across dragstart -> drop. We + // do the whole sequence inside one evaluate() so the instance is shared. + // dragover gets explicit preventDefault() because that's the contract + // React's onDragOver fulfils when the drop is valid - without it, the + // browser would interpret the drop as a navigation attempt for any + // text-typed data and unload the page. + await testPage.evaluate( + ([src, dst]) => { + const dt = new DataTransfer(); + const fireOn = (el: Element, type: string) => { + const ev = new DragEvent(type, { + bubbles: true, + cancelable: true, + composed: true, + dataTransfer: dt, + }); + el.dispatchEvent(ev); + return ev; + }; + fireOn(src, "dragstart"); + fireOn(dst, "dragenter"); + fireOn(dst, "dragover"); + fireOn(dst, "drop"); + // Source may have been removed from the DOM by the optimistic move - + // guard the dragend call. + if (src.isConnected) fireOn(src, "dragend"); + }, + [sourceHandle, targetHandle] as const, + ); +} + +test.describe("File tree drag and drop", () => { + test("drag a file into a folder moves it on disk and in the tree", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("movable.ts", "m"); + git.createFile("target-dir/keep.ts", "k"); + git.stageAll(); + git.commit("seed dnd"); + + const session = await setupTask(testPage, apiClient, seedData, "ft-dnd-move", "FT DnD Move"); + + const file = session.fileTreeNode("movable.ts"); + const folder = session.fileTreeNode("target-dir"); + await expect(file).toBeVisible({ timeout: 15_000 }); + await expect(folder).toBeVisible({ timeout: 15_000 }); + + await dispatchHtmlDnd(testPage, file, folder); + + // The file is removed from the root immediately (optimistic update). + await expect(session.fileTreeNode("movable.ts")).toHaveCount(0, { timeout: 10_000 }); + // Expand the target folder to verify the moved child landed inside. + // moveNodesInTree does not auto-expand the drop target. + await folder.click(); + await expect(session.fileTreeNode("target-dir/movable.ts")).toBeVisible({ timeout: 10_000 }); + + await expect + .poll(() => fs.existsSync(path.join(repoDir, "target-dir", "movable.ts")), { + timeout: 10_000, + }) + .toBe(true); + expect(fs.existsSync(path.join(repoDir, "movable.ts"))).toBe(false); + }); + + test("drop is rejected when dragging a folder onto itself", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("selfdir/leaf.ts", "leaf"); + git.stageAll(); + git.commit("seed selfdir"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-dnd-self", + "FT DnD Self Reject", + ); + + const folder = session.fileTreeNode("selfdir"); + await expect(folder).toBeVisible({ timeout: 15_000 }); + + // Drop onto self: handleDragOver short-circuits via isDropInvalid so + // preventDefault is never called, which means the browser would never + // fire drop in real usage. Dispatching events directly bypasses that + // guard, but the drop handler also calls isDropInvalid and bails. + await dispatchHtmlDnd(testPage, folder, folder); + + // Tree is unchanged: folder is still at root with its original child. + await expect(session.fileTreeNode("selfdir")).toBeVisible({ timeout: 5_000 }); + // Expand and confirm the child is still there. + await folder.click(); + await expect(session.fileTreeNode("selfdir/leaf.ts")).toBeVisible({ timeout: 10_000 }); + + // Disk untouched - no self-nested directory created. + expect(fs.existsSync(path.join(repoDir, "selfdir", "selfdir"))).toBe(false); + }); +}); diff --git a/apps/web/e2e/tests/task/file-tree-lazy-load.spec.ts b/apps/web/e2e/tests/task/file-tree-lazy-load.spec.ts new file mode 100644 index 0000000000..00608f0e07 --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-lazy-load.spec.ts @@ -0,0 +1,114 @@ +import { type Page } from "@playwright/test"; +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../../helpers/git-helper"; + +// Children of a directory are loaded lazily when the directory is first expanded +// (see useFileBrowserHandlers.toggleExpand -> loadNodeChildren). The pre-refactor +// behaviour we need to lock in: children rows render *after* a directory click, +// they were not in the DOM before, and re-expanding doesn't refetch synchronously +// (children persist across collapse/expand once loaded). + +async function setupTask( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; workflowId: string; startStepId: string; repositoryId: string }, + profileName: string, + taskTitle: string, +) { + const profile = await createStandardProfile(apiClient, profileName); + await apiClient.createTaskWithAgent(seedData.workspaceId, taskTitle, profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, taskTitle); + await session.clickTab("Files"); + return session; +} + +test.describe("File tree lazy-load on expand", () => { + test("children render only after directory is expanded", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const git = new GitHelper( + path.join(backend.tmpDir, "repos", "e2e-repo"), + makeGitEnv(backend.tmpDir), + ); + git.createFile("lazyfolder/child-a.ts", "a"); + git.createFile("lazyfolder/child-b.ts", "b"); + git.createFile("lazyfolder/nested/deep.ts", "deep"); + git.stageAll(); + git.commit("seed lazy folder"); + + const session = await setupTask(testPage, apiClient, seedData, "ft-lazy-load", "FT Lazy Load"); + + const folder = session.fileTreeNode("lazyfolder"); + await expect(folder).toBeVisible({ timeout: 15_000 }); + + // Pre-expand: children should not be in the DOM yet. + await expect(session.fileTreeNode("lazyfolder/child-a.ts")).toHaveCount(0); + await expect(session.fileTreeNode("lazyfolder/child-b.ts")).toHaveCount(0); + await expect(session.fileTreeNode("lazyfolder/nested")).toHaveCount(0); + + // Expand the folder. + await folder.click(); + + // After expand: direct children render. Nested subfolder appears but its + // own children are not loaded until that subfolder is also expanded. + await expect(session.fileTreeNode("lazyfolder/child-a.ts")).toBeVisible({ timeout: 10_000 }); + await expect(session.fileTreeNode("lazyfolder/child-b.ts")).toBeVisible({ timeout: 10_000 }); + await expect(session.fileTreeNode("lazyfolder/nested")).toBeVisible({ timeout: 10_000 }); + await expect(session.fileTreeNode("lazyfolder/nested/deep.ts")).toHaveCount(0); + + // Expand nested -> its child loads. + await session.fileTreeNode("lazyfolder/nested").click(); + await expect(session.fileTreeNode("lazyfolder/nested/deep.ts")).toBeVisible({ + timeout: 10_000, + }); + }); + + test("collapsing then re-expanding keeps children without refetch", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const git = new GitHelper( + path.join(backend.tmpDir, "repos", "e2e-repo"), + makeGitEnv(backend.tmpDir), + ); + git.createFile("keepfolder/keep-a.ts", "a"); + git.createFile("keepfolder/keep-b.ts", "b"); + git.stageAll(); + git.commit("seed keep folder"); + + const session = await setupTask(testPage, apiClient, seedData, "ft-lazy-keep", "FT Lazy Keep"); + + const folder = session.fileTreeNode("keepfolder"); + await expect(folder).toBeVisible({ timeout: 15_000 }); + + // Expand + await folder.click(); + await expect(session.fileTreeNode("keepfolder/keep-a.ts")).toBeVisible({ timeout: 10_000 }); + + // Collapse - children removed from DOM + await folder.click(); + await expect(session.fileTreeNode("keepfolder/keep-a.ts")).toHaveCount(0, { timeout: 5_000 }); + + // Re-expand - children come back quickly (already cached in tree state). + await folder.click(); + await expect(session.fileTreeNode("keepfolder/keep-a.ts")).toBeVisible({ timeout: 5_000 }); + await expect(session.fileTreeNode("keepfolder/keep-b.ts")).toBeVisible({ timeout: 5_000 }); + }); +}); diff --git a/apps/web/e2e/tests/task/file-tree-rename.spec.ts b/apps/web/e2e/tests/task/file-tree-rename.spec.ts new file mode 100644 index 0000000000..6228a04477 --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-rename.spec.ts @@ -0,0 +1,196 @@ +import { type Page } from "@playwright/test"; +import path from "node:path"; +import fs from "node:fs"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../../helpers/git-helper"; + +// Inline rename lives in file-context-menu.tsx (useFileRename + TreeNodeName). +// Entry points (today, in product code): +// - Right-click -> "Rename" menu item +// - The input is given focus 150ms after isRenaming=true and blur-commit is +// gated by a 400ms ref so the initial focus race doesn't fire onBlur. +// Commit on Enter, cancel on Escape, commit on blur. +// We test the user-visible flow only (no direct DOM hacks), so the 400ms +// blur gate is exercised implicitly. + +async function setupTask( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; workflowId: string; startStepId: string; repositoryId: string }, + profileName: string, + taskTitle: string, +) { + const profile = await createStandardProfile(apiClient, profileName); + await apiClient.createTaskWithAgent(seedData.workspaceId, taskTitle, profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, taskTitle); + await session.clickTab("Files"); + return session; +} + +async function startRenameViaContextMenu(testPage: Page, node: ReturnType) { + await node.click({ button: "right" }); + const renameItem = testPage.getByRole("menuitem", { name: "Rename" }); + await expect(renameItem).toBeVisible({ timeout: 5_000 }); + await renameItem.click(); + // The input is auto-focused ~150ms after isRenaming flips. The Input inside + // the row is the only one with this className combo; locate via role. + const input = node.getByRole("textbox"); + await expect(input).toBeVisible({ timeout: 2_000 }); + await expect(input).toBeFocused({ timeout: 2_000 }); + return input; +} + +test.describe("File tree inline rename", () => { + test("rename commits on Enter and the new file appears on disk", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("rename-me.ts", "hello"); + git.stageAll(); + git.commit("seed rename file"); + + const session = await setupTask(testPage, apiClient, seedData, "ft-rename", "FT Rename Enter"); + + const node = session.fileTreeNode("rename-me.ts"); + await expect(node).toBeVisible({ timeout: 15_000 }); + + const input = await startRenameViaContextMenu(testPage, node); + // Select-all then type the new name (the hook also calls .select() but + // doing it explicitly is robust against focus-timing edge cases). + await input.press("ControlOrMeta+A"); + await input.fill("renamed.ts"); + await input.press("Enter"); + + // Old node disappears from the tree, new node appears. + await expect(session.fileTreeNode("rename-me.ts")).toHaveCount(0, { timeout: 10_000 }); + await expect(session.fileTreeNode("renamed.ts")).toBeVisible({ timeout: 10_000 }); + + // And the rename hit the file system. + await expect + .poll(() => fs.existsSync(path.join(repoDir, "renamed.ts")), { timeout: 10_000 }) + .toBe(true); + expect(fs.existsSync(path.join(repoDir, "rename-me.ts"))).toBe(false); + }); + + test("rename cancels on Escape and leaves the tree and disk untouched", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("keep-name.ts", "stay"); + git.stageAll(); + git.commit("seed keep file"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-rename-esc", + "FT Rename Escape", + ); + + const node = session.fileTreeNode("keep-name.ts"); + await expect(node).toBeVisible({ timeout: 15_000 }); + + const input = await startRenameViaContextMenu(testPage, node); + await input.press("ControlOrMeta+A"); + await input.fill("nope.ts"); + await input.press("Escape"); + + // No mutation - original node still present, no renamed node, disk unchanged. + await expect(session.fileTreeNode("keep-name.ts")).toBeVisible({ timeout: 5_000 }); + await expect(session.fileTreeNode("nope.ts")).toHaveCount(0); + expect(fs.existsSync(path.join(repoDir, "keep-name.ts"))).toBe(true); + expect(fs.existsSync(path.join(repoDir, "nope.ts"))).toBe(false); + }); + + test("rename commits on blur after typing a new name", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("blur-original.ts", "blur"); + git.createFile("other.ts", "other"); + git.stageAll(); + git.commit("seed blur file"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-rename-blur", + "FT Rename Blur", + ); + + const node = session.fileTreeNode("blur-original.ts"); + await expect(node).toBeVisible({ timeout: 15_000 }); + + const input = await startRenameViaContextMenu(testPage, node); + await input.press("ControlOrMeta+A"); + await input.fill("blur-final.ts"); + // The blur gate is ~400ms after isRenaming flips. Wait that out before + // moving focus elsewhere so the blur handler actually fires the commit. + await testPage.waitForTimeout(500); + // Click another file to blur the input. The other node also belongs to + // the tree, so we don't lose tree-container focus state. + await session.fileTreeNode("other.ts").click(); + + await expect(session.fileTreeNode("blur-final.ts")).toBeVisible({ timeout: 10_000 }); + await expect(session.fileTreeNode("blur-original.ts")).toHaveCount(0); + await expect + .poll(() => fs.existsSync(path.join(repoDir, "blur-final.ts")), { timeout: 10_000 }) + .toBe(true); + }); + + test("rename is a no-op when the name is unchanged", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("noop.ts", "noop"); + git.stageAll(); + git.commit("seed noop"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-rename-noop", + "FT Rename NoOp", + ); + + const node = session.fileTreeNode("noop.ts"); + await expect(node).toBeVisible({ timeout: 15_000 }); + + const input = await startRenameViaContextMenu(testPage, node); + // Don't change anything, just press Enter. + await input.press("Enter"); + + await expect(session.fileTreeNode("noop.ts")).toBeVisible({ timeout: 5_000 }); + expect(fs.existsSync(path.join(repoDir, "noop.ts"))).toBe(true); + }); +}); diff --git a/apps/web/e2e/tests/task/file-tree-search.spec.ts b/apps/web/e2e/tests/task/file-tree-search.spec.ts new file mode 100644 index 0000000000..8d44268fd1 --- /dev/null +++ b/apps/web/e2e/tests/task/file-tree-search.spec.ts @@ -0,0 +1,147 @@ +import { type Page } from "@playwright/test"; +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../../helpers/git-helper"; + +// File panel search lives in file-browser-search-header.tsx (input) + +// file-browser-hooks.ts (useFileBrowserSearch with 300ms debounce + WS call +// to searchWorkspaceFiles). Behaviour to lock in: +// - Clicking the Search files toolbar button reveals the input +// - Typing a query produces results that include matching files anywhere +// in the tree (including inside non-expanded folders) +// - Clearing the input goes back to the tree view +// - Escape closes search + +async function setupTask( + testPage: Page, + apiClient: ApiClient, + seedData: { workspaceId: string; workflowId: string; startStepId: string; repositoryId: string }, + profileName: string, + taskTitle: string, +) { + const profile = await createStandardProfile(apiClient, profileName); + await apiClient.createTaskWithAgent(seedData.workspaceId, taskTitle, profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, taskTitle); + await session.clickTab("Files"); + return session; +} + +test.describe("File tree search", () => { + test("search reveals matches inside collapsed folders", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("search-alpha.ts", "a"); + // Match lives inside a folder we never expand. Search must still find it. + git.createFile("deep/nested/needle-target.ts", "needle"); + git.createFile("other-search/sibling.ts", "s"); + git.stageAll(); + git.commit("seed search"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-search-collapsed", + "FT Search Collapsed", + ); + + await expect(session.fileTreeNode("search-alpha.ts")).toBeVisible({ timeout: 15_000 }); + // Sanity: target file's tree row is not in the DOM (folder is collapsed). + await expect(session.fileTreeNode("deep/nested/needle-target.ts")).toHaveCount(0); + + await testPage.getByRole("button", { name: "Search files" }).click(); + const input = testPage.getByPlaceholder("Search files..."); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.fill("needle-target"); + + // Search panel shows the matching path even though the parent folders + // are not expanded in the tree. + const match = session.files.getByText("needle-target.ts", { exact: false }); + await expect(match).toBeVisible({ timeout: 5_000 }); + // The folder path should appear next to the name. + await expect(session.files.getByText("deep/nested/", { exact: false })).toBeVisible(); + }); + + test("clearing the search input restores the tree view", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("clear-alpha.ts", "a"); + git.createFile("clear-bravo.ts", "b"); + git.stageAll(); + git.commit("seed clear"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-search-clear", + "FT Search Clear", + ); + + await expect(session.fileTreeNode("clear-alpha.ts")).toBeVisible({ timeout: 15_000 }); + await testPage.getByRole("button", { name: "Search files" }).click(); + const input = testPage.getByPlaceholder("Search files..."); + await input.fill("clear-alpha"); + // Wait for the debounced search and the filtered list to settle. + await expect(session.files.getByText("clear-alpha.ts", { exact: false })).toBeVisible({ + timeout: 5_000, + }); + + // Clear input - tree comes back, both files visible again. + await input.fill(""); + await expect(session.fileTreeNode("clear-alpha.ts")).toBeVisible({ timeout: 5_000 }); + await expect(session.fileTreeNode("clear-bravo.ts")).toBeVisible({ timeout: 5_000 }); + }); + + test("Escape closes the search header and returns to the tree", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("escape-target.ts", "e"); + git.stageAll(); + git.commit("seed escape"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-search-escape", + "FT Search Escape", + ); + + await expect(session.fileTreeNode("escape-target.ts")).toBeVisible({ timeout: 15_000 }); + await testPage.getByRole("button", { name: "Search files" }).click(); + const input = testPage.getByPlaceholder("Search files..."); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.press("Escape"); + + await expect(input).toHaveCount(0, { timeout: 5_000 }); + // Toolbar comes back - the search button is once again accessible. + await expect(testPage.getByRole("button", { name: "Search files" })).toBeVisible(); + }); +}); diff --git a/apps/web/hooks/use-tree-keyboard-nav.test.ts b/apps/web/hooks/use-tree-keyboard-nav.test.ts new file mode 100644 index 0000000000..a46168ea64 --- /dev/null +++ b/apps/web/hooks/use-tree-keyboard-nav.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useTree } from "./use-tree"; +import { useTreeKeyboardNav } from "./use-tree-keyboard-nav"; + +interface TestNode { + path: string; + isDir: boolean; + children?: TestNode[]; +} + +const getPath = (n: TestNode) => n.path; +const getChildren = (n: TestNode) => n.children; +const isDir = (n: TestNode) => n.isDir; + +const SRC = "src"; +const SRC_UI = "src/ui"; +const SRC_API = "src/api"; +const BUTTON = "src/ui/Button.tsx"; +const CARD = "src/ui/Card.tsx"; +const USERS = "src/api/users.ts"; +const README = "README.md"; + +const TREE: TestNode[] = [ + { + path: SRC, + isDir: true, + children: [ + { + path: SRC_UI, + isDir: true, + children: [ + { path: BUTTON, isDir: false }, + { path: CARD, isDir: false }, + ], + }, + { + path: SRC_API, + isDir: true, + children: [{ path: USERS, isDir: false }], + }, + ], + }, + { path: README, isDir: false }, +]; + +/** + * Renders useTree + useTreeKeyboardNav together. Use the returned `result` + * to drive arrow keys and read focused-path state. + */ +function setup(defaultExpanded?: "all" | string[]) { + const onActivate = vi.fn(); + const { result } = renderHook(() => { + const tree = useTree({ + nodes: TREE, + getPath, + getChildren, + isDir, + defaultExpanded, + }); + const nav = useTreeKeyboardNav({ + visibleRows: tree.visibleRows, + toggle: tree.toggle, + expand: tree.expand, + collapse: tree.collapse, + isExpanded: tree.isExpanded, + onActivate, + }); + return { tree, nav }; + }); + return { result, onActivate }; +} + +describe("useTreeKeyboardNav", () => { + it("ArrowDown moves focus to the first row when nothing is focused", () => { + const { result } = setup(); + act(() => result.current.nav.dispatchKey("ArrowDown")); + expect(result.current.nav.focusedPath).toBe(SRC); + }); + + it("ArrowDown advances by one visible row, ArrowUp goes back", () => { + const { result } = setup("all"); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowDown")); // src/ui + act(() => result.current.nav.dispatchKey("ArrowDown")); // src/ui/Button.tsx + expect(result.current.nav.focusedPath).toBe(BUTTON); + act(() => result.current.nav.dispatchKey("ArrowUp")); + expect(result.current.nav.focusedPath).toBe(SRC_UI); + }); + + it("ArrowDown clamps at the last row", () => { + const { result } = setup(); + // 2 rows visible: src, README.md + act(() => result.current.nav.dispatchKey("ArrowDown")); + act(() => result.current.nav.dispatchKey("ArrowDown")); + act(() => result.current.nav.dispatchKey("ArrowDown")); + expect(result.current.nav.focusedPath).toBe(README); + }); + + it("ArrowUp clamps at the first row", () => { + const { result } = setup(); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowUp")); + act(() => result.current.nav.dispatchKey("ArrowUp")); + expect(result.current.nav.focusedPath).toBe(SRC); + }); + + it("ArrowRight on a collapsed dir expands it without moving focus", () => { + const { result } = setup(); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowRight")); + expect(result.current.tree.isExpanded(SRC)).toBe(true); + expect(result.current.nav.focusedPath).toBe(SRC); + }); + + it("ArrowRight on an expanded dir moves focus to the next row", () => { + const { result } = setup("all"); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowRight")); + expect(result.current.nav.focusedPath).toBe(SRC_UI); + }); + + it("ArrowLeft on an expanded dir collapses it", () => { + const { result } = setup("all"); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowLeft")); + expect(result.current.tree.isExpanded(SRC)).toBe(false); + expect(result.current.nav.focusedPath).toBe(SRC); + }); + + it("ArrowLeft on a leaf moves focus to its parent dir", () => { + const { result } = setup("all"); + // Walk down to Button.tsx + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("ArrowDown")); // src/ui + act(() => result.current.nav.dispatchKey("ArrowDown")); // Button.tsx + expect(result.current.nav.focusedPath).toBe(BUTTON); + act(() => result.current.nav.dispatchKey("ArrowLeft")); + expect(result.current.nav.focusedPath).toBe(SRC_UI); + }); + + it("Enter on a dir toggles it; Enter on a file fires onActivate", () => { + const { result, onActivate } = setup("all"); + act(() => result.current.nav.dispatchKey("ArrowDown")); // src + act(() => result.current.nav.dispatchKey("Enter")); + expect(result.current.tree.isExpanded(SRC)).toBe(false); + + // Re-expand and walk to README. + act(() => result.current.tree.expandAll()); + // Focus on README (last row) + act(() => result.current.nav.setFocusedPath(README)); + act(() => result.current.nav.dispatchKey("Enter")); + expect(onActivate).toHaveBeenCalledTimes(1); + expect(onActivate.mock.calls[0][0].path).toBe(README); + }); + + it("Space activates the same way as Enter", () => { + const { result, onActivate } = setup("all"); + act(() => result.current.nav.setFocusedPath(BUTTON)); + act(() => result.current.nav.dispatchKey(" ")); + expect(onActivate).toHaveBeenCalledTimes(1); + expect(onActivate.mock.calls[0][0].path).toBe(BUTTON); + }); + + it("ignores unrelated keys and reports no effect", () => { + const { result, onActivate } = setup(); + act(() => result.current.nav.setFocusedPath(SRC)); + act(() => result.current.nav.dispatchKey("a")); + expect(onActivate).not.toHaveBeenCalled(); + expect(result.current.nav.focusedPath).toBe(SRC); + }); +}); diff --git a/apps/web/hooks/use-tree-keyboard-nav.ts b/apps/web/hooks/use-tree-keyboard-nav.ts new file mode 100644 index 0000000000..3ff00bca2e --- /dev/null +++ b/apps/web/hooks/use-tree-keyboard-nav.ts @@ -0,0 +1,144 @@ +"use client"; + +import { useCallback, useState, type KeyboardEvent as ReactKeyboardEvent } from "react"; +import type { VisibleRow } from "./use-tree"; + +/** + * Keyboard navigation on a useTree's flat visibleRows list: + * - ArrowDown / ArrowUp move focus by one visible row. + * - ArrowRight expands a collapsed dir; for an already-expanded dir or a file, + * moves to the next row. + * - ArrowLeft collapses an expanded dir; for a collapsed dir or a file, + * moves to the parent row (first row above whose depth is smaller). + * - Enter (and Space) activates: dir toggles, file fires onActivate. + * + * Focus state is owned by this hook. Consumers wire `handleKeyDown` to the + * scrollable container's onKeyDown and read `focusedPath` to highlight a row. + */ + +export interface UseTreeKeyboardNavOptions { + visibleRows: VisibleRow[]; + toggle: (path: string) => void; + expand: (path: string) => void; + collapse: (path: string) => void; + isExpanded: (path: string) => boolean; + /** Called when Enter / Space is pressed on a file row. */ + onActivate?: (row: VisibleRow) => void; +} + +export interface UseTreeKeyboardNavResult { + focusedPath: string | null; + setFocusedPath: (path: string | null) => void; + handleKeyDown: (e: ReactKeyboardEvent) => void; + /** Equivalent of running `handleKeyDown` programmatically — useful in tests. */ + dispatchKey: (key: string) => void; + visibleRows: VisibleRow[]; +} + +function moveIndex(idx: number, delta: number, max: number): number { + if (idx < 0) return delta > 0 ? 0 : max - 1; + return Math.max(0, Math.min(idx + delta, max - 1)); +} + +function findParentIndex(rows: VisibleRow[], idx: number): number { + const row = rows[idx]; + for (let i = idx - 1; i >= 0; i--) { + if (rows[i].depth < row.depth) return i; + } + return -1; +} + +interface HorizontalCtx { + rows: VisibleRow[]; + idx: number; + isExpanded: (p: string) => boolean; + expand: (p: string) => void; + collapse: (p: string) => void; + setFocusedPath: (p: string | null) => void; +} + +function handleHorizontal(key: "ArrowLeft" | "ArrowRight", ctx: HorizontalCtx): boolean { + const { rows, idx, isExpanded, expand, collapse, setFocusedPath } = ctx; + const row = rows[idx]; + if (!row) return false; + if (key === "ArrowRight") { + if (row.isDir && !isExpanded(row.path)) expand(row.path); + else if (idx + 1 < rows.length) setFocusedPath(rows[idx + 1].path); + return true; + } + if (row.isDir && isExpanded(row.path)) { + collapse(row.path); + return true; + } + const parent = findParentIndex(rows, idx); + if (parent >= 0) setFocusedPath(rows[parent].path); + return true; +} + +function handleActivate( + rows: VisibleRow[], + idx: number, + toggle: (p: string) => void, + onActivate: ((row: VisibleRow) => void) | undefined, +): boolean { + const row = rows[idx]; + if (!row) return false; + if (row.isDir) toggle(row.path); + else onActivate?.(row); + return true; +} + +export function useTreeKeyboardNav( + opts: UseTreeKeyboardNavOptions, +): UseTreeKeyboardNavResult { + const { visibleRows, toggle, expand, collapse, isExpanded, onActivate } = opts; + const [focusedPath, setFocusedPath] = useState(null); + + const apply = useCallback( + (key: string): boolean => { + const max = visibleRows.length; + if (max === 0) return false; + const idx = visibleRows.findIndex((r) => r.path === focusedPath); + + if (key === "ArrowDown") { + setFocusedPath(visibleRows[moveIndex(idx, 1, max)].path); + return true; + } + if (key === "ArrowUp") { + setFocusedPath(visibleRows[moveIndex(idx, -1, max)].path); + return true; + } + if (key === "ArrowLeft" || key === "ArrowRight") { + return handleHorizontal(key, { + rows: visibleRows, + idx, + isExpanded, + expand, + collapse, + setFocusedPath, + }); + } + if (key === "Enter" || key === " ") { + return handleActivate(visibleRows, idx, toggle, onActivate); + } + return false; + }, + [visibleRows, focusedPath, toggle, expand, collapse, isExpanded, onActivate], + ); + + const handleKeyDown = useCallback( + (e: ReactKeyboardEvent) => { + if (apply(e.key)) e.preventDefault(); + }, + [apply], + ); + + const dispatchKey = useCallback( + (key: string) => { + apply(key); + }, + [apply], + ); + + return { focusedPath, setFocusedPath, handleKeyDown, dispatchKey, visibleRows }; +} diff --git a/apps/web/hooks/use-tree.test.ts b/apps/web/hooks/use-tree.test.ts new file mode 100644 index 0000000000..2b33558177 --- /dev/null +++ b/apps/web/hooks/use-tree.test.ts @@ -0,0 +1,275 @@ +import { describe, it, expect } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useTree } from "./use-tree"; + +interface TestNode { + path: string; + isDir: boolean; + children?: TestNode[]; +} + +const getPath = (n: TestNode) => n.path; +const getChildren = (n: TestNode) => n.children; +const isDir = (n: TestNode) => n.isDir; + +function leaf(path: string): TestNode { + return { path, isDir: false }; +} +function dir(path: string, children: TestNode[] = []): TestNode { + return { path, isDir: true, children }; +} + +const SRC = "src"; +const SRC_UI = "src/ui"; +const SRC_API = "src/api"; +const BUTTON_TSX = "src/ui/Button.tsx"; +const CARD_TSX = "src/ui/Card.tsx"; +const INDEX_TS = "src/index.ts"; +const USERS_TS = "src/api/users.ts"; +const README = "README.md"; + +const TREE: TestNode[] = [ + dir(SRC, [dir(SRC_UI, [leaf(BUTTON_TSX), leaf(CARD_TSX)]), leaf(INDEX_TS)]), + leaf(README), +]; + +type Override = Partial>[0]>; + +function renderUseTree(override: Override = {}) { + return renderHook( + (props: Override) => useTree({ nodes: TREE, getPath, getChildren, isDir, ...props }), + { initialProps: override }, + ).result; +} + +describe("useTree — visible rows and expansion", () => { + it("renders only top-level rows when nothing is expanded", () => { + const r = renderUseTree(); + expect(r.current.visibleRows.map((row) => row.path)).toEqual([SRC, README]); + }); + + it("toggle reveals a dir's children", () => { + const r = renderUseTree(); + act(() => r.current.toggle(SRC)); + expect(r.current.visibleRows.map((row) => row.path)).toEqual([SRC, SRC_UI, INDEX_TS, README]); + }); + + it("toggle collapses an already-expanded dir", () => { + const r = renderUseTree(); + act(() => r.current.toggle(SRC)); + act(() => r.current.toggle(SRC)); + expect(r.current.visibleRows.map((row) => row.path)).toEqual([SRC, README]); + }); + + it("expand and collapse are idempotent and explicit", () => { + const r = renderUseTree(); + act(() => r.current.expand(SRC)); + act(() => r.current.expand(SRC)); + expect(r.current.isExpanded(SRC)).toBe(true); + act(() => r.current.collapse(SRC)); + act(() => r.current.collapse(SRC)); + expect(r.current.isExpanded(SRC)).toBe(false); + }); + + it("defaultExpanded='all' pre-expands every dir", () => { + const r = renderUseTree({ defaultExpanded: "all" }); + expect(r.current.visibleRows.map((row) => row.path)).toEqual([ + SRC, + SRC_UI, + BUTTON_TSX, + CARD_TSX, + INDEX_TS, + README, + ]); + }); + + it("defaultExpanded as iterable seeds the expanded set", () => { + const r = renderUseTree({ defaultExpanded: [SRC] }); + expect(r.current.isExpanded(SRC)).toBe(true); + expect(r.current.isExpanded(SRC_UI)).toBe(false); + }); + + it("expandAll walks the current nodes once", () => { + const r = renderUseTree(); + act(() => r.current.expandAll()); + expect(r.current.isExpanded(SRC)).toBe(true); + expect(r.current.isExpanded(SRC_UI)).toBe(true); + }); + + it("collapseAll clears the expanded set", () => { + const r = renderUseTree({ defaultExpanded: "all" }); + act(() => r.current.collapseAll()); + expect(r.current.expanded.size).toBe(0); + }); + + it("expandAncestorsOf adds every ancestor prefix", () => { + const r = renderUseTree(); + act(() => r.current.expandAncestorsOf(BUTTON_TSX)); + expect(r.current.isExpanded(SRC)).toBe(true); + expect(r.current.isExpanded(SRC_UI)).toBe(true); + expect(r.current.isExpanded(BUTTON_TSX)).toBe(false); + }); +}); + +describe("useTree — visible-row depth and metadata", () => { + it("depth is 0 for roots and increments for each level", () => { + const r = renderUseTree({ defaultExpanded: "all" }); + const byPath = Object.fromEntries(r.current.visibleRows.map((row) => [row.path, row.depth])); + expect(byPath[SRC]).toBe(0); + expect(byPath[SRC_UI]).toBe(1); + expect(byPath[BUTTON_TSX]).toBe(2); + expect(byPath[README]).toBe(0); + }); + + it("displayName for non-chain-collapsed rows is the last path segment", () => { + const r = renderUseTree({ defaultExpanded: "all" }); + const byPath = Object.fromEntries( + r.current.visibleRows.map((row) => [row.path, row.displayName]), + ); + expect(byPath[SRC]).toBe(SRC); + expect(byPath[BUTTON_TSX]).toBe("Button.tsx"); + }); +}); + +describe("useTree — chain collapse", () => { + it("merges single-child dir chains into one row with a slash-joined displayName", () => { + const nodes: TestNode[] = [dir("a", [dir("a/b", [dir("a/b/c", [leaf("a/b/c/file.ts")])])])]; + const r = renderHook(() => + useTree({ nodes, getPath, getChildren, isDir, chainCollapse: true }), + ).result; + // Top-level row is "a/b/c" (the whole chain collapsed). It is not expanded + // yet, so the leaf below is hidden. + expect(r.current.visibleRows.map((row) => ({ path: row.path, name: row.displayName }))).toEqual( + [{ path: "a/b/c", name: "a/b/c" }], + ); + }); + + it("does NOT merge when a chain member has multiple children", () => { + const nodes: TestNode[] = [dir("a", [dir("a/b", [leaf("a/b/x.ts"), leaf("a/b/y.ts")])])]; + const r = renderHook(() => + useTree({ + nodes, + getPath, + getChildren, + isDir, + chainCollapse: true, + defaultExpanded: "all", + }), + ).result; + // "a" has one child dir "a/b" — that chain merges. But "a/b" has two + // leaf children, so the chain stops there. + const paths = r.current.visibleRows.map((row) => row.path); + expect(paths).toEqual(["a/b", "a/b/x.ts", "a/b/y.ts"]); + expect(r.current.visibleRows[0].displayName).toBe("a/b"); + }); + + it("expanding a chain-collapsed row uses the deepest node's path", () => { + const nodes: TestNode[] = [dir("a", [dir("a/b", [leaf("a/b/file.ts")])])]; + const r = renderHook(() => + useTree({ nodes, getPath, getChildren, isDir, chainCollapse: true }), + ).result; + // Toggling the chain-row's path "a/b" must expand the deepest dir. + act(() => r.current.toggle("a/b")); + const paths = r.current.visibleRows.map((row) => row.path); + expect(paths).toEqual(["a/b", "a/b/file.ts"]); + }); +}); + +describe("useTree — search modes", () => { + const SEARCH_TREE: TestNode[] = [ + dir(SRC, [dir(SRC_UI, [leaf(BUTTON_TSX), leaf(CARD_TSX)]), dir(SRC_API, [leaf(USERS_TS)])]), + leaf(README), + ]; + + it("hide mode: removes non-matching subtrees and force-expands ancestors of matches", () => { + const r = renderHook(() => + useTree({ + nodes: SEARCH_TREE, + getPath, + getChildren, + isDir, + search: "button", + searchMode: "hide", + }), + ).result; + const paths = r.current.visibleRows.map((row) => row.path); + // src and src/ui are visible because they have a matching descendant; + // src/api and README.md are hidden because nothing under them matches. + // Ancestors are force-expanded so Button.tsx is reachable. + expect(paths).toEqual([SRC, SRC_UI, BUTTON_TSX]); + }); + + it("expand mode: keeps all rows visible, force-expands matching dirs and ancestors", () => { + const r = renderHook(() => + useTree({ + nodes: SEARCH_TREE, + getPath, + getChildren, + isDir, + search: "users", + searchMode: "expand", + }), + ).result; + const paths = r.current.visibleRows.map((row) => row.path); + // src is force-expanded (descendant match), src/api is force-expanded + // (descendant match). src/ui is NOT expanded (no match under it), so its + // children stay hidden. + expect(paths).toEqual([SRC, SRC_UI, SRC_API, USERS_TS, README]); + }); + + it("collapse mode: keeps everything visible, collapses non-matching dirs", () => { + const r = renderHook(() => + useTree({ + nodes: SEARCH_TREE, + getPath, + getChildren, + isDir, + search: "button", + searchMode: "collapse", + defaultExpanded: "all", // start fully open; collapse mode should override + }), + ).result; + const paths = r.current.visibleRows.map((row) => row.path); + // src/api collapses (no match under it); src and src/ui stay expanded + // because there's a match under them. + expect(paths).toEqual([SRC, SRC_UI, BUTTON_TSX, CARD_TSX, SRC_API, README]); + }); + + it("empty search reverts to standard expand behavior", () => { + const r = renderHook(() => + useTree({ + nodes: SEARCH_TREE, + getPath, + getChildren, + isDir, + search: "", + searchMode: "hide", + }), + ).result; + // Nothing expanded, no search — only top level visible. + expect(r.current.visibleRows.map((row) => row.path)).toEqual([SRC, README]); + }); +}); + +describe("useTree — node-shape agnosticism via adapter functions", () => { + it("works with snake_case fields (file-browser's backend shape)", () => { + interface SnakeNode { + path: string; + is_dir: boolean; + children?: SnakeNode[]; + } + const snake: SnakeNode[] = [ + { path: SRC, is_dir: true, children: [{ path: "src/a.ts", is_dir: false }] }, + ]; + const r = renderHook(() => + useTree({ + nodes: snake, + getPath: (n) => n.path, + getChildren: (n) => n.children, + isDir: (n) => n.is_dir, + }), + ).result; + act(() => r.current.toggle(SRC)); + expect(r.current.visibleRows.map((row) => row.path)).toEqual([SRC, "src/a.ts"]); + }); +}); diff --git a/apps/web/hooks/use-tree.ts b/apps/web/hooks/use-tree.ts new file mode 100644 index 0000000000..180ab0f99a --- /dev/null +++ b/apps/web/hooks/use-tree.ts @@ -0,0 +1,325 @@ +"use client"; + +import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; + +/** + * Headless tree state. Owns expansion + derived flat visibleRows + search + * filtering. Selection, lazy-load, drag/drop, rename, etc. are the caller's + * concern — keep this hook small and pure. + * + * Generic over the node type so it can drive every file-tree in the app, + * including ones that use snake_case fields. Adapter functions + * (getPath/getChildren/isDir) decouple the hook from any specific shape. + */ + +export type SearchMode = "hide" | "collapse" | "expand"; + +export interface UseTreeOptions { + nodes: N[]; + getPath: (node: N) => string; + getChildren: (node: N) => N[] | undefined; + isDir: (node: N) => boolean; + /** "all" pre-expands every directory once on mount; iterable seeds the set. */ + defaultExpanded?: "all" | Iterable; + search?: string; + /** + * - "hide": non-matching nodes are removed. Ancestors of matches are kept + * and force-expanded so matches stay reachable. + * - "collapse": every node stays visible. Matching subtrees force-expanded, + * non-matching dirs force-collapsed. + * - "expand": every node stays visible. Matching dirs and their ancestors + * force-expanded; other dirs retain their stored expand state. + * Defaults to "hide". + */ + searchMode?: SearchMode; + /** + * When true, single-child dir chains are merged into one row with a slash- + * joined displayName (e.g. "src/ui"). The effective node is the deepest in + * the chain; expanding it reveals the deepest dir's children. + */ + chainCollapse?: boolean; +} + +export interface VisibleRow { + /** The effective node — the deepest in a collapsed chain, else === chainRoot. */ + node: N; + /** The top of the chain. Same as `node` when chainCollapse is off. */ + chainRoot: N; + /** "src/ui" for chains, else the last path segment. */ + displayName: string; + /** Path of the effective node. */ + path: string; + depth: number; + /** Effective expansion (after applying search-mode overrides). */ + isExpanded: boolean; + isDir: boolean; +} + +export interface UseTreeResult { + visibleRows: VisibleRow[]; + expanded: ReadonlySet; + isExpanded: (path: string) => boolean; + toggle: (path: string) => void; + expand: (path: string) => void; + collapse: (path: string) => void; + expandAll: () => void; + collapseAll: () => void; + /** Adds every ancestor prefix of `path` to the expanded set. */ + expandAncestorsOf: (path: string) => void; + /** Direct setter for the expanded set. Use sparingly — for things like + * rehydrating from persistence on session change. */ + setExpanded: React.Dispatch>>; +} + +function lastSegment(p: string): string { + const i = p.lastIndexOf("/"); + return i < 0 ? p : p.slice(i + 1); +} + +function collectAllDirPaths( + nodes: N[], + isDir: (n: N) => boolean, + getPath: (n: N) => string, + getChildren: (n: N) => N[] | undefined, +): string[] { + const out: string[] = []; + const stack = [...nodes]; + while (stack.length) { + const n = stack.pop() as N; + if (isDir(n)) { + out.push(getPath(n)); + const ch = getChildren(n); + if (ch && ch.length) stack.push(...ch); + } + } + return out; +} + +function ancestorPaths(path: string): string[] { + const parts = path.split("/"); + const out: string[] = []; + for (let i = 1; i < parts.length; i++) out.push(parts.slice(0, i).join("/")); + return out; +} + +interface ComputeOpts { + nodes: N[]; + expanded: Set; + search: string; + searchMode: SearchMode; + chainCollapse: boolean; + getPath: (n: N) => string; + getChildren: (n: N) => N[] | undefined; + isDir: (n: N) => boolean; +} + +/** Walk the chain of single-child dirs starting at `root`, returning the + * deepest effective node and the slash-joined display name. */ +function walkChain( + root: N, + chainCollapse: boolean, + isDir: (n: N) => boolean, + getPath: (n: N) => string, + getChildren: (n: N) => N[] | undefined, +): { effective: N; displayName: string } { + let effective = root; + let displayName = lastSegment(getPath(root)); + if (!chainCollapse || !isDir(root)) return { effective, displayName }; + let kids = getChildren(effective) ?? []; + while (kids.length === 1 && isDir(kids[0])) { + effective = kids[0]; + displayName = `${displayName}/${lastSegment(getPath(effective))}`; + kids = getChildren(effective) ?? []; + } + return { effective, displayName }; +} + +/** Precompute the "self-or-descendant matches search" map. */ +function buildMatchMap( + nodes: N[], + lowerSearch: string, + getPath: (n: N) => string, + getChildren: (n: N) => N[] | undefined, +): Map { + const out = new Map(); + const recurse = (n: N): boolean => { + const path = getPath(n); + const self = lastSegment(path).toLowerCase().includes(lowerSearch); + let any = self; + for (const c of getChildren(n) ?? []) if (recurse(c)) any = true; + out.set(path, any); + return any; + }; + for (const n of nodes) recurse(n); + return out; +} + +/** Decide a dir's effective expansion under the current search mode. */ +function effectiveExpansion( + storedExpanded: boolean, + matches: boolean, + hasSearch: boolean, + searchMode: SearchMode, +): boolean { + if (!hasSearch) return storedExpanded; + if (searchMode === "hide") return true; + if (searchMode === "expand") return storedExpanded || matches; + // "collapse" + return matches; +} + +function computeVisibleRows(opts: ComputeOpts): VisibleRow[] { + const { nodes, expanded, search, searchMode, chainCollapse, getPath, getChildren, isDir } = opts; + const lowerSearch = search.toLowerCase(); + const hasSearch = lowerSearch.length > 0; + const matchMap = hasSearch ? buildMatchMap(nodes, lowerSearch, getPath, getChildren) : null; + + const rows: VisibleRow[] = []; + const walk = (list: N[], depth: number) => { + for (const root of list) { + // Skip non-matching subtrees in "hide" mode. + if (hasSearch && searchMode === "hide" && !(matchMap?.get(getPath(root)) ?? false)) continue; + + const { effective, displayName } = walkChain( + root, + chainCollapse, + isDir, + getPath, + getChildren, + ); + const effPath = getPath(effective); + const effIsDir = isDir(effective); + const isExpandedEff = effIsDir + ? effectiveExpansion( + expanded.has(effPath), + matchMap?.get(effPath) ?? false, + hasSearch, + searchMode, + ) + : false; + + rows.push({ + node: effective, + chainRoot: root, + displayName, + path: effPath, + depth, + isExpanded: isExpandedEff, + isDir: effIsDir, + }); + + if (effIsDir && isExpandedEff) walk(getChildren(effective) ?? [], depth + 1); + } + }; + walk(nodes, 0); + return rows; +} + +function useExpandedState( + nodes: N[], + defaultExpanded: UseTreeOptions["defaultExpanded"], + getPath: (n: N) => string, + getChildren: (n: N) => N[] | undefined, + isDir: (n: N) => boolean, +) { + const nodesRef = useRef(nodes); + const getPathRef = useRef(getPath); + const getChildrenRef = useRef(getChildren); + const isDirRef = useRef(isDir); + useEffect(() => { + nodesRef.current = nodes; + getPathRef.current = getPath; + getChildrenRef.current = getChildren; + isDirRef.current = isDir; + }); + + const [expanded, setExpanded] = useState>(() => { + if (defaultExpanded === "all") { + return new Set(collectAllDirPaths(nodes, isDir, getPath, getChildren)); + } + if (defaultExpanded) return new Set(defaultExpanded); + return new Set(); + }); + + const isExpandedFn = useCallback((p: string) => expanded.has(p), [expanded]); + const toggle = useCallback((p: string) => { + setExpanded((prev) => { + const next = new Set(prev); + if (next.has(p)) next.delete(p); + else next.add(p); + return next; + }); + }, []); + const expand = useCallback((p: string) => { + setExpanded((prev) => (prev.has(p) ? prev : new Set(prev).add(p))); + }, []); + const collapse = useCallback((p: string) => { + setExpanded((prev) => { + if (!prev.has(p)) return prev; + const next = new Set(prev); + next.delete(p); + return next; + }); + }, []); + const expandAll = useCallback(() => { + setExpanded( + new Set( + collectAllDirPaths( + nodesRef.current, + isDirRef.current, + getPathRef.current, + getChildrenRef.current, + ), + ), + ); + }, []); + const collapseAll = useCallback(() => setExpanded(new Set()), []); + const expandAncestorsOf = useCallback((p: string) => { + setExpanded((prev) => { + const next = new Set(prev); + for (const a of ancestorPaths(p)) next.add(a); + return next; + }); + }, []); + + return { + expanded, + setExpanded, + isExpanded: isExpandedFn, + toggle, + expand, + collapse, + expandAll, + collapseAll, + expandAncestorsOf, + }; +} + +export function useTree(opts: UseTreeOptions): UseTreeResult { + const { + nodes, + getPath, + getChildren, + isDir, + defaultExpanded, + search = "", + searchMode = "hide", + chainCollapse = false, + } = opts; + const state = useExpandedState(nodes, defaultExpanded, getPath, getChildren, isDir); + const visibleRows = useMemo[]>( + () => + computeVisibleRows({ + nodes, + expanded: state.expanded, + search, + searchMode, + chainCollapse, + getPath, + getChildren, + isDir, + }), + [nodes, state.expanded, search, searchMode, chainCollapse, getPath, getChildren, isDir], + ); + return { ...state, visibleRows }; +}