fix(stack-files): prompt before discarding unsaved edits on file switch (#1203)

FileViewer tracks dirty state via content !== originalContent, but
StackFileExplorer would swap selectedPath on every tree click and silently
drop the buffered edit. A user editing a script and clicking a sibling
file lost their work with no warning.

FileViewer now exposes onDirtyChange so the explorer learns when the
viewer is dirty. StackFileExplorer intercepts the tree-node click: if
dirty, the next selection is stashed and a ConfirmModal asks whether to
discard. Confirm applies the stash, Cancel keeps the current file.
Clicking the already-selected file is a no-op (no spurious prompt).

The dirty signal is reported via a ref so future consumers passing an
inline callback identity each render do not retrigger the unmount
cleanup effect.
This commit is contained in:
Anso
2026-05-24 23:25:52 -04:00
committed by GitHub
parent c8b095b887
commit 3e56696c91
4 changed files with 209 additions and 4 deletions
+20 -3
View File
@@ -1,4 +1,4 @@
import { useState, useEffect, useMemo, Suspense } from 'react';
import { useState, useEffect, useMemo, useRef, Suspense } from 'react';
import { Editor } from '@/lib/monacoLoader';
import { AlertCircle, FileIcon, Download, Loader2, Save } from 'lucide-react';
import { Button } from '@/components/ui/button';
@@ -14,6 +14,7 @@ interface FileViewerProps {
canEdit: boolean;
isDarkMode: boolean;
onSaved?: () => void;
onDirtyChange?: (dirty: boolean) => void;
}
function getFilename(path: string): string {
@@ -91,6 +92,7 @@ export function FileViewer({
canEdit,
isDarkMode,
onSaved,
onDirtyChange,
}: FileViewerProps) {
const [content, setContent] = useState('');
const [originalContent, setOriginalContent] = useState('');
@@ -102,6 +104,23 @@ export function FileViewer({
const [size, setSize] = useState(0);
const readOnly = !canEdit;
const hasChanges = content !== originalContent;
// Stash the latest callback in a ref so the unmount-cleanup effect can be
// truly unmount-scoped without re-running every time a parent passes a fresh
// function identity.
const onDirtyChangeRef = useRef(onDirtyChange);
useEffect(() => {
onDirtyChangeRef.current = onDirtyChange;
}, [onDirtyChange]);
useEffect(() => {
onDirtyChangeRef.current?.(hasChanges);
}, [hasChanges]);
useEffect(() => {
return () => onDirtyChangeRef.current?.(false);
}, []);
const editorOptions = useMemo(
() => ({
@@ -228,8 +247,6 @@ export function FileViewer({
);
}
const hasChanges = content !== originalContent;
return (
<div className="flex flex-col h-full">
<div className="flex items-center justify-between gap-2 px-3 py-1.5 border-b border-glass-border shrink-0">
@@ -1,6 +1,7 @@
import { useState, useEffect, useCallback } from 'react';
import { Trash2, FolderPlus, Download, Loader2 } from 'lucide-react';
import { Button } from '@/components/ui/button';
import { ConfirmModal } from '@/components/ui/modal';
import { toast } from '@/components/ui/toast-store';
import { downloadStackFile, listStackDirectory } from '@/lib/stackFilesApi';
import { FileTree } from './FileTree';
@@ -60,15 +61,21 @@ export function StackFileExplorer({
const [permissionsRelPath, setPermissionsRelPath] = useState('');
const [permissionsEntryName, setPermissionsEntryName] = useState('');
// ── unsaved-changes guard on file switch ──
const [isViewerDirty, setIsViewerDirty] = useState(false);
const [pendingSelection, setPendingSelection] = useState<{ relPath: string; entry: FileEntry } | null>(null);
useEffect(() => {
setSelectedPath(null);
setSelectedEntry(null);
setCurrentDir('');
setIsViewerDirty(false);
setPendingSelection(null);
}, [stackName]);
const refresh = useCallback(() => setRefreshKey((k) => k + 1), []);
const handleSelectFile = useCallback((relPath: string, entry: FileEntry) => {
const applySelection = useCallback((relPath: string, entry: FileEntry) => {
setSelectedPath(relPath);
setSelectedEntry(entry);
const parts = relPath.split('/');
@@ -76,6 +83,15 @@ export function StackFileExplorer({
setCurrentDir(parts.join('/'));
}, []);
const handleSelectFile = useCallback((relPath: string, entry: FileEntry) => {
if (relPath === selectedPath) return;
if (isViewerDirty) {
setPendingSelection({ relPath, entry });
return;
}
applySelection(relPath, entry);
}, [selectedPath, isViewerDirty, applySelection]);
const handleDeleted = useCallback(() => {
setSelectedPath(null);
setSelectedEntry(null);
@@ -226,6 +242,7 @@ export function StackFileExplorer({
canEdit={canEdit}
isDarkMode={isDarkMode}
onSaved={refresh}
onDirtyChange={setIsViewerDirty}
/>
</div>
</div>
@@ -298,6 +315,27 @@ export function StackFileExplorer({
entryName={permissionsEntryName}
canEdit={canEdit}
/>
{/* Unsaved-changes guard on file switch */}
<ConfirmModal
open={pendingSelection !== null}
onOpenChange={(next) => { if (!next) setPendingSelection(null); }}
onCancel={() => setPendingSelection(null)}
kicker="FILES · UNSAVED CHANGES"
title="Discard unsaved changes?"
description="Switching files will discard the edits in the current viewer."
confirmLabel="Discard and switch"
onConfirm={() => {
if (pendingSelection) {
applySelection(pendingSelection.relPath, pendingSelection.entry);
setPendingSelection(null);
}
}}
>
<p className="text-sm text-muted-foreground">
You have unsaved changes in the current file. Switching to another file will discard them.
</p>
</ConfirmModal>
</div>
);
}
@@ -164,4 +164,28 @@ describe('FileViewer', () => {
await waitFor(() => expect(mockReadFile).toHaveBeenCalledTimes(2));
expect(mockReadFile).toHaveBeenNthCalledWith(2, 'my-stack', 'b.txt');
});
it('reports clean dirty state on initial load of a text file', async () => {
mockReadFile.mockResolvedValue(textResult());
const onDirtyChange = vi.fn();
render(<FileViewer {...defaultProps} selectedPath="config/app.txt" onDirtyChange={onDirtyChange} />);
await waitFor(() => expect(screen.getByTestId('monaco-editor')).toBeInTheDocument());
// content === originalContent immediately after load → dirty=false
expect(onDirtyChange).toHaveBeenLastCalledWith(false);
});
it('resets dirty signal on unmount', async () => {
mockReadFile.mockResolvedValue(textResult());
const onDirtyChange = vi.fn();
const { unmount } = render(<FileViewer {...defaultProps} selectedPath="a.txt" onDirtyChange={onDirtyChange} />);
await waitFor(() => expect(screen.getByTestId('monaco-editor')).toBeInTheDocument());
onDirtyChange.mockClear();
unmount();
expect(onDirtyChange).toHaveBeenCalledWith(false);
});
});
@@ -0,0 +1,126 @@
/**
* Coverage for StackFileExplorer's unsaved-changes interception.
*
* The viewer reports its dirty state up via onDirtyChange. When the user
* clicks a sibling file in the tree, the explorer must intercept the switch
* and show a confirm dialog if there are unsaved edits. The viewer mock
* exposes a "Mark dirty" button so the test can drive the dirty signal
* without instantiating Monaco.
*/
import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { FileEntry } from '@/lib/stackFilesApi';
vi.mock('@/lib/stackFilesApi', () => ({
listStackDirectory: vi.fn().mockResolvedValue([]),
downloadStackFile: vi.fn(),
readStackFile: vi.fn(),
writeStackFile: vi.fn(),
}));
vi.mock('@/components/ui/toast-store', () => ({
toast: { error: vi.fn(), success: vi.fn(), loading: vi.fn(() => 'id'), dismiss: vi.fn() },
}));
vi.mock('../FileUploadDropzone', () => ({
FileUploadDropzone: () => <div data-testid="upload-dropzone" />,
}));
vi.mock('../NewFolderDialog', () => ({ NewFolderDialog: () => null }));
vi.mock('../NewFileDialog', () => ({ NewFileDialog: () => null }));
vi.mock('../DeleteFileConfirm', () => ({ DeleteFileConfirm: () => null }));
vi.mock('../RenameDialog', () => ({ RenameDialog: () => null }));
vi.mock('../FilePermissionsDialog', () => ({ FilePermissionsDialog: () => null }));
// FileTree mock exposes two buttons that synthesise selection of two siblings.
vi.mock('../FileTree', () => ({
FileTree: ({ onSelectFile }: { onSelectFile: (rel: string, entry: FileEntry) => void }) => (
<div>
<button onClick={() => onSelectFile('a.txt', { name: 'a.txt', type: 'file', size: 1, mtime: 0, isProtected: false })}>
select-a
</button>
<button onClick={() => onSelectFile('b.txt', { name: 'b.txt', type: 'file', size: 1, mtime: 0, isProtected: false })}>
select-b
</button>
</div>
),
}));
// FileViewer mock exposes a button that flips its dirty signal.
vi.mock('../FileViewer', () => ({
FileViewer: ({ selectedPath, onDirtyChange }: {
selectedPath: string | null;
onDirtyChange?: (dirty: boolean) => void;
}) => (
<div>
<div data-testid="viewer-selected">{selectedPath ?? '(none)'}</div>
<button onClick={() => onDirtyChange?.(true)}>mark-dirty</button>
<button onClick={() => onDirtyChange?.(false)}>mark-clean</button>
</div>
),
}));
import { StackFileExplorer } from '../StackFileExplorer';
function setup() {
return render(
<StackFileExplorer stackName="my-stack" canEdit isDarkMode={false} />,
);
}
describe('StackFileExplorer unsaved-changes interception', () => {
it('switches files immediately when the viewer is clean', async () => {
const user = userEvent.setup();
setup();
await user.click(screen.getByText('select-a'));
expect(screen.getByTestId('viewer-selected').textContent).toBe('a.txt');
await user.click(screen.getByText('select-b'));
expect(screen.getByTestId('viewer-selected').textContent).toBe('b.txt');
expect(screen.queryByText(/discard unsaved changes/i)).not.toBeInTheDocument();
});
it('intercepts the switch when the viewer is dirty and applies on confirm', async () => {
const user = userEvent.setup();
setup();
await user.click(screen.getByText('select-a'));
await user.click(screen.getByText('mark-dirty'));
// Switching siblings while dirty must NOT swap the selection immediately.
await user.click(screen.getByText('select-b'));
expect(screen.getByTestId('viewer-selected').textContent).toBe('a.txt');
expect(screen.getByText(/discard unsaved changes/i)).toBeInTheDocument();
// Confirming the dialog applies the pending selection.
await user.click(screen.getByRole('button', { name: /discard and switch/i }));
expect(screen.getByTestId('viewer-selected').textContent).toBe('b.txt');
});
it('intercepts the switch when dirty and preserves the original on cancel', async () => {
const user = userEvent.setup();
setup();
await user.click(screen.getByText('select-a'));
await user.click(screen.getByText('mark-dirty'));
await user.click(screen.getByText('select-b'));
expect(screen.getByText(/discard unsaved changes/i)).toBeInTheDocument();
await user.click(screen.getByRole('button', { name: /^cancel$/i }));
expect(screen.getByTestId('viewer-selected').textContent).toBe('a.txt');
});
it('clicking the already-selected file does not prompt even when dirty', async () => {
const user = userEvent.setup();
setup();
await user.click(screen.getByText('select-a'));
await user.click(screen.getByText('mark-dirty'));
await user.click(screen.getByText('select-a'));
expect(screen.queryByText(/discard unsaved changes/i)).not.toBeInTheDocument();
});
});