Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/fe/image upload component #1737

Draft
wants to merge 28 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2f80a9a
create ImageUpload component and extract logic from profilepanel
vishnusn77 Feb 10, 2025
2670375
update profilepanel to use the imageupload component
vishnusn77 Feb 10, 2025
edd98bf
pass theme prop to GenericDialog in ImageUpload component
vishnusn77 Feb 10, 2025
223cad0
resolve prop validation errors in GenericDialog and ImageField
vishnusn77 Feb 10, 2025
41733d2
add logic to handle base64 images and placeholders
vishnusn77 Feb 10, 2025
e9afb90
update the rendering logic to use the new props
vishnusn77 Feb 10, 2025
8c3c23e
replace and comment image component usage for testing
vishnusn77 Feb 10, 2025
81de3b4
remove box material from imageupload component
vishnusn77 Feb 10, 2025
29d142c
revert back image component and usage
vishnusn77 Feb 11, 2025
d5ecf87
add drag and drop event handlers to imageupload
vishnusn77 Feb 11, 2025
b2237e6
replace the usage of imagefield
vishnusn77 Feb 11, 2025
c8b1fb7
display upload icon and message in drag drop field
vishnusn77 Feb 11, 2025
05854f5
replace input with textfield material
vishnusn77 Feb 11, 2025
c3f3353
add validation for files
vishnusn77 Feb 11, 2025
419d982
fix preview image display issue
vishnusn77 Feb 11, 2025
b3599c1
remove imagefield component and replace the usages with imageupload
vishnusn77 Feb 11, 2025
dfcfee4
fix image reupload issue by triggerring state change
vishnusn77 Feb 11, 2025
5b97b19
set drag and drop event handlers correctly
vishnusn77 Feb 11, 2025
eedfdd3
add prop to handle validation error
vishnusn77 Feb 11, 2025
808cc92
added support for props to standardize API
vishnusn77 Feb 11, 2025
3917b8b
remove index.css file
vishnusn77 Feb 13, 2025
1368ffa
remove internal error handling from imageupload component
vishnusn77 Feb 13, 2025
43be0a7
merge drag and drop handling into single function
vishnusn77 Feb 13, 2025
d4d78e8
optimise imageupload component API
vishnusn77 Feb 13, 2025
28c4215
refactor max size to configure by prop
vishnusn77 Feb 13, 2025
7657cd5
replace hardcoded strings with i18next
vishnusn77 Feb 13, 2025
ea18f50
add jsdoc for imageupload component
vishnusn77 Feb 13, 2025
a4f54f7
error handling prop bug fix
vishnusn77 Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
307 changes: 307 additions & 0 deletions Client/src/Components/ImageUpload/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,307 @@
import { useState, useRef } from "react";
import { Box, Button, Stack, IconButton , Typography } from "@mui/material";
import ProgressUpload from "../ProgressBars";
import { formatBytes } from "../../Utils/fileUtils";
import { imageValidation } from "../../Validation/validation";
import ImageIcon from "@mui/icons-material/Image";
import {GenericDialog} from "../Dialog/genericDialog";
import CloudUploadIcon from "@mui/icons-material/CloudUpload";
import { checkImage } from "../../Utils/fileUtils";
import { useTheme } from "@mui/material/styles";
import { useTranslation } from "react-i18next";

/**
* @component
* @param {Object} props - Component properties.
* @param {boolean} props.open - Controls whether the upload modal is open.
* @param {function} props.onClose - Function to close the modal.
* @param {function} props.onUpdate - Callback function triggered when an image is successfully uploaded.
* @param {string} [props.placeholder] - Optional placeholder image (URL or base64).
* @param {number} [props.maxSize=3145728] - Maximum allowed file size in bytes (default: 3MB).
* @param {string[]} [props.acceptedTypes] - List of allowed image MIME types (e.g., `["image/png", "image/jpeg"]`).
* @param {number} [props.previewSize=150] - Size of the image preview in pixels.
* @param {function} [props.setErrors] - Function to set error messages in the parent component.
* @param {Object} [props.errors={}] - Object containing validation errors.
*
* @example
* <ImageUpload
* open={isOpen}
* onClose={setIsOpen(false)}
* onUpdate={(imageUrl) => console.log("Image uploaded:", imageUrl)}
* maxSize={5 * 1024 * 1024} // 5MB
* acceptedTypes={["image/png", "image/jpeg"]}
* />
*/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include JSdoc for the component, see other components for examples

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave comments unresolved so other reviewers can see what's been happening and changes made 👍

