From 9b481212f651655221dbde2d3655f337808d52b1 Mon Sep 17 00:00:00 2001 From: Anso Date: Wed, 5 Aug 2026 08:52:50 -0400 Subject: [PATCH] fix(files): preserve tree expand state after mutations (#1773) Stop remounting the file tree on every refresh so expanded folders and scroll position survive delete, move, rename, and save. Relist open directories in place and drop stale collapsed caches. --- frontend/src/components/files/FileTree.tsx | 79 ++++-- .../components/files/StackFileExplorer.tsx | 2 +- .../files/__tests__/FileTree.test.tsx | 259 +++++++++++++++++- .../resources/VolumeBrowserSheet.tsx | 2 +- 4 files changed, 320 insertions(+), 22 deletions(-) diff --git a/frontend/src/components/files/FileTree.tsx b/frontend/src/components/files/FileTree.tsx index 783bf21b..2efe002e 100644 --- a/frontend/src/components/files/FileTree.tsx +++ b/frontend/src/components/files/FileTree.tsx @@ -16,8 +16,10 @@ import { cn } from '@/lib/utils'; interface FileTreeProps { /** Loads directory contents at `relPath` (use '' for the tree root). */ loadDir: (relPath: string) => Promise; - /** Stable identity for the source. Changing it remounts the tree. */ + /** Stable identity for the source. Parents remount via React `key` when this + * changes so expand state does not leak across sources. */ sourceKey: string; + /** Soft refresh: relist root and expanded dirs in place; keep expand state. */ refreshKey?: number; selectedPath: string; onSelectFile: (relPath: string, entry: FileEntry) => void; @@ -125,37 +127,80 @@ export function FileTree({ } const sourceKeyRef = useRef(sourceKey); const loadDirRef = useRef(loadDir); + // Read at merge time so a concurrent expand/collapse is not overwritten by the snapshot. + const expandedDirsRef = useRef(expandedDirs); + expandedDirsRef.current = expandedDirs; // Always keep the latest loader in a ref so callers can pass a fresh - // function each render without re-triggering the root fetch effect below. + // function each render without re-triggering the refresh effect below. loadDirRef.current = loadDir; useEffect(() => { sourceKeyRef.current = sourceKey; let cancelled = false; + const expandedSnapshot = [...expandedDirs]; - loadDirRef.current('') - .then((entries) => { - if (!cancelled) { - setRootEntries(entries); - setRootLoading(false); + void (async () => { + try { + const entries = await loadDirRef.current(''); + if (cancelled) return; + setRootEntries(entries); + setError(null); + setRootLoading(false); + } catch (err: unknown) { + if (cancelled) return; + const msg = err instanceof Error ? err.message : 'Failed to load files.'; + setError(msg); + setRootLoading(false); + toast.error('Failed to load files.'); + return; + } + + const results = await Promise.all(expandedSnapshot.map(async (path) => { + try { + const entries = await loadDirRef.current(path); + return { path, ok: true as const, entries }; + } catch { + // Folder is gone or unlistable after a mutation. Prune below; do not + // toast (the user often just deleted or moved this folder). + return { path, ok: false as const }; } - }) - .catch((err: unknown) => { - if (!cancelled) { - const msg = err instanceof Error ? err.message : 'Failed to load files.'; - setError(msg); - setRootLoading(false); - toast.error('Failed to load files.'); + })); + if (cancelled) return; + + const failed = results.filter((r) => !r.ok).map((r) => r.path); + function inFailedTree(path: string): boolean { + return failed.some((p) => path === p || path.startsWith(`${p}/`)); + } + + if (failed.length > 0) { + setExpandedDirs((current) => { + const next = new Set([...current].filter((key) => !inFailedTree(key))); + return next.size === current.size ? current : next; + }); + } + + setDirContents((prev) => { + const fetched = new Map(); + for (const r of results) { + if (r.ok) fetched.set(r.path, r.entries); } + const next = new Map(); + for (const path of expandedDirsRef.current) { + if (inFailedTree(path)) continue; + const entries = fetched.get(path) ?? prev.get(path); + if (entries) next.set(path, entries); + } + return next; }); + })(); return () => { cancelled = true; }; - // loadDir is intentionally read through loadDirRef to avoid refetch on - // identity-only changes from the parent (StackFileExplorer rebuilds the - // arrow on every render). + // Do not depend on expandedDirs (snapshot at start; expand/collapse must + // not refetch). loadDir is read via loadDirRef so identity-only parent + // changes do not refetch. // eslint-disable-next-line react-hooks/exhaustive-deps }, [sourceKey, refreshKey]); diff --git a/frontend/src/components/files/StackFileExplorer.tsx b/frontend/src/components/files/StackFileExplorer.tsx index 797e2b28..a4dda80c 100644 --- a/frontend/src/components/files/StackFileExplorer.tsx +++ b/frontend/src/components/files/StackFileExplorer.tsx @@ -565,7 +565,7 @@ export function StackFileExplorer({ )}
listStackDirectory(stackName, p, selectedRootId)} refreshKey={refreshKey} diff --git a/frontend/src/components/files/__tests__/FileTree.test.tsx b/frontend/src/components/files/__tests__/FileTree.test.tsx index c446e5f1..40c91413 100644 --- a/frontend/src/components/files/__tests__/FileTree.test.tsx +++ b/frontend/src/components/files/__tests__/FileTree.test.tsx @@ -1,9 +1,10 @@ /** * Coverage for FileTree. * - * Locks the expand/collapse behavior: root directory loaded on mount, - * subdirectory fetched on first expand, collapsed on second click, and - * re-expanded from cache (no second fetch) on third click. + * Locks expand/collapse: root loaded on mount, subdirectory fetched on first + * expand, collapsed on second click, re-expanded from cache on third click + * when no refresh ran. Also locks soft refresh (`refreshKey`): expand state + * survives, collapsed caches drop, failed trees (and descendants) prune. */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, waitFor, fireEvent } from '@testing-library/react'; @@ -639,3 +640,255 @@ describe('FileTree layout', () => { expect(screen.getByText('Rename')).toBeInTheDocument(); }); }); + +// ── soft refresh: expand + listings survive refreshKey without remount ─────── + +describe('FileTree soft refresh', () => { + const DEEP_ENTRIES: FileEntry[] = [makeFile('nested.ts')]; + const SRC_WITH_DEEP: FileEntry[] = [makeDir('deep'), makeFile('index.ts')]; + + it('keeps expanded dirs open and re-lists them when refreshKey changes', async () => { + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('index.ts'); + + mockLoadDir.mockClear(); + rerender(); + + await waitFor(() => { + expect(mockLoadDir).toHaveBeenCalledWith(''); + expect(mockLoadDir).toHaveBeenCalledWith('src'); + }); + expect(await screen.findByText('index.ts')).toBeInTheDocument(); + expect(screen.getByText('app.ts')).toBeInTheDocument(); + }); + + it('updates an open folder listing in place on soft refresh', async () => { + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('index.ts'); + + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return [makeFile('index.ts'), makeFile('new.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + expect(await screen.findByText('new.ts')).toBeInTheDocument(); + expect(screen.getByText('index.ts')).toBeInTheDocument(); + expect(screen.queryByText('app.ts')).not.toBeInTheDocument(); + }); + + it('prunes descendant expand state when only the parent re-list fails', async () => { + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_WITH_DEEP; + if (relPath === 'src/deep') return DEEP_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('deep'); + await user.click(screen.getByText('deep')); + await screen.findByText('nested.ts'); + + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') throw new Error('Not found'); + if (relPath === 'src/deep') return DEEP_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + await waitFor(() => expect(screen.queryByText('nested.ts')).not.toBeInTheDocument()); + expect(screen.getByText('src')).toBeInTheDocument(); + expect(screen.queryByText('deep')).not.toBeInTheDocument(); + + mockLoadDir.mockClear(); + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_WITH_DEEP; + if (relPath === 'src/deep') return [makeFile('fresh.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + await user.click(screen.getByText('src')); + await screen.findByText('deep'); + // Prune must drop descendant expand state: deep is visible but collapsed. + expect(screen.queryByText('nested.ts')).not.toBeInTheDocument(); + expect(screen.queryByText('fresh.ts')).not.toBeInTheDocument(); + await user.click(screen.getByText('deep')); + // Re-expand must refetch, not reuse the pre-prune listing. + expect(await screen.findByText('fresh.ts')).toBeInTheDocument(); + }); + + it('keeps nested expand state and updates deep listings on successful refresh', async () => { + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_WITH_DEEP; + if (relPath === 'src/deep') return DEEP_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('deep'); + await user.click(screen.getByText('deep')); + await screen.findByText('nested.ts'); + + mockLoadDir.mockClear(); + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_WITH_DEEP; + if (relPath === 'src/deep') return [makeFile('nested.ts'), makeFile('extra.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + await waitFor(() => { + expect(mockLoadDir).toHaveBeenCalledWith(''); + expect(mockLoadDir).toHaveBeenCalledWith('src'); + expect(mockLoadDir).toHaveBeenCalledWith('src/deep'); + }); + expect(await screen.findByText('extra.ts')).toBeInTheDocument(); + expect(screen.getByText('nested.ts')).toBeInTheDocument(); + }); + + it('drops a collapsed sibling cache while another folder stays open', async () => { + const rootWithLib: FileEntry[] = [makeDir('src'), makeDir('lib'), makeFile('README.md')]; + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return rootWithLib; + if (relPath === 'src') return SRC_ENTRIES; + if (relPath === 'lib') return [makeFile('util.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('index.ts'); + await user.click(screen.getByText('lib')); + await screen.findByText('util.ts'); + await user.click(screen.getByText('src')); + await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument()); + + mockLoadDir.mockClear(); + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return rootWithLib; + if (relPath === 'src') return [makeFile('index.ts'), makeFile('added.ts')]; + if (relPath === 'lib') return [makeFile('util.ts'), makeFile('extra.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + await waitFor(() => { + expect(mockLoadDir).toHaveBeenCalledWith(''); + expect(mockLoadDir).toHaveBeenCalledWith('lib'); + }); + expect(mockLoadDir).not.toHaveBeenCalledWith('src'); + expect(await screen.findByText('extra.ts')).toBeInTheDocument(); + + await user.click(screen.getByText('src')); + expect(await screen.findByText('added.ts')).toBeInTheDocument(); + expect(mockLoadDir).toHaveBeenCalledWith('src'); + }); + + it('keeps a sibling expanded when another open folder fails to re-list', async () => { + const rootWithLib: FileEntry[] = [makeDir('src'), makeDir('lib'), makeFile('README.md')]; + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return rootWithLib; + if (relPath === 'src') return SRC_ENTRIES; + if (relPath === 'lib') return [makeFile('util.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('index.ts'); + await user.click(screen.getByText('lib')); + await screen.findByText('util.ts'); + + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return rootWithLib; + if (relPath === 'src') throw new Error('Not found'); + if (relPath === 'lib') return [makeFile('util.ts'), makeFile('kept.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + expect(await screen.findByText('kept.ts')).toBeInTheDocument(); + expect(screen.getByText('util.ts')).toBeInTheDocument(); + await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument()); + expect(screen.getByText('src')).toBeInTheDocument(); + }); + + it('invalidates collapsed dir cache so re-expand refetches after refresh', async () => { + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return SRC_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + + const user = userEvent.setup(); + const { rerender } = render(); + await screen.findByText('src'); + await user.click(screen.getByText('src')); + await screen.findByText('index.ts'); + await user.click(screen.getByText('src')); + await waitFor(() => expect(screen.queryByText('index.ts')).not.toBeInTheDocument()); + + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + if (relPath === 'src') return [makeFile('index.ts'), makeFile('added.ts')]; + throw new Error(`unexpected path ${relPath}`); + }); + mockLoadDir.mockClear(); + rerender(); + await waitFor(() => expect(mockLoadDir).toHaveBeenCalledWith('')); + expect(mockLoadDir).not.toHaveBeenCalledWith('src'); + + await user.click(screen.getByText('src')); + expect(await screen.findByText('added.ts')).toBeInTheDocument(); + expect(mockLoadDir).toHaveBeenCalledWith('src'); + }); + + it('clears a root-load error when a later soft refresh succeeds', async () => { + mockLoadDir.mockRejectedValue(new Error('Network error')); + const { rerender } = render(); + expect(await screen.findByText('Network error')).toBeInTheDocument(); + + mockLoadDir.mockImplementation(async (relPath: string) => { + if (relPath === '') return ROOT_ENTRIES; + throw new Error(`unexpected path ${relPath}`); + }); + rerender(); + + expect(await screen.findByText('src')).toBeInTheDocument(); + expect(screen.getByText('README.md')).toBeInTheDocument(); + expect(screen.queryByText('Network error')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/resources/VolumeBrowserSheet.tsx b/frontend/src/components/resources/VolumeBrowserSheet.tsx index dcf765d3..712c76ea 100644 --- a/frontend/src/components/resources/VolumeBrowserSheet.tsx +++ b/frontend/src/components/resources/VolumeBrowserSheet.tsx @@ -93,7 +93,7 @@ export function VolumeBrowserSheet({ volumeName, onClose }: VolumeBrowserSheetPr