fix: address PR review feedback

- Fix race conditions when rapidly switching projects
  - Added cancellation logic to prevent stale responses from updating state
  - Both project settings and init script loading now properly cancelled on unmount

- Improve error handling in custom icon upload
  - Added toast notifications for validation errors (file type, file size)
  - Added toast notifications for upload success/failure
  - Handle network errors gracefully with user feedback
  - Handle file reader errors
This commit is contained in:
Stefan de Vogelaere
2026-01-16 23:00:47 +01:00
parent 4e53215104
commit 6a23e6ce78
2 changed files with 72 additions and 18 deletions

View File

@@ -8,6 +8,7 @@ import { useAppStore } from '@/store/app-store';
import { IconPicker } from '@/components/layout/project-switcher/components/icon-picker'; import { IconPicker } from '@/components/layout/project-switcher/components/icon-picker';
import { getAuthenticatedImageUrl } from '@/lib/api-fetch'; import { getAuthenticatedImageUrl } from '@/lib/api-fetch';
import { getHttpApiClient } from '@/lib/http-api-client'; import { getHttpApiClient } from '@/lib/http-api-client';
import { toast } from 'sonner';
import type { Project } from '@/lib/electron'; import type { Project } from '@/lib/electron';
interface ProjectIdentitySectionProps { interface ProjectIdentitySectionProps {
@@ -61,11 +62,17 @@ export function ProjectIdentitySection({ project }: ProjectIdentitySectionProps)
// Validate file type // Validate file type
const validTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/webp']; const validTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/webp'];
if (!validTypes.includes(file.type)) { if (!validTypes.includes(file.type)) {
toast.error('Invalid file type', {
description: 'Please upload a PNG, JPG, GIF, or WebP image.',
});
return; return;
} }
// Validate file size (max 2MB for icons) // Validate file size (max 2MB for icons)
if (file.size > 2 * 1024 * 1024) { if (file.size > 2 * 1024 * 1024) {
toast.error('File too large', {
description: 'Please upload an image smaller than 2MB.',
});
return; return;
} }
@@ -74,20 +81,39 @@ export function ProjectIdentitySection({ project }: ProjectIdentitySectionProps)
// Convert to base64 // Convert to base64
const reader = new FileReader(); const reader = new FileReader();
reader.onload = async () => { reader.onload = async () => {
const base64Data = reader.result as string; try {
const result = await getHttpApiClient().saveImageToTemp( const base64Data = reader.result as string;
base64Data, const result = await getHttpApiClient().saveImageToTemp(
`project-icon-${file.name}`, base64Data,
file.type, `project-icon-${file.name}`,
project.path file.type,
); project.path
if (result.success && result.path) { );
handleCustomIconChange(result.path); if (result.success && result.path) {
handleCustomIconChange(result.path);
toast.success('Icon uploaded successfully');
} else {
toast.error('Failed to upload icon', {
description: result.error || 'Please try again.',
});
}
} catch (error) {
toast.error('Failed to upload icon', {
description: 'Network error. Please try again.',
});
} finally {
setIsUploadingIcon(false);
} }
};
reader.onerror = () => {
toast.error('Failed to read file', {
description: 'Please try again with a different file.',
});
setIsUploadingIcon(false); setIsUploadingIcon(false);
}; };
reader.readAsDataURL(file); reader.readAsDataURL(file);
} catch { } catch {
toast.error('Failed to upload icon');
setIsUploadingIcon(false); setIsUploadingIcon(false);
} }
}; };

View File

@@ -64,35 +64,48 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti
// Load project settings (including useWorktrees) when project changes // Load project settings (including useWorktrees) when project changes
useEffect(() => { useEffect(() => {
let isCancelled = false;
const currentPath = project.path;
const loadProjectSettings = async () => { const loadProjectSettings = async () => {
try { try {
const httpClient = getHttpApiClient(); const httpClient = getHttpApiClient();
const response = await httpClient.settings.getProject(project.path); const response = await httpClient.settings.getProject(currentPath);
// Avoid updating state if component unmounted or project changed
if (isCancelled) return;
if (response.success && response.settings) { if (response.success && response.settings) {
// Sync useWorktrees to store if it has a value // Sync useWorktrees to store if it has a value
if (response.settings.useWorktrees !== undefined) { if (response.settings.useWorktrees !== undefined) {
setProjectUseWorktrees(project.path, response.settings.useWorktrees); setProjectUseWorktrees(currentPath, response.settings.useWorktrees);
} }
// Also sync other settings to store // Also sync other settings to store
if (response.settings.showInitScriptIndicator !== undefined) { if (response.settings.showInitScriptIndicator !== undefined) {
setShowInitScriptIndicator(project.path, response.settings.showInitScriptIndicator); setShowInitScriptIndicator(currentPath, response.settings.showInitScriptIndicator);
} }
if (response.settings.defaultDeleteBranchWithWorktree !== undefined) { if (response.settings.defaultDeleteBranchWithWorktree !== undefined) {
setDefaultDeleteBranch(project.path, response.settings.defaultDeleteBranchWithWorktree); setDefaultDeleteBranch(currentPath, response.settings.defaultDeleteBranchWithWorktree);
} }
if (response.settings.autoDismissInitScriptIndicator !== undefined) { if (response.settings.autoDismissInitScriptIndicator !== undefined) {
setAutoDismissInitScriptIndicator( setAutoDismissInitScriptIndicator(
project.path, currentPath,
response.settings.autoDismissInitScriptIndicator response.settings.autoDismissInitScriptIndicator
); );
} }
} }
} catch (error) { } catch (error) {
console.error('Failed to load project settings:', error); if (!isCancelled) {
console.error('Failed to load project settings:', error);
}
} }
}; };
loadProjectSettings(); loadProjectSettings();
return () => {
isCancelled = true;
};
}, [ }, [
project.path, project.path,
setProjectUseWorktrees, setProjectUseWorktrees,
@@ -103,12 +116,19 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti
// Load init script content when project changes // Load init script content when project changes
useEffect(() => { useEffect(() => {
let isCancelled = false;
const currentPath = project.path;
const loadInitScript = async () => { const loadInitScript = async () => {
setIsLoading(true); setIsLoading(true);
try { try {
const response = await apiGet<InitScriptResponse>( const response = await apiGet<InitScriptResponse>(
`/api/worktree/init-script?projectPath=${encodeURIComponent(project.path)}` `/api/worktree/init-script?projectPath=${encodeURIComponent(currentPath)}`
); );
// Avoid updating state if component unmounted or project changed
if (isCancelled) return;
if (response.success) { if (response.success) {
const content = response.content || ''; const content = response.content || '';
setScriptContent(content); setScriptContent(content);
@@ -116,13 +136,21 @@ export function WorktreePreferencesSection({ project }: WorktreePreferencesSecti
setScriptExists(response.exists); setScriptExists(response.exists);
} }
} catch (error) { } catch (error) {
console.error('Failed to load init script:', error); if (!isCancelled) {
console.error('Failed to load init script:', error);
}
} finally { } finally {
setIsLoading(false); if (!isCancelled) {
setIsLoading(false);
}
} }
}; };
loadInitScript(); loadInitScript();
return () => {
isCancelled = true;
};
}, [project.path]); }, [project.path]);
// Save script // Save script