From 8a88d2c04fd348ead6416d145bb63bfda0601557 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Fri, 2 Jan 2026 16:40:25 +0100 Subject: [PATCH] fix(file-tree): cleanup based on given path not what's visible --- .../components/__test__/file-tree.test.tsx | 86 +++++++++++++------ app/client/components/file-tree.tsx | 17 ++-- 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/app/client/components/__test__/file-tree.test.tsx b/app/client/components/__test__/file-tree.test.tsx index be693916..7589fe63 100644 --- a/app/client/components/__test__/file-tree.test.tsx +++ b/app/client/components/__test__/file-tree.test.tsx @@ -1,15 +1,16 @@ +/** biome-ignore-all lint/style/noNonNullAssertion: Testing file - non-null assertions are acceptable here */ import { expect, test, describe } from "bun:test"; import { render, screen, fireEvent } from "@testing-library/react"; import { FileTree, type FileEntry } from "../file-tree"; describe("FileTree Selection Logic", () => { - const immichFiles: FileEntry[] = [ - { name: "immich", path: "/immich", type: "folder" }, - { name: "immich_photos", path: "/immich/immich_photos", type: "folder" }, - { name: "backups", path: "/immich/immich_photos/backups", type: "folder" }, - { name: "library", path: "/immich/immich_photos/library", type: "folder" }, - { name: "profile", path: "/immich/immich_photos/profile", type: "folder" }, - { name: "upload", path: "/immich/immich_photos/upload", type: "folder" }, + const testFiles: FileEntry[] = [ + { name: "root", path: "/root", type: "folder" }, + { name: "photos", path: "/root/photos", type: "folder" }, + { name: "backups", path: "/root/photos/backups", type: "folder" }, + { name: "library", path: "/root/photos/library", type: "folder" }, + { name: "profile", path: "/root/photos/profile", type: "folder" }, + { name: "upload", path: "/root/photos/upload", type: "folder" }, ]; test("selecting a folder simplifies to parent if it's the only child", async () => { @@ -20,50 +21,48 @@ describe("FileTree Selection Logic", () => { render( f.path))} + expandedFolders={new Set(testFiles.map((f) => f.path))} />, ); - const immichPhotosCheckbox = screen - .getByText("immich_photos") - .parentElement?.querySelector('button[role="checkbox"]'); - expect(immichPhotosCheckbox).toBeTruthy(); + const photosCheckbox = screen.getByText("photos").parentElement?.querySelector('button[role="checkbox"]'); + expect(photosCheckbox).toBeTruthy(); - fireEvent.click(immichPhotosCheckbox!); + fireEvent.click(photosCheckbox!); - expect(currentSelection.has("/immich")).toBe(true); + expect(currentSelection.has("/root")).toBe(true); expect(currentSelection.size).toBe(1); }); test("unselecting a child removes the parent from selection", async () => { - let currentSelection = new Set(["/immich"]); + let currentSelection = new Set(["/root"]); const onSelectionChange = (selection: Set) => { currentSelection = selection; }; render( f.path))} + expandedFolders={new Set(testFiles.map((f) => f.path))} />, ); const libraryCheckbox = screen.getByText("library").parentElement?.querySelector('button[role="checkbox"]'); fireEvent.click(libraryCheckbox!); - expect(currentSelection.has("/immich")).toBe(false); - expect(currentSelection.has("/immich/immich_photos")).toBe(false); + expect(currentSelection.has("/root")).toBe(false); + expect(currentSelection.has("/root/photos")).toBe(false); - expect(currentSelection.has("/immich/immich_photos/backups")).toBe(true); - expect(currentSelection.has("/immich/immich_photos/profile")).toBe(true); - expect(currentSelection.has("/immich/immich_photos/upload")).toBe(true); + expect(currentSelection.has("/root/photos/backups")).toBe(true); + expect(currentSelection.has("/root/photos/profile")).toBe(true); + expect(currentSelection.has("/root/photos/upload")).toBe(true); expect(currentSelection.size).toBe(3); }); @@ -75,11 +74,11 @@ describe("FileTree Selection Logic", () => { const { rerender } = render( f.path))} + expandedFolders={new Set(testFiles.map((f) => f.path))} />, ); @@ -91,16 +90,16 @@ describe("FileTree Selection Logic", () => { rerender( f.path))} + expandedFolders={new Set(testFiles.map((f) => f.path))} />, ); } - expect(currentSelection.has("/immich")).toBe(true); + expect(currentSelection.has("/root")).toBe(true); expect(currentSelection.size).toBe(1); }); @@ -133,4 +132,35 @@ describe("FileTree Selection Logic", () => { expect(currentSelection.has("/root")).toBe(false); expect(currentSelection.size).toBe(1); }); + + test("simplifies existing deep paths when parent is selected", async () => { + const files: FileEntry[] = [ + { name: "hello", path: "/hello", type: "folder" }, + { name: "hello_prev", path: "/hello_prev", type: "folder" }, + { name: "service", path: "/service", type: "folder" }, + ]; + + let currentSelection = new Set(["/hello", "/hello_prev", "/service/app/data/upload"]); + const onSelectionChange = (selection: Set) => { + currentSelection = selection; + }; + + render( + , + ); + + const serviceCheckbox = screen.getByText("service").parentElement?.querySelector('button[role="checkbox"]'); + expect(serviceCheckbox).toBeTruthy(); + + fireEvent.click(serviceCheckbox!); + + expect(currentSelection.has("/service")).toBe(true); + expect(currentSelection.has("/service/app/data/upload")).toBe(false); + expect(currentSelection.size).toBe(3); // /hello, /hello_prev, /service + }); }); diff --git a/app/client/components/file-tree.tsx b/app/client/components/file-tree.tsx index fd70a968..fa9559c2 100644 --- a/app/client/components/file-tree.tsx +++ b/app/client/components/file-tree.tsx @@ -116,13 +116,15 @@ export const FileTree = memo((props: Props) => { // Add new folders to collapsed set when file list changes useEffect(() => { setCollapsedFolders((prevSet) => { + let hasChanges = false; const newSet = new Set(prevSet); for (const item of fileList) { if (item.kind === "folder" && !newSet.has(item.fullPath) && !expandedFolders.has(item.fullPath)) { newSet.add(item.fullPath); + hasChanges = true; } } - return newSet; + return hasChanges ? newSet : prevSet; }); }, [fileList, expandedFolders]); @@ -149,9 +151,9 @@ export const FileTree = memo((props: Props) => { newSelection.add(path); // Remove any descendants from selection since parent now covers them - for (const item of fileList) { - if (item.fullPath.startsWith(`${path}/`)) { - newSelection.delete(item.fullPath); + for (const selectedPath of newSelection) { + if (selectedPath.startsWith(`${path}/`)) { + newSelection.delete(selectedPath); } } } else { @@ -488,8 +490,9 @@ const NodeButton = memo(({ depth, icon, onClick, onMouseEnter, className, childr const paddingLeft = useMemo(() => `${8 + depth * NODE_PADDING_LEFT}px`, [depth]); return ( - + ); });