Conversation
…in DragAndDropZone
There was a problem hiding this comment.
Pull request overview
Refactors the client-side image conversion flow by extracting upload/validation and algorithm-application logic into dedicated hooks and utilities, while updating UI components to use the new abstractions.
Changes:
- Introduced shared utilities for algorithm application/pixel extraction and for file type validation/rejection formatting.
- Extracted image upload and processing logic into
useFileUploadanduseImageProcessinghooks. - Updated image conversion UI components, including adding JSROOT cleanup on unmount and simplifying
ImageConverter.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/utils/imageConversion.ts | Centralizes algorithm execution and pixel extraction from WASM Image. |
| client/src/utils/fileValidation.ts | Defines accepted/allowed file types and formats drop rejection reasons. |
| client/src/hooks/useImageProcessing.ts | Implements conversion pipeline (progress + algorithm loop) as a hook. |
| client/src/hooks/useFileUpload.ts | Handles file validation + WASM image loading/extraction as a hook. |
| client/src/components/images/ImageJSRootPreview.tsx | Adds JSROOT cleanup during effect teardown to avoid leaked render state. |
| client/src/components/images/ImageConverter.tsx | Simplifies the converter UI by delegating to the new hooks/utils. |
| client/src/components/common/DragAndDropZone.tsx | Internalizes rejected-file error handling and displays a local error banner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
client/src/hooks/useFileUpload.ts
Outdated
| return; | ||
| } | ||
| if (!isAllowedFileType(file)){ | ||
| setErrorMessage(' Unsupported image type. Supported types are: PNG, TIFF, JPEG'); |
There was a problem hiding this comment.
The error message string has a leading whitespace character, which will show up in the UI (" Unsupported..."). Remove the leading space so the message renders cleanly.
| setErrorMessage(' Unsupported image type. Supported types are: PNG, TIFF, JPEG'); | |
| setErrorMessage('Unsupported image type. Supported types are: PNG, TIFF, JPEG'); |
| } | ||
| }, | ||
| onDropRejected: (rejectedFiles) => { | ||
| if (!onFileReject) return; | ||
| try { | ||
| onFileReject(rejectedFiles); | ||
| } catch (error) { | ||
| console.error('Error handling rejected files:', error); | ||
| } | ||
| if (rejectedFiles.length === 0) return; | ||
| setLocalError(`Rejected: ${formatRejectionReasons(rejectedFiles)}`); | ||
| }, |
There was a problem hiding this comment.
localError is set on onDropRejected, but it is never cleared on a subsequent successful drop, so the rejection banner can persist even after the user uploads a valid file. Clear localError in onDropAccepted (and/or when a new drag starts) so the UI reflects the current state.
| // this will interrupt the processing and user would have to re-run the conversion (maybe it should be changed or at least a warning should be added in UI) | ||
| if(!imageState.imageToConvert){ | ||
| const imageToConvert = load_image(imageState.rawBytes); | ||
| setImageState((prev) => ({...prev, imageToConvert })); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This branch loads imageToConvert and then immediately returns, forcing the user to click Run a second time if rawBytes is set but imageToConvert is null. Since useFileUpload already sets imageToConvert alongside rawBytes, this is likely dead code; consider either removing it or continuing processing after creating imageToConvert (instead of returning) to avoid the double-run behavior if the state ever gets out of sync.
simplified client code and fixed some problems