From 8d6446bdd642e526725663a5ea879a1c1eefb0dd Mon Sep 17 00:00:00 2001 From: Arnaud_Cayrol Date: Wed, 11 Feb 2026 22:17:26 +0100 Subject: [PATCH] Frontend code cleanup, remove dead code --- .gitignore | 7 + frontend/REFACTOR_SUMMARY.md | 330 ------------------ frontend/src/App.svelte | 23 +- frontend/src/lib/asyncState.js | 116 ------ .../lib/components/ConnectionStatus.svelte | 4 +- .../src/lib/components/GalleryView.svelte | 42 ++- .../src/lib/components/OutputManager.svelte | 1 + .../src/lib/components/PeopleSelector.svelte | 2 +- .../lib/components/ProcessingControls.svelte | 9 - .../src/lib/components/ProgressDisplay.svelte | 4 +- .../src/lib/components/ResultsView.svelte | 1 + .../src/lib/components/SettingsPanel.svelte | 25 +- .../visual/BrightnessIndicator.svelte | 7 +- .../lib/components/visual/EyeIndicator.svelte | 4 +- frontend/src/lib/constants.js | 79 ----- frontend/src/lib/errorHandler.js | 78 ----- 16 files changed, 71 insertions(+), 661 deletions(-) delete mode 100644 frontend/REFACTOR_SUMMARY.md delete mode 100644 frontend/src/lib/asyncState.js diff --git a/.gitignore b/.gitignore index 39f681c..4d8ed18 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,13 @@ output/ # Configuration with secrets config.toml +.env + +# ONNX models (large binaries) +models/ # OS .DS_Store + +# Debug/temp artifacts +*.dot diff --git a/frontend/REFACTOR_SUMMARY.md b/frontend/REFACTOR_SUMMARY.md deleted file mode 100644 index 62dc088..0000000 --- a/frontend/REFACTOR_SUMMARY.md +++ /dev/null @@ -1,330 +0,0 @@ -# Frontend Refactor - Priority 1 Fixes - -## Summary - -Completed all Priority 1 fixes from the code review: -1. ✅ Extracted `sanitizeFolderName` to shared utility -2. ✅ Extracted `formatSize` to shared utility -3. ✅ Refactored folder state management (eliminated circular flow) - -## Changes Made - -### 1. Created Shared Utilities (`src/lib/utils.js`) - -**New file:** `/frontend/src/lib/utils.js` - -Extracted two functions that were duplicated across components: - -- **`sanitizeFolderName(name, id)`** - Previously duplicated in: - - App.svelte (lines 65-84) - - ProcessingControls.svelte (lines 15-34) - -- **`formatSize(bytes)`** - Previously duplicated in: - - ProcessingControls.svelte (lines 113-117) - - OutputManager.svelte (lines 84-88) - - GalleryView.svelte (lines 123-127) - -**Impact:** -- Eliminated 5 instances of duplicated code (~50 lines total) -- Single source of truth for these utilities -- Easier maintenance and testing -- Functions properly documented with JSDoc - -### 2. Refactored Folder State Management - -**Problem:** Circular data flow created complexity and potential race conditions: -``` -OutputManager (loads) → App (stores) → ProcessingControls (consumes) -``` - -**Solution:** Simplified to unidirectional flow: -``` -App (loads & stores) → OutputManager (displays) + ProcessingControls (consumes) -``` - -**Changes:** - -#### App.svelte -- **Added:** `loadOutputFolders()` function to fetch folder list -- **Added:** `$effect` to load folders when connection established -- **Modified:** `handleFolderDeleted()` now calls `loadOutputFolders()` to refresh -- **Removed:** `outputFolderRefreshKey` state variable (no longer needed) -- **Removed:** Duplicated `sanitizeFolderName()` function -- **Simplified:** OutputManager props - now just passes `folders` directly - -#### OutputManager.svelte -- **Removed:** Internal `folders` state (now receives as prop) -- **Removed:** `loading` and `error` state (no longer manages fetching) -- **Removed:** `loadFolders()` function (parent handles this) -- **Removed:** `refreshKey` effect (parent controls updates) -- **Removed:** `onFoldersLoaded` callback (no longer needed) -- **Removed:** Refresh button from UI (folders auto-refresh) -- **Removed:** Unused CSS for refresh button and error states -- **Simplified:** Component is now purely presentational - -#### ProcessingControls.svelte -- **Removed:** Duplicated `sanitizeFolderName()` function -- **Removed:** Duplicated `formatSize()` function -- **Added:** Import from shared utils - -#### GalleryView.svelte -- **Removed:** Duplicated `formatSize()` function -- **Added:** Import from shared utils - -### 3. Benefits - -**Before:** -- 5 copies of utility functions -- Circular state flow between components -- OutputManager managed its own loading state -- Manual refresh button required -- Multiple sources of truth for folder data - -**After:** -- Single copy of each utility in shared location -- Clear unidirectional data flow (App → children) -- OutputManager is a pure presentation component -- Automatic refresh when folders change -- Single source of truth (App state) - -**Code Quality Improvements:** -- **DRY:** No duplicated logic -- **Maintainability:** Changes to utilities happen in one place -- **Testability:** Utilities can be unit tested independently -- **Clarity:** Clear ownership of state (App owns, children display/consume) -- **Reliability:** No race conditions from multiple components fetching - -### 4. Build Status - -✅ **Frontend builds successfully** with no errors - -Warnings (pre-existing, unrelated to refactor): -- Accessibility warnings in SettingsPanel (label associations) -- State reference warning in PeopleSelector (intended behavior) - -### 5. Testing Recommendations - -Manual testing should verify: -1. ✅ Output folders list displays correctly -2. ✅ Folders refresh after job completion -3. ✅ Folders refresh after deletion -4. ✅ Folder deletion warning shows correct size/count -5. ✅ No console errors related to folder loading -6. ✅ ProcessingControls shows "already exists" warning correctly - -## Files Modified - -- **Created:** - - `frontend/src/lib/utils.js` (42 lines) - -- **Modified:** - - `frontend/src/App.svelte` (-28 lines, refactored state management) - - `frontend/src/lib/components/ProcessingControls.svelte` (-29 lines) - - `frontend/src/lib/components/OutputManager.svelte` (-63 lines, major simplification) - - `frontend/src/lib/components/GalleryView.svelte` (-8 lines) - -**Total:** ~120 lines of code removed, architecture significantly improved - ---- - -# Frontend Refactor - Priority 2 Fixes - -## Summary - -Completed all Priority 2 fixes (2026-02-08): -1. ✅ Moved global CSS out of App.svelte -2. ✅ Created constants file for magic values -3. ✅ Standardized error handling -4. ✅ Added loading states to all async buttons - -## Changes Made - -### 1. Global CSS Organization - -**Created:** `frontend/src/styles/global.css` -- Moved global reset styles (box-sizing, margin, padding) -- Moved body styles (font-family, background, color, line-height) - -**Updated:** `frontend/src/main.js` -- Added import for global CSS file - -**Updated:** `frontend/src/App.svelte` -- Removed `:global(*)` and `:global(body)` styles -- Component-specific styles remain - -**Benefits:** -- Clear separation of global vs component styles -- Single location for global style changes -- Follows standard Svelte/Vite patterns - -### 2. Constants Consolidation - -**Created:** `frontend/src/lib/constants.js` - -Centralized all magic values into organized exports: -- `COLORS` - All color values (backgrounds, text, borders, status) -- `SPACING` - Standard spacing scale -- `RADIUS` - Border radius values -- `TRANSITIONS` - Animation transitions -- `FONT_SIZES` - Typography scale -- `Z_INDEX` - Z-index layering system -- `TIMING` - Timing values (timeouts, delays, durations) -- `STORAGE_KEYS` - LocalStorage keys -- `API` - API endpoint paths -- `WS` - WebSocket configuration -- `JOB_STATUS` - Job status constants -- `DEFAULT_CONFIG` - Default configuration (moved from SettingsPanel) - -**Components Updated:** -- `App.svelte` - Uses STORAGE_KEYS, WS, JOB_STATUS -- `ProcessingControls.svelte` - Uses JOB_STATUS -- `SettingsPanel.svelte` - Uses DEFAULT_CONFIG, TIMING (removed 40+ lines of duplicate config) - -**Benefits:** -- Single source of truth for all magic values -- Easy to update colors, spacing, timing globally -- Better discoverability -- Eliminates duplicate constant definitions - -### 3. Standardized Error Handling - -**Created:** `frontend/src/lib/errorHandler.js` - -Utilities for consistent error handling: -- `parseError()` - Parse errors from Response or Error objects -- `handleError()` - Handle errors with console logging -- `showErrorAlert()` - Display error in alert dialog -- `fetchWithErrorHandling()` - Fetch wrapper with error handling -- `safeJsonParse()` - Safe JSON parsing with fallback -- `executeAsync()` - Generic async operation wrapper -- `ErrorMode` enum - Error display modes - -**Components Updated (all use standardized error handling):** -- `App.svelte` -- `ProcessingControls.svelte` -- `SettingsPanel.svelte` -- `OutputManager.svelte` -- `GalleryView.svelte` -- `ConnectionStatus.svelte` -- `PeopleSelector.svelte` - -**Error Handling Pattern:** - -Before: -```javascript -try { - const res = await fetch('/api/endpoint'); - if (!res.ok) { - const data = await res.json().catch(() => ({})); - throw new Error(data.message || 'Operation failed'); - } -} catch (e) { - console.error('Operation failed:', e); - error = e.message; -} -``` - -After: -```javascript -try { - const res = await fetch('/api/endpoint'); - if (!res.ok) throw res; -} catch (e) { - error = await handleError('Operation failed', e); -} -``` - -**Benefits:** -- Consistent error messages across the application -- Automatic error logging -- Flexible error display (alert, console, inline) -- Proper parsing of Response objects with JSON error messages -- Less boilerplate code - -### 4. Async Button Loading States - -**Created:** `frontend/src/lib/asyncState.js` -- `createAsyncState()` - Async state manager using Svelte 5 runes -- `getButtonText()` - Get button text based on loading state -- `createAsyncHandler()` - Create async handler with loading state - -**Components Updated:** -- `ProcessingControls.svelte` - Added `starting` state, button shows "Starting..." -- `SettingsPanel.svelte` - Already had `saving` state (verified working) -- `OutputManager.svelte` - Already had `deleting` state (verified working) -- `GalleryView.svelte` - Already had `deleting` and `compiling` states (verified working) - -**Loading State Pattern:** -```javascript -let loading = $state(false); - -async function doSomething() { - loading = true; - try { - await someAsyncOperation(); - } finally { - loading = false; - } -} - -// In template: - -``` - -**Benefits:** -- Clear visual feedback during async operations -- Prevents double-clicks and race conditions -- Better user experience -- Consistent loading patterns -- Reusable utilities for future components - -## File Structure - -``` -frontend/src/ -├── styles/ -│ └── global.css # NEW: Global styles -├── lib/ -│ ├── components/ # UPDATED: All components -│ ├── constants.js # NEW: Application constants -│ ├── errorHandler.js # NEW: Error handling utilities -│ ├── asyncState.js # NEW: Async state management -│ └── utils.js # EXISTING: Utility functions -├── App.svelte # UPDATED: Uses constants -└── main.js # UPDATED: Imports global CSS -``` - -## Build Status - -✅ **Frontend builds successfully** (`npm run build`) - -All changes verified: -- No runtime errors -- All async operations have loading states -- Error handling is consistent -- Constants are properly imported and used - -## Code Quality Improvements - -**Before Priority 2:** -- Global styles mixed with component styles -- Magic values duplicated across files (colors, timing, etc.) -- Inconsistent error handling (alert, console.error, inline) -- Some async buttons lacked loading states - -**After Priority 2:** -- Global styles in dedicated file -- All magic values in centralized constants file -- Standardized error handling with utilities -- All async buttons have proper loading states - -## Future Improvements - -Suggestions for further enhancement: -1. **Toast Notifications** - Replace `alert()` calls with toast system -2. **Error Boundary** - Add Svelte error boundary for component errors -3. **Retry Logic** - Add exponential backoff retry for failed requests -4. **Loading Skeletons** - Replace loading text with skeleton screens -5. **CSS Variables** - Convert constants to CSS custom properties for theming diff --git a/frontend/src/App.svelte b/frontend/src/App.svelte index 691ac94..7dabf5a 100644 --- a/frontend/src/App.svelte +++ b/frontend/src/App.svelte @@ -1,5 +1,5 @@