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

57-dropzone-images #80

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

57-dropzone-images #80

wants to merge 3 commits into from

Conversation

Priyansh4444
Copy link
Member

@Priyansh4444 Priyansh4444 commented Jan 26, 2025

  • Adding a way for drag and Drop for images
  • Adding an X button to reomve images

Note: the frontend is tested while confirmation with backend hasnt been done though it should work since it doesnt change much but could be better
Would have liked to added:
1: Adding Optimistic Updates using XHR (a loading bar of sorts) (I have an example of doing this using https://powerpoint-scraper.deno.dev/)
2: Making the UI a bit bettter (especially considering I feel like this does a lot of rerenders the way i am doing it)
3: Wouldve liked to also use chakra 3.5's dropzone since it looks better but we are still on 2.6 and there is a lot of dependancy issues based on that :(

Hopefully will be able to do more this upcoming week

@Priyansh4444
Copy link
Member Author

https://www.chakra-ui.com/docs/components/file-upload

dropzone component in question

@Priyansh4444
Copy link
Member Author

am still a bit ill so will work on this whenever i feel better

Copy link
Contributor

@tentsundue tentsundue left a comment

Choose a reason for hiding this comment

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

LGTM! Dropzone worked as intended when I tested it.

@Priyansh4444
Copy link
Member Author

I know the pr is approved but can i get to make the xhr thing before next saturday on this pr, or should i wait till this pr is merged and just make a new one for then? 🤔 latter might be better thinking about it

Copy link
Contributor

@ElianMalessy ElianMalessy left a comment

Choose a reason for hiding this comment

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

Seems like it works correctly, but I'm just a little confused on why XHR was used here instead of fetch. I don't know if I'm being pedantic here, but we should probably stick to fetch in writing modern JS.

@Priyansh4444
Copy link
Member Author

Priyansh4444 commented Feb 7, 2025

Oh I was thinking this as the workflow, make an API route, have image uploading happen on this API route so that we can perform the optimizations and such on the image especially if it's a detailed and large one ( with new models of iPhones and higher screen res I am expecting them to be decently big) and it's a sort of optimistic update, which shows that your image is actually being uploaded with like a progress bar which is nice to see as UX, that was my thought process, but I see your point about how it could be pedantic as well, I think I'll leave this up for leads to decide!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants