Skip to content

Feature/Upload_media#74

Open
Smokashi23 wants to merge 6 commits intomasterfrom
Feature/Upload_media
Open

Feature/Upload_media#74
Smokashi23 wants to merge 6 commits intomasterfrom
Feature/Upload_media

Conversation

@Smokashi23
Copy link
Collaborator

-Added feature can upload pdf ,vedio, excel
-Added file size limit while uploading
-Changed the label from image to attachment

-Added file size limit while uploading
-Changed the label from image to attachment

export const ImageS3Tag = ({ path }) => {
const [srcImg, setSrcImg] = useState<string | undefined>();
export const MediaS3Tag = ({ path }: { path: string }) => {

Choose a reason for hiding this comment

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

I think the type definition affects readability in this case

  1. Define individual parameters with individual types
export const MediaS3Tag = (path: string) => {
  1. Define an interface for parameter object
interface MediaS3TagParams {
  path: string;
}

export const MediaS3Tag = ({path}: MediaS3TagParams) => {

const oversizedFile = selectedFiles.find((file) => file.size > MAX_FILE_SIZE);

if (oversizedFile) {
setErrorMessage(`File size exceeds limit: ${oversizedFile.name}`);

Choose a reason for hiding this comment

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

File exceeds size limit seems more clear?

return <CircularProgress />;
}

if (fileType === 'pdf') {

Choose a reason for hiding this comment

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

We should'nt be dependent on file extension to determine file type.

-Added the view and Download button
-Reframed the error message
Copy link

@Chetan-Satpute-Josh Chetan-Satpute-Josh left a comment

Choose a reason for hiding this comment

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

Please make changes discussed in the meeting.

-Used the usequery to fetch image
@Smokashi23 Smokashi23 mentioned this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants