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

Limit on number of images uploaded at once #3101

Open
ilgazer opened this issue Jul 30, 2019 · 30 comments · Fixed by #5369
Open

Limit on number of images uploaded at once #3101

ilgazer opened this issue Jul 30, 2019 · 30 comments · Fixed by #5369

Comments

@ilgazer
Copy link
Collaborator

ilgazer commented Jul 30, 2019

Summary:

For multiple uploads, there should be an upper limit on the number of images a user can upload at a time. From #604 :

We need to enforce the limit on the number of images that can be uploaded at one go on our own.

One suggestion is to have this check in the 1st step of the upload flow itself. If the user has selected more than 5 images, he will need to remove the extra ones before proceeding.

The top bar can have a close icon to remove an image before proceeding. The user can simply remove any image before proceeding.

@harith96
Copy link

harith96 commented Aug 5, 2019

If this issue is not assigned to anyone, I would like to look into the matter

@nicolas-raoul
Copy link
Member

@harith96 Thanks! :-)

@harith96
Copy link

harith96 commented Aug 9, 2019

I've implemented the feature and am currently improving the code and adding javadocs. There are several small new functions as I've used existing functions for the most part. Should I write unit tests for the new ones?

@ilgazer
Copy link
Collaborator Author

ilgazer commented Aug 9, 2019 via email

@harith96

This comment has been minimized.

@ilgazer

This comment has been minimized.

@harith96

This comment has been minimized.

@harith96

This comment has been minimized.

@318anushka

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@arishta

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@ahsanz024

This comment has been minimized.

@misaochan

This comment has been minimized.

@arishta

This comment has been minimized.

@misaochan

This comment has been minimized.

@neslihanturan
Copy link
Collaborator

There is a sleeping PR almost ready for this issue: #3124 . If anyone interested this issue, please make sure you checked the code at mentioned PR.

@nicolas-raoul

This comment has been minimized.

@misaochan

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@misaochan

This comment has been minimized.

@Ayan-10

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@KeshavAnandBhagat

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@KeshavAnandBhagat

This comment has been minimized.

@nicolas-raoul

This comment has been minimized.

@u7469570
Copy link
Contributor

Hi, is this issue still relevant? If so, I'd be interested in pursuing it as a first foray into open source for a university project.

@nicolas-raoul
Copy link
Member

@u7469570 Yes still relevant, just replace 5 with 20 as upload reliability has improved. 🙂

nicolas-raoul pushed a commit that referenced this issue Nov 20, 2023
* Add basic image limit warning to custom selector

* Block upload button when image limit is exceeded

* Complete basic functionality for upload limit: Disabled button, warning sign & toast

* Consolidate functionality between upload limit and not for upload marking

* Upload Limit: write unit tests and optimize control flow

* Upload Limit Tests: improve logic coverage and alter to stay valid if limit is changed

* Upload Limit: polish javadocs and add explanation to warning toast.

* Upload Limit: refactor variable names

* Upload Limit: change warning toast to dialog box, repurposing welcome dialog code & layout

* Upload Limit: remove error icon when the mark as not for upload button is pressed
@nicolas-raoul nicolas-raoul reopened this Nov 20, 2023
@nicolas-raoul
Copy link
Member

nicolas-raoul commented Nov 20, 2023

Thanks @u7469570 for fixing the issue in the custom selector!

The next steps would be to do the same with the normal picker, and with the share intent. For these two, I guess the only thing we can do is to keep only the first 20 uploads, and show the same popup (implemented by @u7469570) explaining why we do this.

@u7469570 I unassigned you but of course if you want to also implement the above then you are more than welcome, I can assign you again if you ask. :-)

nicolas-raoul pushed a commit that referenced this issue Nov 21, 2023
* Add basic image limit warning to custom selector

* Block upload button when image limit is exceeded

* Complete basic functionality for upload limit: Disabled button, warning sign & toast

* Consolidate functionality between upload limit and not for upload marking

* Upload Limit: write unit tests and optimize control flow

* Upload Limit Tests: improve logic coverage and alter to stay valid if limit is changed

* Upload Limit: polish javadocs and add explanation to warning toast.

* Upload Limit: refactor variable names

* Upload Limit: change warning toast to dialog box, repurposing welcome dialog code & layout

* Upload Limit: remove error icon when the mark as not for upload button is pressed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.