From b18117bc0789697453c833419ea309e7ac0ce9d1 Mon Sep 17 00:00:00 2001 From: Shashwat Kedia <142137555+ShashwatKedia@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:37:51 +0530 Subject: [PATCH] Resolved Problems in UploadMediaDetails flow and UX #5511 (#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 --- .../nrw/commons/upload/UploadActivity.java | 31 +++++++++ .../nrw/commons/upload/UploadContract.java | 18 +++++ .../nrw/commons/upload/UploadPresenter.java | 2 + .../UploadMediaDetailFragment.java | 67 +++++++++++-------- .../mediaDetails/UploadMediaPresenter.java | 13 ++-- 5 files changed, 96 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java index f7798e1657..3b3b03992d 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java +++ b/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: Issue + * + * @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: Issue + * + * @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 diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java index 2ce133a7cd..b498563aa6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java +++ b/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: Issue + * + * @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: Issue + * + * @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); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java index 1f8deaa7c0..bffff389f3 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java +++ b/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()); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java index 1413816b6e..85ce3b78bc 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java +++ b/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(); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java index 40278b7eff..05390544cf 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java +++ b/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); } - } /**