Fix/upload limit #3101

app/src/main/java/fr/free/nrw/commons/customselector/ui/adapter/ImageAdapter.kt

@@ -11,6 +11,7 @@ import androidx.constraintlayout.widget.Group
 import androidx.recyclerview.widget.DiffUtil
 import androidx.recyclerview.widget.RecyclerView
 import com.bumptech.glide.Glide
+import com.bumptech.glide.load.engine.DiskCacheStrategy
 import com.simplecityapps.recyclerview_fastscroll.views.FastScrollRecyclerView
 import fr.free.nrw.commons.R
 import fr.free.nrw.commons.contributions.Contribution
@@ -202,105 +203,44 @@ class ImageAdapter(
                 defaultDispatcher,
                 uploadingContributionList,
             )
-            scope.launch {
-                val sharedPreferences: SharedPreferences =
-                    context.getSharedPreferences(CUSTOM_SELECTOR_PREFERENCE_KEY, 0)
-                val showAlreadyActionedImages =
-                    sharedPreferences.getBoolean(SHOW_ALREADY_ACTIONED_IMAGES_PREFERENCE_KEY, true)
-                if (!showAlreadyActionedImages) {
-                    // If the position is not already visited, that means the position is new then
-                    // finds the next actionable image position from all images
-                    if (!alreadyAddedPositions.contains(position)) {
-                        processThumbnailForActionedImage(
-                            holder,
-                            position,
-                            uploadingContributionList
-                        )
-                        _isLoadingImages.value = false
-                        // If the position is already visited, that means the image is already present
-                        // inside map, so it will fetch the image from the map and load in the holder
-                    } else {
-                        val actionableImages: List<Image> = ArrayList(actionableImagesMap.values)
-                        if (actionableImages.size > position) {
-                            image = actionableImages[position]
-                            Glide
-                                .with(holder.image)
-                                .load(image.uri)
-                                .thumbnail(0.3f)
-                                .into(holder.image)
-                        }
-                    }
-
-                    // If switch is turned off, it just fetches the image from all images without any
-                    // further operations
-                } else {
-                    Glide
-                        .with(holder.image)
-                        .load(image.uri)
-                        .thumbnail(0.3f)
-                        .into(holder.image)
-                }
-            }
-
             holder.itemView.setOnClickListener {
-                onThumbnailClicked(position, holder)
+                //we just prevent auto-selection, but the user can still tap to select/unmark
+                if (!holder.isItemUploaded()) {
+                    onThumbnailClicked(position, holder)
+                }
             }
-
-            // launch media preview on long click.
             holder.itemView.setOnLongClickListener {
-                imageSelectListener.onLongPress(images.indexOf(image), images, selectedImages)
+                imageSelectListener.onLongPress(position, images, ArrayList(selectedImages))
                 true
             }
