Skip to content

Commit

Permalink
Resolved Problems in UploadMediaDetails flow and UX #5511 (#5527)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@ShashwatKedia @nicolas-raoul
ShashwatKedia and nicolas-raoul authored Feb 14, 2024
1 parent a308a1c commit b18117b
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 35 deletions.
31 changes: 31 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -329,6 +330,8 @@ public void checkStoragePermissions() {

@Override
protected void onStop() {
// Resetting setImageCancelled to false
setImageCancelled(false);
super.onStop();
}

Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadContract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());

}

Expand Down
67 changes: 40 additions & 27 deletions app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
});
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}

Expand All @@ -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();
}
Expand Down
13 changes: 5 additions & 8 deletions app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}

/**
Expand Down

0 comments on commit b18117b

Please sign in to comment.