#3101: Add image upload limit of 20 to custom selector #5369
Conversation
… limit is changed
app/src/main/res/values/strings.xml
Outdated
| @@ -716,6 +716,8 @@ Upload your first media by tapping on the add button.</string> | |||
| <string name="custom_selector_info_text2">Unlike the picture on the left, the picture on the right has the Commons logo indicating it is already uploaded. \n Touch and hold for image preview.</string> | |||
| <string name="welcome_custom_selector_ok">Awesome</string> | |||
| <string name="custom_selector_already_uploaded_image_text">This image has already been uploaded to Commons.</string> | |||
| <string name="custom_selector_over_limit_warning">The upload limit of %1$d has been exceeded by %2$d. </string> | |||
There was a problem hiding this comment.
Here is a good place to explain why we impose a limit.
How about the message below?
For technical reasons, the app can't reliably upload more than %1$d pictures at once. The upload limit of %1$d has been exceeded by %2$d.
| @@ -67,6 +68,22 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL | |||
| */ | |||
| private lateinit var prefs: SharedPreferences | |||
|
|
|||
| /** | |||
| * Maximum number of images that can be selected. <br> | |||
|
I've made the changes; I reordered the message and split it into two toasts to make sure it gets the point across on smaller screens w/ the two-line limit. |
app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/CustomSelectorActivity.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Sorry I just found two small issues:
A) Red icon stays when pressing Back
- Go to any folder, select 21 pictures, the red icon appears
- Press Back
- You are now on the list of folders
- Problem: The red icon is still present.
B) Help message cut
You were right that the message is cut, actually even when splitting it into two toasts.
How about replacing the two toasts with a single popup? I believe we have a method to easily create popups.
Thanks a lot, and sorry for mentioning those only now!
|
I did notice A, but I left as is because, at least for me, the bottom bar with w/ the upload button also stays in place when you press the back button (w/ working functionality to upload/mark the images you had selected before) until you select any other image, and the error is relevant to the bottom bar. |
|
Great observation about the bottom bar! Then your choice makes sense. 🙂 |
… dialog code & layout
|
Added a pop-up, just adapted the code from the welcome dialog; I put a screenshot of it in the initial PR comment. |
|
Fixed that, cheers. |
* 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
Description (required)
Fixes #3101
What changes did you make and why?
Changed the custom image picker so that it only allows submissions of 20 or less images, interweaved with the pre-existing functionality to disable the upload button when not-for-upload images are selected, as per the reasoning in this comment. Added a text change to the button and a warning symbol to indicate the limit being breached. Kept the mark not for upload button active.
Tests performed (required)
Tested betaDebug on Pixel 3 with API level 31.
Wrote unit tests for one new function and for the logic operations, within the custom selector activity tests.
Screenshots (for UI changes only)
Warnings for the upload limit breach. Note that the limit was changed to 3 locally to make the demonstration easier, but the version in the PR has it set to 20. A dialog will also appear stating the upload limit, the amount it has been breached by and an explanation for it if the error icon is tapped.
Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.