-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Limit on number of images uploaded at once #3101
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
Comments
If this issue is not assigned to anyone, I would like to look into the matter |
@harith96 Thanks! :-) |
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? |
That's great to hear :) You indeed need to write tests for your code. Keep
in mind that the focus is to test the feature, it's up to you to find the
best way to do that.
…On Aug 9, 2019 7:21 AM, "harith96" ***@***.***> wrote:
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?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3101?email_source=notifications&email_token=AB2EXI2SIUAYODHBU26XH5DQDTWFBA5CNFSM4IH3FV6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35RLMQ#issuecomment-519771570>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB2EXI24BHPZZUBF5YK7UO3QDTWFBANCNFSM4IH3FV6A>
.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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. |
@u7469570 Yes still relevant, just replace 5 with 20 as upload reliability has improved. 🙂 |
* 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
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. :-) |
* 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
Is this issue still Relevant? |
Actually not as relevant as before, as the app has become much better at uploading large numbers of uploads. |
Summary:
For multiple uploads, there should be an upper limit on the number of images a user can upload at a time. From #604 :
The text was updated successfully, but these errors were encountered: