Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resolved Problems in UploadMediaDetails flow and UX #5511 #5527

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Resolved Problems in UploadMediaDetails flow and UX #5511 #5527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9550a14
Fixed arrow flickering issue on zoom
ShashwatKedia Feb 4, 2024
e13caa7
Resolved issue point 1 and 3
ShashwatKedia Feb 5, 2024
d28f4fc
Resolved issue point 2
ShashwatKedia Feb 6, 2024
a27b020
Minor changes
ShashwatKedia Feb 8, 2024
b3e0503
Update UploadActivity.java
nicolas-raoul Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading