From 2f9a7858e2e4195444e2e634fd8ce44c6c4b7e0c Mon Sep 17 00:00:00 2001 From: Cristhian Zanforlin Lousa Date: Wed, 15 Jan 2025 15:03:39 -0300 Subject: [PATCH] refactor: Improve createFileUpload reliability and performance (#5697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ (create-file-upload.ts): Refactor createFileUpload function to improve readability and maintainability. Move input element creation and event listeners to separate functions for better separation of concerns. Use module-level variables for input element and promise resolver to handle cleanup and resolution more effectively. * ✨ (create-file-upload.ts): refactor createFileUpload function to improve code readability and maintainability. Remove unnecessary module-level variables and simplify the logic for handling file uploads. * 📝 (create-file-upload.ts): refactor createFileUpload function to improve code readability and maintainability 🐛 (create-file-upload.ts): fix issue with resolving promise multiple times and improve file input handling * 🔧 (create-file-upload.ts): refactor createFileUpload function to improve file upload process and cleanup event listeners for better performance and code readability * 📝 (index.tsx): Remove unused useRef import and fileInputRef variable ♻️ (index.tsx): Refactor checkFileType function to use a for loop for better readability and performance ✨ (index.tsx): Introduce createFileUpload helper function to handle file selection and validation 📝 (index.tsx): Update handleButtonClick function to use createFileUpload helper function and improve file handling logic 📝 (index.tsx): Update handleButtonClick function to handle file upload and error messages more efficiently 📝 (index.tsx): Update input element to display file name and handle file selection more effectively * add error handling in case document.body.removeChild Fails --------- Co-authored-by: anovazzi1 --- .../components/inputFileComponent/index.tsx | 125 ++++++++---------- .../src/helpers/create-file-upload.ts | 53 +++++--- 2 files changed, 85 insertions(+), 93 deletions(-) diff --git a/src/frontend/src/components/core/parameterRenderComponent/components/inputFileComponent/index.tsx b/src/frontend/src/components/core/parameterRenderComponent/components/inputFileComponent/index.tsx index dd52afb16..7ec0ab7fc 100644 --- a/src/frontend/src/components/core/parameterRenderComponent/components/inputFileComponent/index.tsx +++ b/src/frontend/src/components/core/parameterRenderComponent/components/inputFileComponent/index.tsx @@ -1,7 +1,8 @@ import { usePostUploadFile } from "@/controllers/API/queries/files/use-post-upload-file"; +import { createFileUpload } from "@/helpers/create-file-upload"; import useFileSizeValidator from "@/shared/hooks/use-file-size-validator"; import { cn } from "@/utils/utils"; -import { useEffect, useRef } from "react"; +import { useEffect } from "react"; import { CONSOLE_ERROR_MSG, INVALID_FILE_ALERT, @@ -23,77 +24,70 @@ export default function InputFileComponent({ const currentFlowId = useFlowsManagerStore((state) => state.currentFlowId); const setErrorData = useAlertStore((state) => state.setErrorData); const { validateFileSize } = useFileSizeValidator(setErrorData); - const fileInputRef = useRef(null); // Clear component state useEffect(() => { if (disabled && value !== "") { handleOnNewValue({ value: "", file_path: "" }, { skipSnapshot: true }); } - }, [disabled, handleOnNewValue, value]); + }, [disabled, handleOnNewValue]); function checkFileType(fileName: string): boolean { - if (!fileTypes?.length) return true; - return fileTypes.some((type) => - fileName.toLowerCase().endsWith(type.toLowerCase()), - ); + if (fileTypes === undefined) return true; + for (let index = 0; index < fileTypes.length; index++) { + if (fileName.endsWith(fileTypes[index])) { + return true; + } + } + return false; } const { mutate, isPending } = usePostUploadFile(); - const handleFileSelection = (file: File | null) => { - if (!file) { - setErrorData({ - title: "Error selecting file", - list: ["No file was selected"], - }); - return; - } + const handleButtonClick = (): void => { + createFileUpload({ multiple: false, accept: fileTypes?.join(",") }).then( + (files) => { + const file = files[0]; + if (file) { + if (!validateFileSize(file)) { + return; + } - if (!validateFileSize(file)) { - return; - } + if (checkFileType(file.name)) { + // Upload the file + mutate( + { file, id: currentFlowId }, + { + onSuccess: (data) => { + // Get the file name from the response + const { file_path } = data; - if (!checkFileType(file.name)) { - setErrorData({ - title: INVALID_FILE_ALERT, - list: [fileTypes?.join(", ") || ""], - }); - return; - } - - mutate( - { file, id: currentFlowId }, - { - onSuccess: (data) => { - const { file_path } = data; - handleOnNewValue({ value: file.name, file_path }); - }, - onError: (error) => { - console.error(CONSOLE_ERROR_MSG); - setErrorData({ - title: "Error uploading file", - list: [error.response?.data?.detail || "Unknown error occurred"], - }); - }, + // sets the value that goes to the backend + // Update the state and on with the name of the file + // sets the value to the user + handleOnNewValue({ value: file.name, file_path }); + }, + onError: (error) => { + console.error(CONSOLE_ERROR_MSG); + setErrorData({ + title: "Error uploading file", + list: [error.response?.data?.detail], + }); + }, + }, + ); + } else { + // Show an error if the file type is not allowed + setErrorData({ + title: INVALID_FILE_ALERT, + list: [fileTypes?.join(", ") || ""], + }); + } + } }, ); }; - const handleButtonClick = () => { - fileInputRef.current?.click(); - }; - - const handleNativeInputChange = ( - event: React.ChangeEvent, - ) => { - const file = event.target.files?.[0] || null; - handleFileSelection(file); - if (event.target) { - event.target.value = ""; - } - }; - const isDisabled = disabled || isPending; return ( @@ -102,29 +96,18 @@ export default function InputFileComponent({
- - e.stopPropagation()} + onClick={handleButtonClick} />
diff --git a/src/frontend/src/helpers/create-file-upload.ts b/src/frontend/src/helpers/create-file-upload.ts index 196716756..6f309499e 100644 --- a/src/frontend/src/helpers/create-file-upload.ts +++ b/src/frontend/src/helpers/create-file-upload.ts @@ -1,60 +1,69 @@ -export async function createFileUpload(props?: { +export function createFileUpload(props?: { accept?: string; multiple?: boolean; }): Promise { return new Promise((resolve) => { + // Create input element const input = document.createElement("input"); input.type = "file"; + input.style.position = "fixed"; + input.style.top = "0"; + input.style.left = "0"; + input.style.opacity = "0.001"; + input.style.pointerEvents = "none"; input.accept = props?.accept ?? ".json"; input.multiple = props?.multiple ?? true; - input.style.display = "none"; - let isResolved = false; + let isHandled = false; const cleanup = () => { - // Check if the input element still exists in the DOM before attempting to remove it - if (input && document.body.contains(input)) { + if (document.body.contains(input)) { try { document.body.removeChild(input); } catch (error) { console.warn("Error removing input element:", error); } } - window.removeEventListener("focus", handleFocus); + input.removeEventListener("change", handleChange); + document.removeEventListener("focus", handleFocus); }; - const handleChange = (e: Event) => { - if (!isResolved) { - isResolved = true; - const files = Array.from((e.target as HTMLInputElement).files!); - cleanup(); - resolve(files); - } + const handleChange = (event: Event) => { + if (isHandled) return; + isHandled = true; + + const files = Array.from((event.target as HTMLInputElement).files || []); + cleanup(); + resolve(files); }; const handleFocus = () => { setTimeout(() => { - if (!isResolved) { - isResolved = true; + if (!isHandled) { + isHandled = true; cleanup(); resolve([]); } - }, 300); + }, 100); }; input.addEventListener("change", handleChange); - window.addEventListener("focus", handleFocus); + document.addEventListener("focus", handleFocus); document.body.appendChild(input); - input.click(); - // Fallback timeout to ensure resolution + requestAnimationFrame(() => { + if (!isHandled) { + input.click(); + } + }); + setTimeout(() => { - if (!isResolved) { - isResolved = true; + if (!isHandled) { + isHandled = true; cleanup(); resolve([]); } - }, 60000); + }, 30000); }); }