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

Allow multiple images to be uploaded at once #777

Closed
sam365724 opened this issue Sep 15, 2022 · 9 comments · Fixed by #971
Closed

Allow multiple images to be uploaded at once #777

sam365724 opened this issue Sep 15, 2022 · 9 comments · Fixed by #971
Labels
enhancement New feature or request

Comments

@sam365724
Copy link
Contributor

sam365724 commented Sep 15, 2022

Is your proposal related to a problem?

Not directly. Uploading many pictures is a time-intense process with the current implementation. Each image has to be selected by hand.

Describe the solution you'd like

Allow the picture open dialog to select multiple images at once, and upload all of them afterwards, resulting in multiple links being inserted into the markdown body.
Nice to have:

  • Some better progress indication, showing x out of n uploaded...

Describe alternatives you've considered

Drag and drop, but I'd prefer the open dialog, it's more common.

Additional context

I'm sure there is an existing typescript component that could be used. How difficult is this to implement?

@sam365724 sam365724 added the enhancement New feature or request label Sep 15, 2022
@dessalines
Copy link
Member

We just use the browser native file uploader, which could support multiple images, but I wouldn't want to implement that as it'd potentially be a way to subvert the image rate limit.

@dessalines dessalines closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2022
@sam365724
Copy link
Contributor Author

Dear @dessalines, i was planning to implement this but only if you agree to merge it later. Or if needed make it configurable?

My plan is to implement it perfectly with an async parallel upload (e.g. 3 parallel uploads). We might need to adjust the rate limit i agree.

Please reconsider.

@dessalines
Copy link
Member

Yes I would merge this if you added it. Now that I think of it, the backend's rate limits would still be functioning, which is the main thing for me.

So far one of lemmy's main limitations is that picture uploads are quickly filling up hard drive space, and I don't like to do much to exacerbate that.

@dessalines dessalines reopened this Sep 25, 2022
@sam365724
Copy link
Contributor Author

Ok, cool.

Regarding disk usage: Honestly that is the problem of the owner or hoster of an instance. But we can always make this configurable if you think that's important.
Implementing LemmyNet/lemmy#2416 would help with disk space too actually and is important. I mean ppl could still upload tons of large images with the single uploader today.

I'll share some of my design ideas later if you want to review them. I guess i will modify the image-upload-form.tsx to be configurable for multi upload (as it's used for clear single-upload only as well for avatars and icons). Default is single-upload.

@sam365724
Copy link
Contributor Author

@dessalines It already works with a simple forEach loop. We need the following decision: Should we present a toast top right for each image or one at the end to delete all the uploaded ones? I would only present that at the end.

Also I would toast for each failed image and also try to output the users original file name, so he knows which one failed.

Also we could add two more checks:

  1. Max number of images --> Configurable
  2. Max file size per image, we could check that before uploading.

What do you think?

@dessalines
Copy link
Member

I would let the images insert themselves, and the delete toasts pop up individually as the images get uploaded. Seems like the least work.

The max # of images could just be a hardcoded value, something like 10 max seems good to me, and is also the default rate limit.

The max file size per image is going to be set by the server, ones over a certain amount will get rejected automatically.

@sam365724
Copy link
Contributor Author

Alright cool. It's almost ready. Can you help me understand this part in the else clause:

Not sure what the event is instead of a file upload?! How would i test this clause? I'll just try to call the code as before, but i was wondering.

@sam365724
Copy link
Contributor Author

@dessalines Would you take the pull request on the 0.16.7 version or in main only? I cannot get main to work at the moment, so i'm not sure.

LemmyNet/lemmy#2468

@dessalines
Copy link
Member

I'm not sure that the else is even necessary, but I'd have to test.

You can develop wherever you like, but please do your PR commits to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants