🐛 Keep upload modal open when some files fail
The failure report lives inside the upload modal, but the modal
auto-closed the instant the transfer finished (handleUploadComplete →
onClose), unmounting the report before the user could read it — so the
"which files failed" list never actually appeared.
Split the modal's completion callback in two:
- onUploadComplete: refresh the grid only (no close), as bytes land and
again when processing finishes
- onUploadSettled({ hasFailures }): fired once the transfer settles; the
modal auto-closes only on a clean upload and stays open (report
visible) when any file failed
Also reset the transfer UI when nothing was queued (every file failed),
which previously left the modal spinning forever. Add a PhotoUploadModal
test covering close-on-clean vs stay-open-on-failure.
This commit is contained in:
@@ -13,7 +13,14 @@ import { useUploadProgress } from '../../hooks/useUploadProgress';
|
||||
|
||||
interface PhotoUploadProps {
|
||||
eventId: number;
|
||||
/** Refresh the photo grid. Called early (as bytes land) and again when
|
||||
* processing finishes. Never closes the modal. */
|
||||
onUploadComplete?: () => void;
|
||||
/** Fired once the transfer stage is done, reporting whether any file
|
||||
* failed. The host (modal) uses this to decide whether to auto-close:
|
||||
* a clean upload closes as before; a partial failure keeps the modal
|
||||
* open so the failure report stays visible. */
|
||||
onUploadSettled?: (result: { hasFailures: boolean }) => void;
|
||||
}
|
||||
|
||||
const DEFAULT_MAX_FILES_PER_UPLOAD = 500;
|
||||
@@ -43,7 +50,7 @@ interface UploadFailure {
|
||||
kind: UploadFailureKind;
|
||||
}
|
||||
|
||||
export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadComplete }) => {
|
||||
export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadComplete, onUploadSettled }) => {
|
||||
const { t } = useTranslation();
|
||||
const [isUploading, setIsUploading] = useState(false);
|
||||
const [selectedFiles, setSelectedFiles] = useState<File[]>([]);
|
||||
@@ -234,6 +241,8 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
|
||||
// Accumulates transfer-stage failures (per-file rejections + whole-chunk
|
||||
// failures) with their reasons, so the report can name each one.
|
||||
const collected: UploadFailure[] = [];
|
||||
// Whether at least one chunk was accepted for background processing.
|
||||
let anyQueued = false;
|
||||
|
||||
try {
|
||||
for (let chunkIndex = 0; chunkIndex < chunks.length; chunkIndex++) {
|
||||
@@ -311,6 +320,7 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
|
||||
// Backend returns a per-request upload_id. Track it so the
|
||||
// processing-status hook can poll/stream live progress.
|
||||
if (response.data?.upload_id) {
|
||||
anyQueued = true;
|
||||
const newId = response.data.upload_id as string;
|
||||
setUploadIds((prev) => (prev.includes(newId) ? prev : [...prev, newId]));
|
||||
}
|
||||
@@ -361,6 +371,22 @@ export const PhotoUpload: React.FC<PhotoUploadProps> = ({ eventId, onUploadCompl
|
||||
onUploadComplete();
|
||||
}
|
||||
|
||||
// Tell the host the transfer stage settled. A clean upload lets the
|
||||
// modal auto-close (unchanged behaviour); a partial failure keeps it
|
||||
// open so the failure report below stays visible.
|
||||
onUploadSettled?.({ hasFailures: collected.length > 0 });
|
||||
|
||||
// Nothing was queued for background processing (every chunk failed,
|
||||
// or a pre-async backend) — there's no processing phase to wait on,
|
||||
// so reset the transfer UI here. The failure report persists.
|
||||
if (!anyQueued) {
|
||||
setIsUploading(false);
|
||||
setUploadProgress(0);
|
||||
setCurrentChunk(0);
|
||||
setTotalChunks(0);
|
||||
setPhase({ kind: 'idle' });
|
||||
}
|
||||
|
||||
// If the backend never returned an upload_id (e.g. only failures
|
||||
// or pre-async-backend deployment), we have nothing to wait for —
|
||||
// fall through to the finally cleanup which resets state.
|
||||
|
||||
@@ -21,11 +21,19 @@ export const PhotoUploadModal: React.FC<PhotoUploadModalProps> = ({
|
||||
|
||||
if (!isOpen) return null;
|
||||
|
||||
// Refresh the host's grid, but do NOT close here — closing is decided by
|
||||
// onUploadSettled so a partial-failure upload keeps the modal (and its
|
||||
// failure report) open.
|
||||
const handleUploadComplete = () => {
|
||||
if (onUploadComplete) {
|
||||
onUploadComplete();
|
||||
onUploadComplete?.();
|
||||
};
|
||||
|
||||
// Auto-close only on a clean upload; keep the modal open when some files
|
||||
// failed so the report stays visible until the user dismisses it.
|
||||
const handleUploadSettled = ({ hasFailures }: { hasFailures: boolean }) => {
|
||||
if (!hasFailures) {
|
||||
onClose();
|
||||
}
|
||||
onClose();
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -46,9 +54,10 @@ export const PhotoUploadModal: React.FC<PhotoUploadModalProps> = ({
|
||||
|
||||
{/* Scrollable Content */}
|
||||
<div className="flex-1 overflow-y-auto p-6">
|
||||
<PhotoUpload
|
||||
eventId={eventId}
|
||||
<PhotoUpload
|
||||
eventId={eventId}
|
||||
onUploadComplete={handleUploadComplete}
|
||||
onUploadSettled={handleUploadSettled}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
/**
|
||||
* The upload modal must keep itself open when an upload partially fails, so
|
||||
* the failure report stays visible; a clean upload still auto-closes. We stub
|
||||
* PhotoUpload with buttons that fire its onUploadSettled callback both ways.
|
||||
*/
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { PhotoUploadModal } from '../PhotoUploadModal';
|
||||
|
||||
vi.mock('react-i18next', async () => {
|
||||
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||
return {
|
||||
...actual,
|
||||
useTranslation: () => ({ t: (_k: string, fb?: any) => (typeof fb === 'string' ? fb : _k) }),
|
||||
};
|
||||
});
|
||||
|
||||
// Stub PhotoUpload: expose buttons that settle clean vs. with failures.
|
||||
vi.mock('../PhotoUpload', () => ({
|
||||
PhotoUpload: ({ onUploadSettled }: any) => (
|
||||
<div>
|
||||
<button onClick={() => onUploadSettled?.({ hasFailures: false })}>settle-clean</button>
|
||||
<button onClick={() => onUploadSettled?.({ hasFailures: true })}>settle-failed</button>
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
describe('PhotoUploadModal auto-close behaviour', () => {
|
||||
it('closes after a clean upload', async () => {
|
||||
const user = userEvent.setup();
|
||||
const onClose = vi.fn();
|
||||
render(<PhotoUploadModal isOpen eventId={1} onClose={onClose} />);
|
||||
|
||||
await user.click(screen.getByText('settle-clean'));
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('stays open when some files failed', async () => {
|
||||
const user = userEvent.setup();
|
||||
const onClose = vi.fn();
|
||||
render(<PhotoUploadModal isOpen eventId={1} onClose={onClose} />);
|
||||
|
||||
await user.click(screen.getByText('settle-failed'));
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user