diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index e9d93684378c..968a9e9f01dd 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -22,7 +22,7 @@ import { readLocalApi } from "~/localApi"; import { T3_PIERRE_ICONS } from "~/pierre-icons"; import { PIERRE_TREE_UNSAFE_CSS, pierreTreeStyle } from "~/pierre-tree-theme"; -import { fileTreeEntryTarget, isRootPath } from "./filePath"; +import { fileTreeEntryTarget, isRootPath, rootGroupFolder } from "./filePath"; import { labelForRoot, type ProjectFileRoot, @@ -180,10 +180,14 @@ export default function FileBrowserPanel({ if (query.trim() && !pathSearch.isPending) { for (const searchEntry of pathSearch.entries) { let entry = searchEntry; + // Segments above a root's own node only group roots (`dupe-a` over + // `dupe-a/docs`); they don't belong to this root, so they get none. + let rootDepth = 0; if (rootLabels) { const label = searchEntry.root ? labelForRoot(rootLabels, searchEntry.root) : undefined; if (label === undefined) continue; entry = { ...searchEntry, path: `${label}/${searchEntry.path}` }; + rootDepth = label.split("/").length; } if (!result.has(entry.path)) result.set(entry.path, entry); const segments = entry.path.split("/"); @@ -193,7 +197,7 @@ export default function FileBrowserPanel({ result.set(path, { path, kind: "directory", - ...(entry.root ? { root: entry.root } : {}), + ...(entry.root && index >= rootDepth ? { root: entry.root } : {}), }); } } @@ -233,8 +237,15 @@ export default function FileBrowserPanel({ const entryInfoRef = useRef>(entryInfo); // What a row's right-click actions and drag mention act on: its real file, // resolved through its root rather than the label its tree path starts with. + // A row that only groups roots acts on the folder they sit in, or on nothing + // when they don't share one. const entryTarget = (treePath: string) => { - const info = entryInfoRef.current.get(treePath); + const groupFolder = directoryRoots && rootGroupFolder(directoryRoots, treePath); + if (groupFolder === null) return null; + const info = + groupFolder === undefined + ? entryInfoRef.current.get(treePath) + : { root: groupFolder, relativePath: "" }; return fileTreeEntryTarget({ treePath, cwd, @@ -275,13 +286,26 @@ export default function FileBrowserPanel({ } const relativePath = item.path.replace(/\/$/, ""); const target = entryTarget(relativePath); - const mention = serializeComposerFileLink(target.mentionPath); const pointer = contextMenuPointerRef.current; const pointerIsFresh = pointer !== null && performance.now() - pointer.at < 1000; const anchorRect = context.anchorElement.getBoundingClientRect(); const position = pointerIsFresh ? { x: pointer.x, y: pointer.y } : { x: anchorRect.left, y: anchorRect.bottom }; + if (!target) { + // A group row whose folders don't share one parent has nothing to act on; + // say so rather than leave the right-click looking broken. + try { + await api.contextMenu.show( + [{ id: "no-folder", label: "No single folder for this group", disabled: true }], + position, + ); + } finally { + context.close(); + } + return; + } + const mention = serializeComposerFileLink(target.mentionPath); const fileTarget = { environmentId, filePath: target.filePath, @@ -352,7 +376,7 @@ export default function FileBrowserPanel({ () => createFileTreeDragMentionController({ deselect: (path) => treeModelRef.current?.getItem(path)?.deselect(), - mentionPath: (path) => entryTargetRef.current(path).mentionPath, + mentionPath: (path) => entryTargetRef.current(path)?.mentionPath ?? null, }), [], ); @@ -577,7 +601,8 @@ export default function FileBrowserPanel({ // not depend on running after the tree's own dragstart handler; the drag // data store is writable for every dragstart listener in the dispatch. // The capture phase runs before the tree's own dragstart handler selects - // the dragged row, so the drag flag is up before that selection emits. + // the dragged row, so the drag flag is up before that selection emits, and + // a drag of rows with nothing to mention is cancelled before the tree starts it. const panelRef = useRef(null); useEffect(() => { treeModelRef.current = model; diff --git a/apps/web/src/components/files/filePath.test.ts b/apps/web/src/components/files/filePath.test.ts index c5581a8cf20d..a15156cd3826 100644 --- a/apps/web/src/components/files/filePath.test.ts +++ b/apps/web/src/components/files/filePath.test.ts @@ -6,6 +6,7 @@ import { fileBreadcrumbs, fileTreeEntryTarget, isRootPath, + rootGroupFolder, } from "./filePath"; describe("fileBreadcrumbs", () => { @@ -171,3 +172,66 @@ describe("fileTreeEntryTarget", () => { }); }); }); + +describe("rootGroupFolder", () => { + const roots = [ + { root: "/home/user/dev/dupe-a/docs", label: "dupe-a/docs" }, + { root: "/home/user/dev/dupe-b/docs/", label: "dupe-b/docs" }, + { root: "/home/user/dev/api", label: "api" }, + ]; + + it("resolves a row that only groups roots to the folder they sit in", () => { + expect(rootGroupFolder(roots, "dupe-a")).toBe("/home/user/dev/dupe-a"); + expect(rootGroupFolder(roots, "dupe-b")).toBe("/home/user/dev/dupe-b"); + }); + + it("leaves roots and the rows inside them alone", () => { + expect(rootGroupFolder(roots, "dupe-a/docs")).toBeUndefined(); + expect(rootGroupFolder(roots, "dupe-a/docs/guide")).toBeUndefined(); + expect(rootGroupFolder(roots, "api")).toBeUndefined(); + expect(rootGroupFolder(roots, "dupe")).toBeUndefined(); + }); + + it("keeps a Windows root's separators", () => { + expect( + rootGroupFolder([{ root: "C:\\dev\\dupe-a\\docs", label: "dupe-a/docs" }], "dupe-a"), + ).toBe("C:\\dev\\dupe-a"); + }); + + it("names no folder for a row that is a drive or a network host", () => { + // `C:\\docs` and `D:\\docs` are labelled `C:/docs` and `D:/docs`, so the + // tree shows a `C:` row. + const drives = [ + { root: "C:\\docs", label: "C:/docs" }, + { root: "D:\\docs", label: "D:/docs" }, + ]; + expect(rootGroupFolder(drives, "C:")).toBeNull(); + const shares = [ + { root: "\\\\server\\share\\docs", label: "server/share/docs" }, + { root: "\\\\other\\share\\docs", label: "other/share/docs" }, + ]; + expect(rootGroupFolder(shares, "server")).toBeNull(); + expect(rootGroupFolder(shares, "server/share")).toBe("\\\\server\\share"); + }); + + it("names no folder for a worktree, whose path doesn't end in its label", () => { + expect( + rootGroupFolder( + [{ root: "/home/user/.t3/worktrees/docs-feature", label: "dupe-a/docs" }], + "dupe-a", + ), + ).toBeNull(); + }); + + it("names no folder when the grouped roots sit in different folders", () => { + expect( + rootGroupFolder( + [ + { root: "/x/shared/dupe-a/docs", label: "dupe-a/docs" }, + { root: "/y/dupe-a/notes", label: "dupe-a/notes" }, + ], + "dupe-a", + ), + ).toBeNull(); + }); +}); diff --git a/apps/web/src/components/files/filePath.ts b/apps/web/src/components/files/filePath.ts index 6902b297af5d..3ff7dc74ed33 100644 --- a/apps/web/src/components/files/filePath.ts +++ b/apps/web/src/components/files/filePath.ts @@ -98,3 +98,30 @@ export function fileTreeEntryTarget(input: { mentionPath: `${root}/${input.relativePath}`, }; } + +/** + * The real folder behind a row that only groups roots. When listed folders + * share a name their labels grow parent segments (`dupe-a/docs`), and the tree + * shows `dupe-a` as a row of its own. Undefined when `treePath` is no such row; + * null when its roots don't name one folder, as with an isolated run's + * worktree, whose path doesn't end in its repo's label, or when that folder is + * a drive (`C:`) or network host (`\\server`), which the file actions can't + * name from a parent folder. + */ +export function rootGroupFolder( + roots: readonly { readonly root: string; readonly label: string }[], + treePath: string, +): string | null | undefined { + let folder: string | undefined; + for (const { root, label } of roots) { + if (!label.startsWith(`${treePath}/`)) continue; + const trimmed = root.replace(/[\\/]+$/, ""); + const normalized = trimmed.replaceAll("\\", "/"); + if (normalized !== label && !normalized.endsWith(`/${label}`)) return null; + const candidate = trimmed.slice(0, trimmed.length - (label.length - treePath.length)); + if (/^(?:[A-Za-z]:|[\\/]{2}[^\\/]+)$/.test(candidate)) return null; + if (folder !== undefined && folder !== candidate) return null; + folder = candidate; + } + return folder; +} diff --git a/apps/web/src/components/files/fileTreeDragMention.test.ts b/apps/web/src/components/files/fileTreeDragMention.test.ts index e5552ff750b6..5688326054e6 100644 --- a/apps/web/src/components/files/fileTreeDragMention.test.ts +++ b/apps/web/src/components/files/fileTreeDragMention.test.ts @@ -12,6 +12,19 @@ const makeTransfer = (plainText = "") => { }; }; +const dragStart = (path: ReadonlyArray, dataTransfer = makeTransfer()) => { + const calls: Array = []; + return { + calls, + event: { + dataTransfer, + composedPath: () => path, + preventDefault: () => void calls.push("preventDefault"), + stopPropagation: () => void calls.push("stopPropagation"), + }, + }; +}; + const rowNode = (path: string) => ({ getAttribute: (name: string) => (name === "data-item-path" ? path : null), }); @@ -20,10 +33,7 @@ describe("createFileTreeDragMentionController", () => { it("tags a row drag with the mention payload and flags the drag", () => { const controller = createFileTreeDragMentionController({ deselect: () => {} }); const transfer = makeTransfer(); - controller.handleDragStart({ - dataTransfer: transfer, - composedPath: () => [{}, rowNode("docs/index.md"), {}], - }); + controller.handleDragStart(dragStart([{}, rowNode("docs/index.md"), {}], transfer).event); expect(transfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe("[index.md](docs/index.md)"); expect(controller.isDragInProgress()).toBe(true); }); @@ -34,22 +44,45 @@ describe("createFileTreeDragMentionController", () => { mentionPath: (treePath) => `/home/user/dev/${treePath}`, }); const transfer = makeTransfer(); - controller.handleDragStart({ - dataTransfer: transfer, - composedPath: () => [rowNode("notes/ideas.md")], - }); + controller.handleDragStart(dragStart([rowNode("notes/ideas.md")], transfer).event); expect(transfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe( "[ideas.md](/home/user/dev/notes/ideas.md)", ); }); + it("cancels a drag of rows that name no real path before the tree sees it", () => { + const deselected: Array = []; + const controller = createFileTreeDragMentionController({ + deselect: (path) => deselected.push(path), + mentionPath: (treePath) => (treePath === "dupe-a" ? null : `/dev/${treePath}`), + }); + const drag = dragStart([rowNode("dupe-a/")]); + controller.handleDragStart(drag.event); + expect(drag.calls).toEqual(["preventDefault", "stopPropagation"]); + expect(drag.event.dataTransfer.data.has(COMPOSER_MENTION_DRAG_TYPE)).toBe(false); + expect(controller.isDragInProgress()).toBe(false); + controller.handleDragEnd(); + expect(deselected).toEqual([]); + }); + + it("drags the mentionable part of a selection that includes an unmentionable row", () => { + const controller = createFileTreeDragMentionController({ + deselect: () => {}, + mentionPath: (treePath) => (treePath === "dupe-a" ? null : `/dev/${treePath}`), + }); + controller.handleSelectionChange(["dupe-a/", "notes/ideas.md"]); + const drag = dragStart([rowNode("dupe-a/")]); + controller.handleDragStart(drag.event); + expect(drag.calls).toEqual([]); + expect(drag.event.dataTransfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe( + "[ideas.md](/dev/notes/ideas.md)", + ); + }); + it("strips the trailing slash from directory rows", () => { const controller = createFileTreeDragMentionController({ deselect: () => {} }); const transfer = makeTransfer(); - controller.handleDragStart({ - dataTransfer: transfer, - composedPath: () => [rowNode("docs/architecture/")], - }); + controller.handleDragStart(dragStart([rowNode("docs/architecture/")], transfer).event); expect(transfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe("[architecture](docs/architecture)"); }); @@ -59,7 +92,7 @@ describe("createFileTreeDragMentionController", () => { // pill into the composer. const controller = createFileTreeDragMentionController({ deselect: () => {} }); const transfer = makeTransfer("selected text"); - controller.handleDragStart({ dataTransfer: transfer, composedPath: () => [{}] }); + controller.handleDragStart(dragStart([{}], transfer).event); expect(transfer.data.has(COMPOSER_MENTION_DRAG_TYPE)).toBe(false); expect(controller.isDragInProgress()).toBe(false); }); @@ -67,7 +100,7 @@ describe("createFileTreeDragMentionController", () => { it("ignores drags that carry no row path", () => { const controller = createFileTreeDragMentionController({ deselect: () => {} }); const transfer = makeTransfer(); - controller.handleDragStart({ dataTransfer: transfer, composedPath: () => [{}] }); + controller.handleDragStart(dragStart([{}], transfer).event); expect(transfer.data.has(COMPOSER_MENTION_DRAG_TYPE)).toBe(false); expect(controller.isDragInProgress()).toBe(false); }); @@ -77,10 +110,7 @@ describe("createFileTreeDragMentionController", () => { const controller = createFileTreeDragMentionController({ deselect: (path) => deselected.push(path), }); - controller.handleDragStart({ - dataTransfer: makeTransfer(), - composedPath: () => [rowNode("src/app.ts")], - }); + controller.handleDragStart(dragStart([rowNode("src/app.ts")], makeTransfer()).event); controller.handleDragEnd(); controller.handleDragEnd(); expect(deselected).toEqual(["src/app.ts"]); @@ -94,10 +124,7 @@ describe("createFileTreeDragMentionController", () => { }); controller.handleSelectionChange(["docs/index.md", "docs/api.md", "src/app.ts"]); const transfer = makeTransfer(); - controller.handleDragStart({ - dataTransfer: transfer, - composedPath: () => [rowNode("docs/api.md")], - }); + controller.handleDragStart(dragStart([rowNode("docs/api.md")], transfer).event); expect(transfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe( "[index.md](docs/index.md) [api.md](docs/api.md) [app.ts](src/app.ts)", ); @@ -109,10 +136,7 @@ describe("createFileTreeDragMentionController", () => { const controller = createFileTreeDragMentionController({ deselect: () => {} }); controller.handleSelectionChange(["docs/index.md"]); const transfer = makeTransfer(); - controller.handleDragStart({ - dataTransfer: transfer, - composedPath: () => [rowNode("src/app.ts")], - }); + controller.handleDragStart(dragStart([rowNode("src/app.ts")], transfer).event); expect(transfer.getData(COMPOSER_MENTION_DRAG_TYPE)).toBe("[app.ts](src/app.ts)"); }); diff --git a/apps/web/src/components/files/fileTreeDragMention.ts b/apps/web/src/components/files/fileTreeDragMention.ts index 7dc6c6cc87e1..e3e86e5d3623 100644 --- a/apps/web/src/components/files/fileTreeDragMention.ts +++ b/apps/web/src/components/files/fileTreeDragMention.ts @@ -10,13 +10,18 @@ interface FileTreeDragTransfer { export interface FileTreeDragStartEvent { readonly dataTransfer: FileTreeDragTransfer | null; composedPath(): ReadonlyArray; + preventDefault(): void; + stopPropagation(): void; } export interface FileTreeDragMentionHost { /** Drop the tree's gesture-applied selection of the dragged row. */ deselect(treePath: string): void; - /** The path a dragged row is mentioned by; its tree path when omitted. */ - mentionPath?(treePath: string): string; + /** + * The path a dragged row is mentioned by, or null when it names no real + * path; its tree path when omitted. + */ + mentionPath?(treePath: string): string | null; } export interface FileTreeDragMentionController { @@ -78,12 +83,16 @@ export function createFileTreeDragMentionController( const mentions = dragged .map((path) => { const treePath = path.replace(/\/+$/, ""); - return composerMentionFromTreePath( - treePath && host.mentionPath ? host.mentionPath(treePath) : treePath, - ); + const mentionPath = treePath && host.mentionPath ? host.mentionPath(treePath) : treePath; + return mentionPath === null ? null : composerMentionFromTreePath(mentionPath); }) .filter((mention): mention is string => mention !== null); if (mentions.length === 0) { + // Nothing here names a real path, so there is nothing to drag. Cancel + // before the tree sees the event: it would select the rows and add + // their tree path as text/plain, which the composer inserts as text. + event.preventDefault(); + event.stopPropagation(); return; } draggedPaths = dragged;