fix: report a download in one place, and let a failed one be dismissed

.changeset/download-progress-in-one-place.md

@@ -0,0 +1,42 @@
+---
+'frontend': patch
+---
+
+Report a download in one place, and let a failed one be dismissed
+
+A download that hit a network error left a row nothing could remove, so the
+panel showing it stayed up until the page was reloaded. The store gave
+`completed` and `cancelled` rows a delay after which they clear themselves but
+had no case for `error`; the operations panel mapped `upload` and `delete` to
+their remove actions and never handled `download`, in both the row's remove and
+the panel's close button; and the panel only hides once its list is empty. So
+the close button did nothing, however many times it was pressed.
+
+A failed row also had no button on it at all. The row drew its trailing control
+for active, then completed, then cancelled operations, and anything that had
+gone wrong fell past all three to nothing - so the one row a reader most wants
+rid of was the only one with nothing to press. That was true of failed uploads
+and deletes too, and is fixed for all three.
+
+Settled downloads now wait to be dismissed instead of clearing themselves. They
+used to go on a timer, three seconds for a completion and two for a cancel,
+which read as tidy until a failure needed the same treatment: a reason for a
+failure that takes itself off the screen is no use to anyone who was not looking
+at that moment, and the panel can be collapsed. Uploads and deletes have always
+waited to be dismissed, and downloads now match them.
+
+The same download was also announcing itself three times over: a toast, a card
+of its own in the top right, and a row on the operations card in the bottom
+right. The operations card is the one that stays, since it already carried every
+status the separate card did, down to the queue position and the reason for a
+failure. The separate card is gone, along with the toasts for starting,
+cancelling and failing. One toast is left for a download that throws before the
+service can record it, which leaves no row to read. The row now shows the
+percentage while downloading, the only thing the removed card said that it did
+not.
+
+The three copies of "which statuses count as still running" are now one, and it
+knows about downloads. Each listed upload and delete statuses only, so a running
+download read as settled - which would have mattered the moment the close button
+learned to remove downloads, since it would have dropped the row of a transfer
+still in flight rather than offering to cancel it.

frontend/src/app/dashboard/layout.tsx

@@ -16,7 +16,6 @@ import { DetailsManager } from '@/features/dashboard/components/ui/details/detai
 import { FilePreviewModal } from '@/components/file-preview';
 import { RenameModalManager } from '@/features/dashboard/components/ui/dialogs/rename-modal-manager';
 import { ShareModalManager } from '@/features/dashboard/components/ui/dialogs/share-modal-manager';
-import { DownloadProgressManager } from '@/features/dashboard/components/ui/download-progress-manager';
 import { useAuth } from '@/hooks/use-auth';
 import { UploadCard } from '@/features/upload/components/upload-card';
 import { useDeleteUnloadGuard } from '@/features/upload/hooks/use-delete-unload-guard';
