Resolved Problems in UploadMediaDetails flow and UX commons-app#5511 (commons-app#5527)

* Fixed arrow flickering issue on zoom

* Resolved issue point 1 and 3

* Resolved issue point 2

* Minor changes

* Update UploadActivity.java

---------

Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>

Author: ShashwatKedia

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

@@ -48,6 +48,7 @@
 import fr.free.nrw.commons.contributions.MainActivity;
 import fr.free.nrw.commons.filepicker.Constants.RequestCodes;
 import fr.free.nrw.commons.filepicker.UploadableFile;
+import fr.free.nrw.commons.kvstore.BasicKvStore;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.location.LatLng;
 import fr.free.nrw.commons.location.LocationPermissionsHelper;
@@ -329,6 +330,8 @@ public void checkStoragePermissions() {
 
     @Override
     protected void onStop() {
+        // Resetting setImageCancelled to false
+        setImageCancelled(false);
         super.onStop();
     }
 
@@ -711,6 +714,34 @@ private boolean isLocationTagUncheckedInTheSettings() {
         return true;
     }
 
+    /**
+     * Changes current image when one image upload is cancelled, to highlight next image in the top thumbnail.
+     * Fixes: <a href="https://github.com/commons-app/apps-android-commons/issues/5511">Issue</a>
+     *
+     * @param index Index of image to be removed
+     * @param maxSize Max size of the {@code uploadableFiles}
+     */
+    @Override
+    public void highlightNextImageOnCancelledImage(int index, int maxSize) {
+        if (vpUpload != null && index < (maxSize)) {
+            vpUpload.setCurrentItem(index + 1, false);
+            vpUpload.setCurrentItem(index, false);
+        }
+    }
+
+    /**
+     * Used to check if user has cancelled upload of any image in current upload
+     * so that location compare doesn't show up again in same upload.
+     * Fixes: <a href="https://github.com/commons-app/apps-android-commons/issues/5511">Issue</a>
+     *
+     * @param isCancelled Is true when user has cancelled upload of any image in current upload
+     */
+    @Override
+    public void setImageCancelled(boolean isCancelled) {
+        BasicKvStore basicKvStore = new BasicKvStore(this,"IsAnyImageCancelled");
+        basicKvStore.putBoolean("IsAnyImageCancelled", isCancelled);
+    }
+
     /**
      * Calculate the difference between current location and
      * location recorded before capturing the image

app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java

@@ -20,6 +20,24 @@ public interface View {
 
         void askUserToLogIn();
 
+        /**
+         * Changes current image when one image upload is cancelled, to highlight next image in the top thumbnail.
+         * Fixes: <a href="https://github.com/commons-app/apps-android-commons/issues/5511">Issue</a>
+         *
+         * @param index Index of image to be removed
+         * @param maxSize Max size of the {@code uploadableFiles}
+         */
+        void highlightNextImageOnCancelledImage(int index, int maxSize);
+
+        /**
+         * Used to check if user has cancelled upload of any image in current upload
+         * so that location compare doesn't show up again in same upload.
+         * Fixes: <a href="https://github.com/commons-app/apps-android-commons/issues/5511">Issue</a>
+         *
+         * @param isCancelled Is true when user has cancelled upload of any image in current upload
+         */
+        void setImageCancelled(boolean isCancelled);
+
         void showProgress(boolean shouldShow);
 
         void showMessage(int messageResourceId);

app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java

@@ -141,6 +141,7 @@ public void deletePictureAtIndex(int index) {
         if (index == uploadableFiles.size() - 1) {//If the next fragment to be shown is not one of the MediaDetailsFragment, lets hide the top card
             view.showHideTopCard(false);
         }
+        view.setImageCancelled(true);
         repository.deletePicture(uploadableFiles.get(index).getFilePath());
         if (uploadableFiles.size() == 1) {
             view.showMessage(R.string.upload_cancelled);
@@ -155,6 +156,7 @@ public void deletePictureAtIndex(int index) {
 
         //In case lets update the number of uploadable media
         view.updateTopCardTitle();
+        view.highlightNextImageOnCancelledImage(index, uploadableFiles.size());
 
     }
 

app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java

@@ -36,6 +36,7 @@
 import fr.free.nrw.commons.edit.EditActivity;
 import fr.free.nrw.commons.contributions.MainActivity;
 import fr.free.nrw.commons.filepicker.UploadableFile;
+import fr.free.nrw.commons.kvstore.BasicKvStore;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.location.LatLng;
 import fr.free.nrw.commons.nearby.Place;
@@ -251,7 +252,10 @@ private void attachImageViewScaleChangeListener() {
         photoViewBackgroundImage.setOnScaleChangeListener(
             (scaleFactor, focusX, focusY) -> {
                 //Whenever the uses plays with the image, lets collapse the media detail container
-                expandCollapseLlMediaDetail(false);
+                //only if it is not already collapsed, which resolves flickering of arrow
+                if (isExpanded) {
+                    expandCollapseLlMediaDetail(false);
+                }
             });
     }
 
@@ -306,31 +310,35 @@ public void onEditButtonClicked() {
     @Override
     public void showSimilarImageFragment(String originalFilePath, String possibleFilePath,
         ImageCoordinates similarImageCoordinates) {
-        SimilarImageDialogFragment newFragment = new SimilarImageDialogFragment();
-        newFragment.setCallback(new SimilarImageDialogFragment.Callback() {
-            @Override
-            public void onPositiveResponse() {
-                Timber.d("positive response from similar image fragment");
-                presenter.useSimilarPictureCoordinates(similarImageCoordinates, callback.getIndexInViewFlipper(UploadMediaDetailFragment.this));
-
-                // set the description text when user selects to use coordinate from the other image
-                // which was taken within 20s
-                // fixing: https://github.com/commons-app/apps-android-commons/issues/4700
-                uploadMediaDetailAdapter.getItems().get(0).setDescriptionText(
-                    getString(R.string.similar_coordinate_description_auto_set));
-                updateMediaDetails(uploadMediaDetailAdapter.getItems());
-            }
+        BasicKvStore basicKvStore = new BasicKvStore(getActivity(), "IsAnyImageCancelled");
+        if (!basicKvStore.getBoolean("IsAnyImageCancelled", false)) {
+            SimilarImageDialogFragment newFragment = new SimilarImageDialogFragment();
+            newFragment.setCallback(new SimilarImageDialogFragment.Callback() {
+                @Override
+                public void onPositiveResponse() {
+                    Timber.d("positive response from similar image fragment");
+                    presenter.useSimilarPictureCoordinates(similarImageCoordinates,
+                        callback.getIndexInViewFlipper(UploadMediaDetailFragment.this));
+
+                    // set the description text when user selects to use coordinate from the other image
+                    // which was taken within 120s
+                    // fixing: https://github.com/commons-app/apps-android-commons/issues/4700
+                    uploadMediaDetailAdapter.getItems().get(0).setDescriptionText(
+                        getString(R.string.similar_coordinate_description_auto_set));
+                    updateMediaDetails(uploadMediaDetailAdapter.getItems());
+                }
 
-            @Override
-            public void onNegativeResponse() {
-                Timber.d("negative response from similar image fragment");
-            }
-        });
-        Bundle args = new Bundle();
-        args.putString("originalImagePath", originalFilePath);
-        args.putString("possibleImagePath", possibleFilePath);
-        newFragment.setArguments(args);
-        newFragment.show(getChildFragmentManager(), "dialog");
+                @Override
+                public void onNegativeResponse() {
+                    Timber.d("negative response from similar image fragment");
+                }
+            });
+            Bundle args = new Bundle();
+            args.putString("originalImagePath", originalFilePath);
+            args.putString("possibleImagePath", possibleFilePath);
+            newFragment.setArguments(args);
+            newFragment.show(getChildFragmentManager(), "dialog");
+        }
     }
 
     @Override
@@ -455,7 +463,8 @@ public void showDuplicatePicturePopup(UploadItem uploadItem) {
                 false);
         } else {
             uploadItem.setImageQuality(ImageUtils.IMAGE_KEEP);
-            onNextButtonClicked();
+            // Calling below, instead of onNextButtonClicked() to not show locationDialog twice
+            onImageValidationSuccess();
         }
     }
 
@@ -478,7 +487,11 @@ public void showBadImagePopup(Integer errorCode,
 
                     // validate image only when same file name error does not occur
                     // show the same file name error if exists.
-                    if ((errorCode & FILE_NAME_EXISTS) == 0) {
+                    // If image with same file name exists check the bit in errorCode is set or not
+                    if ((errorCode & FILE_NAME_EXISTS) != 0) {
+                        Timber.d("Trying to show duplicate picture popup");
+                        showDuplicatePicturePopup(uploadItem);
+                    } else {
                         uploadItem.setImageQuality(ImageUtils.IMAGE_KEEP);
                         onImageValidationSuccess();
                     }

app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java

@@ -345,17 +345,14 @@ public void handleBadImage(Integer errorCode,
             view.showMessage(R.string.add_caption_toast, R.color.color_error);
         }
 
-        // If image with same file name exists check the bit in errorCode is set or not
-        if ((errorCode & FILE_NAME_EXISTS) != 0) {
-            Timber.d("Trying to show duplicate picture popup");
-            view.showDuplicatePicturePopup(uploadItem);
-        }
-
-        // If image has some other problems, show popup accordingly
+        // If image has some problems, show popup accordingly
         if (errorCode != EMPTY_CAPTION && errorCode != FILE_NAME_EXISTS) {
             view.showBadImagePopup(errorCode, uploadItem);
+        } else if ((errorCode & FILE_NAME_EXISTS) != 0) {
+            // When image has duplicate caption problem
+            Timber.d("Trying to show duplicate picture popup");
+            view.showDuplicatePicturePopup(uploadItem);
         }
-
     }
 
     /**