fix: address PR review issues in upload and save features

- Replace deprecated substr() with substring() in UploadContext
- Fix broken error handling that checked stale task status
- Fix missing useEffect dependency in UploadStatus
- Fix CSS class conflict in progress bar styling
- Add error recovery for save state in Editor (reset on failure)
- Use .finally() instead of .then() to ensure refresh on upload failure
- Fix inconsistent indentation in UploadContext

Author: ZimengXiong

frontend/src/components/UploadStatus.tsx

@@ -10,7 +10,7 @@ export const UploadStatus: React.FC = () => {
 
   // Auto-open when upload starts
   useEffect(() => {
-    if (isUploading && !isOpen) {
+    if (isUploading) {
       setIsOpen(true);
     }
   }, [isUploading]);
@@ -84,8 +84,7 @@ export const UploadStatus: React.FC = () => {
                       <div 
                         className={clsx(
                           "h-full transition-all duration-300 ease-out rounded-full",
-                          task.status === 'error' ? "bg-rose-500" : "bg-indigo-600",
-                          task.status === 'success' && "bg-emerald-500"
+                          task.status === 'error' ? "bg-rose-500" : task.status === 'success' ? "bg-emerald-500" : "bg-indigo-600"
                         )}
                         style={{ width: `${task.progress}%` }}
                       />

frontend/src/context/UploadContext.tsx

@@ -48,42 +48,34 @@ export const UploadProvider: React.FC<{ children: ReactNode }> = ({ children })
 
   const uploadFiles = useCallback(async (files: File[], targetCollectionId: string | null) => {
     const newTasks: UploadTask[] = files.map(f => ({
-      id: Math.random().toString(36).substr(2, 9),
+      id: Math.random().toString(36).substring(2, 11),
       fileName: f.name,
       status: 'pending',
       progress: 0
     }));
 
     setTasks(prev => [...newTasks, ...prev]);
 
-    // Process files
-    // Note: We are using a modified importDrawings that will be updated to accept onProgress
-    // We map the tasks to the files for the callback
-    const fileTaskMap = new Map<string, string>(); // fileName -> taskId
+    // Map file names to task IDs for progress callbacks
+    const fileTaskMap = new Map<string, string>();
     newTasks.forEach(t => fileTaskMap.set(t.fileName, t.id));
 
-    // We start all uploads. The Utils will handle the actual async work.
-    // For now we assume sequential or parallel is handled by util, but we need to pass a callback per file or a global one.
-
     const handleProgress = (fileName: string, status: UploadStatus, progress: number, error?: string) => {
-        const taskId = fileTaskMap.get(fileName);
-        if (taskId) {
-            updateTask(taskId, { status, progress, error });
-        }
+      const taskId = fileTaskMap.get(fileName);
+      if (taskId) {
+        updateTask(taskId, { status, progress, error });
+      }
     };
 
     try {
-        await importDrawings(files, targetCollectionId, undefined, handleProgress);
+      await importDrawings(files, targetCollectionId, undefined, handleProgress);
     } catch (e) {
-        console.error("Global upload error", e);
-        // Mark pending as error if something crashed completely
-        newTasks.forEach(t => {
-            if (t.status === 'pending' || t.status === 'uploading') {
-                updateTask(t.id, { status: 'error', error: 'Upload failed unexpectedly' });
-            }
-        });
+      console.error("Global upload error", e);
+      // Mark all new tasks as error if something crashed completely
+      newTasks.forEach(t => {
+        updateTask(t.id, { status: 'error', error: 'Upload failed unexpectedly' });
+      });
     }
-
   }, [updateTask]);
 
   return (

frontend/src/pages/Dashboard.tsx

@@ -307,10 +307,9 @@ export const Dashboard: React.FC = () => {
     const targetCollectionId = selectedCollectionId === undefined ? null : selectedCollectionId;
 
     // Use the global upload context
-    uploadFiles(fileArray, targetCollectionId).then(() => {
-       // Refresh after upload is presumably done or started
-       // Since uploadFiles waits for all, we can refresh here.
-       refreshData();
+    uploadFiles(fileArray, targetCollectionId).finally(() => {
+      // Refresh after all uploads complete (success or failure)
+      refreshData();
     });
   };
 
@@ -534,8 +533,8 @@ export const Dashboard: React.FC = () => {
 
       const drawingFiles = files.filter(f => !f.name.endsWith('.excalidrawlib'));
       if (drawingFiles.length > 0) {
-        uploadFiles(drawingFiles, targetCollectionId).then(() => {
-            refreshData();
+        uploadFiles(drawingFiles, targetCollectionId).finally(() => {
+          refreshData();
         });
       }
 

frontend/src/pages/Editor.tsx

@@ -593,26 +593,28 @@ export const Editor: React.FC = () => {
 
   const handleBackClick = async () => {
     if (isSavingOnLeave) return; // Prevent double clicks
-    
+
     setIsSavingOnLeave(true);
 
     // Save drawing and generate preview before navigating
-    if (excalidrawAPI.current && saveDataRef.current && savePreviewRef.current) {
-      const elements = excalidrawAPI.current.getSceneElementsIncludingDeleted();
-      const appState = excalidrawAPI.current.getAppState();
-      const files = excalidrawAPI.current.getFiles() || {};
-      latestElementsRef.current = elements;
-      latestFilesRef.current = files;
-      
-      try {
+    try {
+      if (excalidrawAPI.current && saveDataRef.current && savePreviewRef.current) {
+        const elements = excalidrawAPI.current.getSceneElementsIncludingDeleted();
+        const appState = excalidrawAPI.current.getAppState();
+        const files = excalidrawAPI.current.getFiles() || {};
+        latestElementsRef.current = elements;
+        latestFilesRef.current = files;
+
         await Promise.all([
           saveDataRef.current(elements, appState),
           savePreviewRef.current(elements, appState, files)
         ]);
         console.log("[Editor] Saved on back navigation", { drawingId: id });
-      } catch (err) {
-        console.error('Failed to save on back navigation', err);
       }
+    } catch (err) {
+      console.error('Failed to save on back navigation', err);
+    } finally {
+      setIsSavingOnLeave(false);
     }
     navigate('/');
   };