@@ -150,10 +149,10 @@ const LayoutShell = ({ children }: { children: React.ReactNode }) => {
         <DetailsManager />
       </div>
 
+      {/* Downloads report into this same card. They used to have a second one
+          of their own in the opposite corner, showing the same transfer twice. */}
       <UploadCard />
 
-      <DownloadProgressManager />
-
       <FilePreviewModal />
 
       <RenameModalManager />

frontend/src/features/dashboard/components/ui/index.ts

@@ -14,8 +14,6 @@ export { CreateMenu } from './menus/create-menu';
 export { FileOverflowMenu } from './menus/file-overflow-menu';
 export { FolderOverflowMenu } from './menus/folder-overflow-menu';
 
-export { DownloadProgressManager } from './download-progress-manager';
-
 export { SuggestedSectionSkeleton, DashboardLoading } from './skeletons/dashboard-skeleton';
 export {
   FileSkeletonGrid,

frontend/src/features/dashboard/hooks/use-download.ts

@@ -40,18 +40,11 @@ async function withConcurrency<T>(
  * row in the listing on every chunk of every download.
  */
 export const useDownloadActions = () => {
-  const { error: showError, info } = useNotification();
+  const { error: showError } = useNotification();
   const { apiS3 } = useAuthGuard();
 
   const startDownload = useDownloadStore((state) => state.startDownload);
-  const cancelInStore = useDownloadStore((state) => state.cancelDownload);
-
-  const handleError = useCallback(
-    (_fileId: string, error: string) => {
-      showError(error);
-    },
-    [showError]
-  );
+  const cancelDownload = useDownloadStore((state) => state.cancelDownload);
 
   const downloadFile = useCallback(
     async (file: FileItem) => {
@@ -61,30 +54,34 @@ export const useDownloadActions = () => {
       if (!apiS3) return;
 
       try {
-        await startDownload(apiS3, file, { onError: handleError });
+        await startDownload(apiS3, file);
       } catch (error) {
+        /**
+         * The last download toast, and the only one worth keeping.
+         *
+         * Starting, cancelling and failing are all written to the operations
+         * card as they happen, so a toast saying the same thing was the same
+         * news twice, in two corners of the screen at once.
+         *
+         * This branch is different, and rare: the service reports its own
+         * failures through `onProgress` and never rejects, so the only way here
+         * is something throwing before it takes over - building the service for
+         * a new provider, say. That leaves no row on the card to read, and it is
+         * the one failure that would otherwise pass in silence.
+         */
         showError(`Failed to download ${file.name}, ${error}`);
       }
     },
-    [apiS3, startDownload, handleError, showError]
+    [apiS3, startDownload, showError]
   );
 
   const downloadMultipleFiles = useCallback(
     async (files: FileItem[]) => {
       if (!apiS3 || files.length === 0) return;
 
-      info(`Downloading ${files.length} file${files.length > 1 ? 's' : ''}...`);
       await withConcurrency(files, MULTI_DOWNLOAD_CONCURRENCY, downloadFile);
     },
-    [apiS3, downloadFile, info]
-  );
-
-  const cancelDownload = useCallback(
-    (fileId: string) => {
-      cancelInStore(fileId);
-      info('Download cancelled');
-    },
-    [cancelInStore, info]
+    [apiS3, downloadFile]
   );
 
   return { downloadFile, downloadMultipleFiles, cancelDownload };
@@ -109,12 +106,19 @@ export const useIsFileDownloading = (fileId: string): boolean =>
 export const useDownloadList = () => {
   const downloads = useDownloadStore((state) => state.downloads);
   const { cancelDownload } = useDownloadActions();
+  /**
+   * Drops a settled row without touching the transfer, which is what a failed
+   * download needs: cancelling one is a no-op because there is nothing left in
+   * flight to abort, so the panels had no way to clear it. Selecting the action
+   * on its own is free - zustand action identities are stable.
+   */
+  const removeDownload = useDownloadStore((state) => state.removeDownload);
 
   const downloadProgress = useMemo(() => Array.from(downloads.values()), [downloads]);
   const getAllDownloads = useCallback(
     (): DownloadProgress[] => downloadProgress,
     [downloadProgress]
   );
 
-  return { downloadProgress, getAllDownloads, cancelDownload };
+  return { downloadProgress, getAllDownloads, cancelDownload, removeDownload };
 };

frontend/src/features/dashboard/stores/use-download-store.test.ts

@@ -172,39 +172,33 @@ describe('startDownload', () => {
     expect(store().downloads.get('file-1')!.progress).toBe(25);
   });
 
-  it('clears a completed download after the linger delay', async () => {
-    vi.useFakeTimers();
-    const service = fakeService();
-    service.downloadFile.mockImplementation(async (_f, opts) => {
-      opts.onComplete('file-1');
-    });
-    createDownloadServiceMock.mockReturnValue(service as never);
-
-    store().setProgress(progress({ status: 'completed', progress: 100 }));
-    await store().startDownload(apiA, file);
-
-    // The row stays briefly so the user sees it finish.
-    expect(store().downloads.has('file-1')).toBe(true);
-    vi.advanceTimersByTime(3000);
-    expect(store().downloads.has('file-1')).toBe(false);
-  });
-
-  it('clears a cancelled download sooner than a completed one', async () => {
-    vi.useFakeTimers();
-    const service = fakeService();
-    service.downloadFile.mockImplementation(async (_f, opts) => {
-      opts.onProgress(progress({ status: 'cancelled' }));
-    });
-    createDownloadServiceMock.mockReturnValue(service as never);
-
-    await store().startDownload(apiA, file);
-
-    expect(store().downloads.has('file-1')).toBe(true);
-    vi.advanceTimersByTime(2000);
-    expect(store().downloads.has('file-1')).toBe(false);
-  });
-
-  it('does not schedule removal for ordinary progress updates', async () => {
+  it.each(['completed', 'cancelled', 'error'] as const)(
+    'keeps a %s download on the list until it is removed',
+    async (status) => {
+      vi.useFakeTimers();
+      const service = fakeService();
+      service.downloadFile.mockImplementation(async (_f, opts) => {
+        opts.onProgress(
+          progress({ status, error: status === 'error' ? 'Network error' : undefined })
+        );
+      });
+      createDownloadServiceMock.mockReturnValue(service as never);
+
+      await store().startDownload(apiA, file);
+
+      // Nothing takes it away on a timer. A row that removes itself is a
+      // reason for a failure that disappears before anyone reads it, and the
+      // panel it sits on can be collapsed at the time. Uploads and deletes
+      // have always waited to be dismissed.
+      vi.advanceTimersByTime(60_000);
+      expect(store().downloads.has('file-1')).toBe(true);
+
+      store().removeDownload('file-1');
+      expect(store().downloads.has('file-1')).toBe(false);
+    }
+  );
+
+  it('leaves an in-flight download on the list', async () => {
     vi.useFakeTimers();
     const service = fakeService();
     service.downloadFile.mockImplementation(async (_f, opts) => {
@@ -213,9 +207,8 @@ describe('startDownload', () => {
     createDownloadServiceMock.mockReturnValue(service as never);
 
     await store().startDownload(apiA, file);
-    vi.advanceTimersByTime(10_000);
+    vi.advanceTimersByTime(60_000);
 
-    // An in-flight download must never disappear from the list on a timer.
     expect(store().downloads.has('file-1')).toBe(true);
   });
 

frontend/src/features/dashboard/stores/use-download-store.ts

@@ -19,10 +19,6 @@ import {
 } from '../services/download-service';
 import type { FileItem } from '../types/file';
 
-/** How long a finished row lingers before it clears itself from the list. */
-const COMPLETED_LINGER_MS = 3000;
-const CANCELLED_LINGER_MS = 2000;
-
 interface DownloadState {
   downloads: Map<string, DownloadProgress>;
   /**
@@ -77,19 +73,21 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
       return { downloads };
     }),
 
+  /**
+   * A settled row stays until someone removes it.
+   *
+   * Downloads used to clear themselves on a timer - three seconds for a
+   * completion, two for a cancel - which read as tidy until a failure needed
+   * the same treatment. A reason for the failure that takes itself off the
+   * screen is no use to anyone who was not looking at that moment, and the
+   * panel can be collapsed. Uploads and deletes have always waited to be
+   * dismissed; downloads now do too, and every settled row has a button on it.
+   */
   startDownload: async (api, file, handlers) => {
-    const { getService, setProgress, removeDownload } = get();
+    const { getService, setProgress } = get();
 
     await getService(api).downloadFile(file, {
-      onProgress: (progress) => {
-        setProgress(progress);
-        if (progress.status === 'cancelled') {
-          setTimeout(() => removeDownload(file.id), CANCELLED_LINGER_MS);
-        }
-      },
-      onComplete: (fileId) => {
-        setTimeout(() => removeDownload(fileId), COMPLETED_LINGER_MS);
-      },
+      onProgress: setProgress,
       onError: handlers?.onError,
     });
   },