apps-android-commons
apps-android-commons copied to clipboard
commons-app
Limit on number of images uploaded at once
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.
If this issue is not assigned to anyone, I would like to look into the matter
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" [email protected] 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 https://github.com/commons-app/apps-android-commons/issues/3101?email_source=notifications&email_token=AB2EXI2SIUAYODHBU26XH5DQDTWFBA5CNFSM4IH3FV6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35RLMQ#issuecomment-519771570, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2EXI24BHPZZUBF5YK7UO3QDTWFBANCNFSM4IH3FV6A .
Nice work! I'm kinda on holiday right now, but I'll take a look when I can. :)
Hey, @ilgazer did you have time to take a look at code in the pull request?
@nicolas-raoul Looks like no one is working on this issue rn. Can you assign this to me :-)
@318anushka Actually, I wonder why nobody reviewed Harith's pull request...
@harith96 Sorry for the delay, would you mind merging the current master into your pull request branch, and fixing the conflicts? If you don't have time now, @318anushka will take over in a week. Thanks! :-)
My team and I were looking for issues to resolve and this seems like a good starting point. If this issue is unassigned, can it be assigned to us?
@arishta It is yours! Please checkout @harith96's pull request, merge the current master into it, test it, then submit a pull request :-)
We are done with the designing part and currently in the middle of implementation.
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.
As the app has become more stable and usable, I have started uploading 10 or 20 of pictures at once. It is very convenient for a batch of pictures of the same castle, for instance. So I am starting to wonder whether we really need an artificial limit... Is there a specific reason we need a limit?
Coincidentally, two days ago I tried uploading 45 pictures at once (of a small island's landscapes) and the app became so slow that it froze for several minutes, until it crashed. I tried another time with no luck. Then I tried with only about 15 pictures and it worked.
@nicolas-raoul When we initially proposed multiple uploads, there were some concerns in the Commons community about the potential for inexperienced users to use this feature to flood Commons with nonsensical uploads, e.g. uploading their entire phone gallery. The limit was proposed as a compromise between the needs of legitimate users, and reassuring the community that they wouldn't get inundated with selfies.
Interesting! This could be a candidate for gamification (score 1: must upload 1 by 1, score 2: can upload up to 2 pictures at the same time, score 18: can upload up to 18 pictures at the same time, etc) in the spirit of https://github.com/commons-app/apps-android-commons/issues/2852
Sounds good to me, but we would need to fix the upstream Achievements issue first. :)
Hey @nicolas-raoul @misaochan , I was working on this topic past few days to see what can be done. So, As android doesn't have the functionality to directly count the number of images selected. I was thinking what if I make a custom image picker so that I can count the images and while selecting the images if user exceeds the limit of selecting images, user can not select any more image and gets a toast about that and also user can deselect and reselect the images at the same time. I will make an AlbumActivity where all albums of the device will be shown and By clicking an album all images will be displayed on the screen. By clicking the FAB I will send an intent to AlbumActivity with the maximum number of images that can be select. I will override onItemClick and manipulate the limitation of images that can be selected. After that, I will put all selected images in an ArrayList of UploadableFile and send an intent with putParcelableArrayListExtra and get the Arraylist in onPictureReturnedFromGallery method in FilePicker class. Hope I can able to clearly explain my solution to this issue. Also please give your thoughts on this.
@nicolas-raoul can I work on this?
@Ayan-10 Sounds like a good idea if Android does not have the functionality usable directly. It could clash with this year's GSoC though (many proposals are about creating such a custom image picker), so I suggest waiting until August 24 before implementing it this way.
@nicolas-raoul I am working on the issue and it will be completed very soon Can please add this issue under hacktoberfest label to get me motivated
@cakepineapple When you have implemented your currently assigned issue, please ask again. Thanks for your interest!
@KeshavAnandBhagat How is your progress? :-)
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. 🙂
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. :-)