-        }
-    }
-
-    /**
-     * Process thumbnail for actioned image
-     */
-    suspend fun processThumbnailForActionedImage(
-        holder: ImageViewHolder,
-        position: Int,
-        uploadingContributionList: List<Contribution>,
-    ) {
-        _isLoadingImages.value = true
-        val next =
-            imageLoader.nextActionableImage(
-                allImages,
-                ioDispatcher,
-                defaultDispatcher,
-                nextImagePosition,
-                uploadingContributionList,
-            )
-
-        // If next actionable image is found, saves it, as the the search for
-        // finding next actionable image will start from this position
-        if (next > -1) {
-            nextImagePosition = next + 1
+            //handle close button click for deselection
+            holder.closeButton.setOnClickListener {
+                if (isSelected) {
+                    selectedImages.removeAt(selectedIndex)
+                    holder.itemUnselected()
+                    notifyItemChanged(position, ImageUnselected())
+                    imageSelectListener.onSelectedImagesChanged(selectedImages, selectedImages.size)
+                }
+            }
 
-            // If map doesn't contains the next actionable image, that means it's a
-            // new actionable image, it will put it to the map as actionable images
-            // and it will load the new image in the view holder
-            if (!actionableImagesMap.containsKey(next)) {
-                actionableImagesMap[next] = allImages[next]
-                alreadyAddedPositions.add(imagePositionAsPerIncreasingOrder)
-                imagePositionAsPerIncreasingOrder++
-                _currentImagesCount.value = imagePositionAsPerIncreasingOrder
-                Glide
-                    .with(holder.image)
-                    .load(allImages[next].uri)
-                    .thumbnail(0.3f)
-                    .into(holder.image)
-                notifyItemInserted(position)
-                notifyItemRangeChanged(position, itemCount + 1)
+            //lazy loading for the actionable images
+            if (!showAlreadyActionedImages && position == actionableImagesMap.size && !reachedEndOfFolder) {
+                scope.launch {
+                    processThumbnailForActionedImage(
+                        holder,
+                        position,
+                        uploadingContributionList
+                    )
+                }
             }
 
-            // If next actionable image is not found, that means searching is
-            // complete till end, and it will stop searching.
-        } else {
-            reachedEndOfFolder = true
-            notifyItemRemoved(position)
+            //fallback glide load if query fails
+            Glide.with(context)
+                .load(image.uri)
+                .diskCacheStrategy(DiskCacheStrategy.ALL)
+                .thumbnail(0.3f)
+                .into(holder.image)
         }
-        _isLoadingImages.value = false
     }
 
     /**
@@ -338,78 +278,107 @@ class ImageAdapter(
         val showAlreadyActionedImages =
             sharedPreferences.getBoolean(SHOW_ALREADY_ACTIONED_IMAGES_PREFERENCE_KEY, true)
 
-        // Getting clicked index from all images index when show_already_actioned_images
-        // switch is on
-        if (singleSelection) {
-            // If single selection mode, clear previous selection and select only the new one
-            if (selectedImages.isNotEmpty() && (selectedImages[0] != images[position])) {
-                val prevIndex = images.indexOf(selectedImages[0])
-                selectedImages.clear()
-                notifyItemChanged(prevIndex, ImageUnselected())
-            }
+        //determines which image was clicked
+        val clickedImage = if (showAlreadyActionedImages) {
+            images[position]
+        } else if (actionableImagesMap.size > position) {
+            ArrayList(actionableImagesMap.values)[position]
+        } else {
+            return //saftey
         }
-        val clickedIndex: Int =
-            if (showAlreadyActionedImages) {
-                ImageHelper.getIndex(selectedImages, images[position])
-            } else {
-                ImageHelper.getIndex(selectedImages, ArrayList(actionableImagesMap.values)[position])
-            }
 
-        if (clickedIndex != -1) {
-            selectedImages.removeAt(clickedIndex)
-            if (holder.isItemNotForUpload()) {
-                numberOfSelectedImagesMarkedAsNotForUpload--
-            }
+        if (singleSelection && selectedImages.isNotEmpty() && selectedImages[0] != clickedImage) {
+            val prevIndex = images.indexOf(selectedImages[0])
+            selectedImages.clear()
+            numberOfSelectedImagesMarkedAsNotForUpload = 0
+            if (prevIndex != -1) notifyItemChanged(prevIndex, ImageUnselected())
+        }
+
+        //checks if already selected -> deselect
+        val alreadySelectedIndex = selectedImages.indexOf(clickedImage)
+        if (alreadySelectedIndex != -1) {
+            selectedImages.removeAt(alreadySelectedIndex)
+            if (holder.isItemNotForUpload()) numberOfSelectedImagesMarkedAsNotForUpload--
+            holder.itemUnselected()
             notifyItemChanged(position, ImageUnselected())
-            // Notify listener of deselection to update UI
             imageSelectListener.onSelectedImagesChanged(selectedImages, numberOfSelectedImagesMarkedAsNotForUpload)
-        } else {
-            //check the maximum limit before allowing the selection
-            if (!singleSelection && selectedImages.size >= maxUploadLimit) {
-                // limit reached, show a toast and prevent selection
-                Toast.makeText(
-                    context,
-                    context.getString(
-                        R.string.custom_selector_max_image_limit_reached,
-                        maxUploadLimit
-                    ),
-                    Toast.LENGTH_SHORT
-                ).show()
-                return //exit the function, preventing selection
-            }
+            return
+        }
 
-            // Prevent adding the same image multiple times
-            val image = if (showAlreadyActionedImages) images[position] else ArrayList(actionableImagesMap.values)[position]
-            if (selectedImages.contains(image)) {
-                return // Image already selected, ignore additional clicks
-            }
-            scope.launch(ioDispatcher) {
-                val imageSHA1 = imageLoader.getSHA1(image, defaultDispatcher)
-                withContext(Dispatchers.Main) {
-                    if (holder.isItemUploaded()) {
-                        Toast.makeText(context, R.string.custom_selector_already_uploaded_image_text, Toast.LENGTH_SHORT).show()
-                        return@withContext
-                    }
-
-                    if (imageSHA1.isNotEmpty() && imageLoader.getFromUploaded(imageSHA1) != null) {
-                        holder.itemUploaded()
-                        Toast.makeText(context, R.string.custom_selector_already_uploaded_image_text, Toast.LENGTH_SHORT).show()
-                        return@withContext
-                    }
-
-                    if (!holder.isItemUploaded() && imageSHA1.isNotEmpty() && imageLoader.getFromUploaded(imageSHA1) != null) {
-                        Toast.makeText(context, R.string.custom_selector_already_uploaded_image_text, Toast.LENGTH_SHORT).show()
-                    }
-
-                    if (holder.isItemNotForUpload()) {
-                        numberOfSelectedImagesMarkedAsNotForUpload++
-                    }
-                    selectedImages.add(image)
-                    notifyItemChanged(position, ImageSelectedOrUpdated())
-                    imageSelectListener.onSelectedImagesChanged(selectedImages, numberOfSelectedImagesMarkedAsNotForUpload)
+        //block selection if limit reached (and shows the toast)
+        if (!singleSelection && selectedImages.size >= maxUploadLimit) {
+            Toast.makeText(
+                context,
+                context.getString(R.string.custom_selector_max_image_limit_reached, maxUploadLimit),
+                Toast.LENGTH_LONG
+            ).show()
+            return
+        }
+
+        //proceeds with the selection
+        scope.launch(ioDispatcher) {
+            val imageSHA1 = imageLoader.getSHA1(clickedImage, defaultDispatcher)
+
+            withContext(Dispatchers.Main) {
+                //checks if already uploaded
+                if (imageSHA1.isNotEmpty() && imageLoader.getFromUploaded(imageSHA1) != null) {
+                    holder.itemUploaded()
+                    Toast.makeText(context, R.string.custom_selector_already_uploaded_image_text, Toast.LENGTH_LONG).show()
+                    return@withContext
                 }
+
+                //finalises the selection
+                if (holder.isItemNotForUpload()) {
+                    numberOfSelectedImagesMarkedAsNotForUpload++
+                }
+                selectedImages.add(clickedImage)
+                holder.itemSelected()
+                notifyItemChanged(position, ImageSelectedOrUpdated())
+                imageSelectListener.onSelectedImagesChanged(selectedImages, numberOfSelectedImagesMarkedAsNotForUpload)
+            }
+        }
+    }
+
+    /**
+     * Process thumbnail for actioned image
+     */
+    suspend fun processThumbnailForActionedImage(
+        holder: ImageViewHolder,
+        position: Int,
+        uploadingContributionList: List<Contribution>,
+    ) {
+        _isLoadingImages.value = true
+        val next =
+            imageLoader.nextActionableImage(
+                allImages,
+                ioDispatcher,
+                defaultDispatcher,
+                nextImagePosition,
+                uploadingContributionList,
+            )
+
+        //if next actionable image is found, saves it, as the the search for
+        //finding next actionable image will start from this position
+        if (next > -1) {
+            nextImagePosition = next + 1
+            if (!actionableImagesMap.containsKey(next)) {
+                actionableImagesMap[next] = allImages[next]
+                alreadyAddedPositions.add(imagePositionAsPerIncreasingOrder)
+                imagePositionAsPerIncreasingOrder++
+                _currentImagesCount.value = imagePositionAsPerIncreasingOrder
+                Glide
+                    .with(holder.image)
+                    .load(allImages[next].uri)
+                    .thumbnail(0.3f)
+                    .into(holder.image)
+                notifyItemInserted(position)
+                notifyItemRangeChanged(position, itemCount + 1)
             }
+        } else {
+            reachedEndOfFolder = true
+            notifyItemRemoved(position)
         }
+        _isLoadingImages.value = false
     }
 
     /**
@@ -459,7 +428,7 @@ class ImageAdapter(
     ) {
         numberOfSelectedImagesMarkedAsNotForUpload = 0
         images.clear()
-        selectedImages = arrayListOf()
+        selectedImages = ArrayList(selectedImages)
         init(newImages, fixedImages, TreeMap(), uploadingImages)
         notifyDataSetChanged()
     }
@@ -551,19 +520,22 @@ class ImageAdapter(
         private val uploadingGroup: Group = itemView.findViewById(R.id.uploading_group)
         private val notForUploadGroup: Group = itemView.findViewById(R.id.not_for_upload_group)
         private val selectedGroup: Group = itemView.findViewById(R.id.selected_group)
+        val closeButton: ImageView = itemView.findViewById(R.id.close_button) //added for close button
 
         /**
          * Item selected view.
          */
         fun itemSelected() {
             selectedGroup.visibility = View.VISIBLE
+            closeButton.visibility = View.GONE
         }
 
         /**
          * Item Unselected view.
          */
         fun itemUnselected() {
             selectedGroup.visibility = View.GONE
+            closeButton.visibility = View.GONE
         }
 
         /**

app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/CustomSelectorActivity.kt

@@ -522,7 +522,7 @@ class CustomSelectorActivity :
         val folder = File(folderPath)
 
         supportFragmentManager.popBackStack(null,
-                                androidx.fragment.app.FragmentManager.POP_BACK_STACK_INCLUSIVE)
+            androidx.fragment.app.FragmentManager.POP_BACK_STACK_INCLUSIVE)
 
         //refresh MediaStore for the deleted folder path to ensure metadata updates
         FolderDeletionHelper.refreshMediaStore(this, folder)
@@ -595,7 +595,7 @@ class CustomSelectorActivity :
             bottomSheetBinding.upload.text = resources.getString(R.string.upload)
         }
 
-        if (uploadLimitExceeded || selectedNotForUploadImages > 0) {
+        if (selectedNotForUploadImages > 0) {
             bottomSheetBinding.upload.isEnabled = false
             bottomSheetBinding.upload.alpha = 0.5f
         } else {
@@ -652,7 +652,7 @@ class CustomSelectorActivity :
             return
         }
         scope.launch(ioDispatcher) {
-            val uniqueImages = selectedImages.distinctBy { image ->
+            val uniqueImages = selectedImages.take(uploadLimit).distinctBy { image ->
                 CustomSelectorUtils.getImageSHA1(
                     image.uri,
                     ioDispatcher,