const isValidBase64Image = (data) => {
return /^[A-Za-z0-9+/=]+$/.test(data);
};

const DEFAULT_MAX_SIZE = 3 * 1024 * 1024; // 3MB

const ImageUpload = ({ open, onClose, onUpdate, placeholder, maxSize = DEFAULT_MAX_SIZE, acceptedTypes, previewSize = 150, setErrors, errors = {}}) => {

const [file, setFile] = useState();
const [progress, setProgress] = useState({ value: 0, isLoading: false });
const [isDragging, setIsDragging] = useState(false);
const intervalRef = useRef(null);
const theme = useTheme();
const UPLOAD_PROGRESS_INCREMENT = 12; // Controls the speed of upload progress
const UPLOAD_PROGRESS_INTERVAL = 120; // Controls how often progress updates (milliseconds)
const UPLOAD_COMPLETE = 100; // Represents the max progress percentage
const { t } = useTranslation();

// Handle base64 and placeholder logic
let imageSrc = placeholder || ""; // Default to placeholder or empty string

if (file?.src) {
imageSrc = file.src; // Use uploaded file's preview
} else if (placeholder && isValidBase64Image(placeholder)) {
imageSrc = `data:image/png;base64,${placeholder}`; // Convert base64 string if valid
} else {
imageSrc = ""; // Ensure it's an empty string instead of "undefined"
}

const handleDrag = (event, isDragging) => {
event.preventDefault();
setIsDragging(isDragging);

if (event.type === "drop" && event.dataTransfer.files.length > 0) {
handlePicture({ target: { files: event.dataTransfer.files } });
}
};

// Handles image file selection
const handlePicture = (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name can be a bit confusing here. The parameter expects an event, but we are passing an object that contains part of the event, I think we should rename that

const pic = event.target.files[0];
if (!pic) return;

event.target.value = "";
setFile(null);

if (maxSize && pic.size > maxSize) {
const errorMsg = `File size exceeds ${formatBytes(maxSize)}`;
if (setErrors) setErrors?.((prev) => ({ ...prev, picture: errorMsg }));
return;
}

if (acceptedTypes && !acceptedTypes.includes(pic.type)) {
const errorMsg = `File type not supported. Allowed: ${acceptedTypes.join(", ")}`;
if (setErrors) setErrors?.((prev) => ({ ...prev, picture: errorMsg }));
return;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I like the early returns.

// Validate file type and size using schema
let error = validateField({ type: pic.type, size: pic.size }, imageValidation);
if (error) return;

setProgress({ value: 0, isLoading: true });
setFile({
src: URL.createObjectURL(pic),
name: pic.name,
size: formatBytes(pic.size),
delete: false,
});

// Simulate upload progress
intervalRef.current = setInterval(() => {
setProgress((prev) => {
const nextValue = prev.value + UPLOAD_PROGRESS_INCREMENT;
if (nextValue >= UPLOAD_COMPLETE) {
clearInterval(intervalRef.current);
return { value: UPLOAD_COMPLETE, isLoading: false };
}
return { ...prev, value: nextValue };
});
}, UPLOAD_PROGRESS_INTERVAL);
};

// Validates input against provided schema and updates error state
const validateField = (toValidate, schema, name = "picture") => {
const { error } = schema.validate(toValidate, { abortEarly: false });
if (setErrors) {
setErrors?.((prev) => {
const prevErrors = { ...prev };
if (error) {
prevErrors[name] = error.details[0].message;
} else {
delete prevErrors[name];
}
return prevErrors;
});
}
return !!error;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the possibilities for the error returned?

};

// Resets picture-related states and clears interval
const removePicture = () => {
errors["picture"] && setErrors((prev) => ({ ...prev, picture: undefined }));
setFile(null);
clearInterval(intervalRef.current);
setProgress({ value: 0, isLoading: false });
};

// Updates the profile picture and closes the modal
const handleUpdatePicture = () => {
if (file?.src) {
onUpdate(file.src);
}
setProgress({ value: 0, isLoading: false });
onClose(""); // Close the modal
};

return (
<GenericDialog
id="modal-update-picture"
open={open}
onClose={onClose}
title={t("uploadImage")}
description={t("selectImage")}
confirmationButtonLabel={t("update")}
onConfirm={handleUpdatePicture}
theme={theme}
isLoading={false}
>
<Box
className="image-field-wrapper"
sx={{
position: "relative",
display: "flex",
flexDirection: "column",
alignItems: "center",
justifyContent: "center",
minHeight: "180px",
maxHeight: "220px",
border: "dashed",
borderRadius: theme.shape.borderRadius,
borderColor: isDragging ? theme.palette.primary.main : theme.palette.primary.lowContrast,
borderWidth: "2px",
transition: "0.2s",
"&:hover": {
borderColor: theme.palette.primary.main,
backgroundColor: "hsl(215, 87%, 51%, 0.05)",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Colors, heights, and borderWidth/radius should not be hardcoded. Please touch base with @marcelluscaio on colors and widths.

},
}}
onDragEnter={(e) => handleDrag(e, true)}
onDragLeave={(e) => handleDrag(e, false)}
onDrop={(e) => handleDrag(e, false)}
onDragOver={(e) => e.preventDefault()}
>
<input
id="update-profile-picture"
type="file"
onChange={handlePicture}
style={{
position: "absolute",
top: 0,
left: 0,
width: "100%",
height: "100%",
opacity: 0,
cursor: "pointer",
zIndex: 3,
pointerEvents: "all",
}}
/>
Comment on lines +190 to +205
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use MUIs Input component?


{!checkImage(imageSrc) || progress.isLoading ? (
<>
<Stack
className="custom-file-text"
alignItems="center"
gap={theme.spacing(0.5)}
sx={{
position: "absolute",
top: "50%",
left: "50%",
transform: "translate(-50%, -50%)",
zIndex: 1,
width: "100%",
}}
>
<IconButton
sx={{
pointerEvents: "none",
borderRadius: theme.shape.borderRadius,
border: `solid ${theme.shape.borderThick}px ${theme.palette.primary.lowContrast}`,
boxShadow: theme.shape.boxShadow,
}}
>
<CloudUploadIcon />
</IconButton>
<Typography component="h2" color={theme.palette.primary.contrastTextTertiary}>
<Typography component="span" fontSize="inherit" color="info" fontWeight={500}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fontWeight should not be hardcoded, please use the theme. @marcelluscaio is the most knowledgeable on the theme so please reach out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should be using color info, will take a look into that

Click to upload
</Typography>{" "}
or drag and drop
</Typography>
<Typography
component="p"
color={theme.palette.primary.contrastTextTertiary}
sx={{ opacity: 0.6 }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tested if this passes contrast tests?

Copy link
Contributor

@gorkem-bwl gorkem-bwl Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can guide devs on how to do contrast tests - maybe a link or similar?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely!
Normally I use these two tools:

https://colourcontrast.cc/

https://webaim.org/resources/contrastchecker/

However the first one doesn't support opacity changes. So basically you put the hex values and change the alpha to 0.6, which is the opacity being used here

>
(Maximum size: {formatBytes(maxSize)})
</Typography>
</Stack>
</>
) : (
<Box
sx={{
width: `${previewSize}px`,
height: `${previewSize}px`,
overflow: "hidden",
backgroundImage: imageSrc ? `url(${imageSrc})` : "none",
backgroundSize: "cover",
backgroundPosition: "center",
borderRadius: "50%",
display: "flex",
alignItems: "center",
justifyContent: "center",
}}
/>
)}
</Box>

{progress.isLoading || progress.value !== 0 || errors?.picture ? (
<ProgressUpload
icon={<ImageIcon />}
label={file?.name}
size={file?.size || "0 KB"}
progress={progress.value}
onClick={removePicture}
error={errors?.picture}
/>
) : null}

<Stack
direction="row"
mt={theme.spacing(2)}
gap={theme.spacing(2)}
justifyContent="flex-end"
>
<Button
variant="text"
color="info"
onClick={removePicture}
>
Remove
</Button>
<Button
variant="contained"
color="accent"
onClick={handleUpdatePicture}
disabled={
(Object.keys(errors).length !== 0 && errors?.picture) ||
progress.value !== UPLOAD_COMPLETE
? true
: false
}
>
Update
</Button>
</Stack>
</GenericDialog>
);
};

export default ImageUpload;
18 changes: 0 additions & 18 deletions Client/src/Components/Inputs/Image/index.css

This file was deleted.

Loading