Adds a 'Remove Location' button to the UploadWizard #5247 (#5672)

* Implemented basic flow to remove location

* Fixed and added new tests and enhanced UX
This commit is contained in:
Shashwat Kedia 2024-03-30 20:04:55 +05:30 committed by GitHub
parent 7dd00efa64
commit 0a6257b27b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 150 additions and 21 deletions

View file

@ -41,6 +41,7 @@ import fr.free.nrw.commons.location.LocationPermissionsHelper.Dialog;
import fr.free.nrw.commons.location.LocationPermissionsHelper.LocationPermissionCallback;
import fr.free.nrw.commons.location.LocationServiceManager;
import fr.free.nrw.commons.theme.BaseActivity;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.SystemThemeUtils;
import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.schedulers.Schedulers;
@ -95,6 +96,10 @@ public class LocationPickerActivity extends BaseActivity implements
* modifyLocationButton : button for start editing location
*/
Button modifyLocationButton;
/**
* removeLocationButton : button to remove location metadata
*/
Button removeLocationButton;
/**
* showInMapButton : button for showing in map
*/
@ -205,6 +210,7 @@ public class LocationPickerActivity extends BaseActivity implements
if ("UploadActivity".equals(activity)) {
placeSelectedButton.setVisibility(View.GONE);
modifyLocationButton.setVisibility(View.VISIBLE);
removeLocationButton.setVisibility(View.VISIBLE);
showInMapButton.setVisibility(View.VISIBLE);
largeToolbarText.setText(getResources().getString(R.string.image_location));
smallToolbarText.setText(getResources().
@ -257,6 +263,7 @@ public class LocationPickerActivity extends BaseActivity implements
markerImage = findViewById(R.id.location_picker_image_view_marker);
tvAttribution = findViewById(R.id.tv_attribution);
modifyLocationButton = findViewById(R.id.modify_location);
removeLocationButton = findViewById(R.id.remove_location);
showInMapButton = findViewById(R.id.show_in_map);
showInMapButton.setText(getResources().getString(R.string.show_in_map_app).toUpperCase());
shadow = findViewById(R.id.location_picker_image_view_shadow);
@ -275,6 +282,7 @@ public class LocationPickerActivity extends BaseActivity implements
private void setupMapView() {
adjustCameraBasedOnOptions();
modifyLocationButton.setOnClickListener(v -> onClickModifyLocation());
removeLocationButton.setOnClickListener(v -> onClickRemoveLocation());
showInMapButton.setOnClickListener(v -> showInMap());
darkThemeSetup();
requestLocationPermissions();
@ -286,6 +294,7 @@ public class LocationPickerActivity extends BaseActivity implements
private void onClickModifyLocation() {
placeSelectedButton.setVisibility(View.VISIBLE);
modifyLocationButton.setVisibility(View.GONE);
removeLocationButton.setVisibility(View.GONE);
showInMapButton.setVisibility(View.GONE);
markerImage.setVisibility(View.VISIBLE);
shadow.setVisibility(View.VISIBLE);
@ -301,6 +310,35 @@ public class LocationPickerActivity extends BaseActivity implements
}
}
/**
* Handles onclick event of removeLocationButton
*/
private void onClickRemoveLocation() {
DialogUtil.showAlertDialog(this,
getString(R.string.remove_location_warning_title),
getString(R.string.remove_location_warning_desc),
getString(R.string.continue_message),
getString(R.string.cancel), () -> removeLocationFromImage(), null);
}
/**
* Method to remove the location from the picture
*/
private void removeLocationFromImage() {
if (media != null) {
compositeDisposable.add(coordinateEditHelper.makeCoordinatesEdit(getApplicationContext()
, media, "0.0", "0.0", "0.0f")
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(s -> {
Timber.d("Coordinates are removed from the image");
}));
}
final Intent returningIntent = new Intent();
setResult(AppCompatActivity.RESULT_OK, returningIntent);
finish();
}
/**
* Show the location in map app
*/

View file

@ -20,6 +20,7 @@ import android.widget.ImageView;
import android.widget.Toast;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.exifinterface.media.ExifInterface;
import androidx.recyclerview.widget.LinearLayoutManager;
import fr.free.nrw.commons.CameraPosition;
import fr.free.nrw.commons.LocationPicker.LocationPicker;
@ -77,6 +78,11 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
public static final String UPLOAD_MEDIA_DETAILS = "upload_media_detail_adapter";
/**
* True when user removes location from the current image
*/
private boolean hasUserRemovedLocation;
private UploadMediaDetailAdapter uploadMediaDetailAdapter;
@ -295,7 +301,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
if (callback == null) {
return;
}
presenter.displayLocDialog(indexOfFragment, inAppPictureLocation);
presenter.displayLocDialog(indexOfFragment, inAppPictureLocation, hasUserRemovedLocation);
}
public void onPreviousButtonClicked() {
@ -634,14 +640,15 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
final double zoom = cameraPosition.getZoom();
editLocation(latitude, longitude, zoom);
/*
If isMissingLocationDialog is true, it means that the user has already tapped the
"Next" button, so go directly to the next step.
*/
// If isMissingLocationDialog is true, it means that the user has already tapped the
// "Next" button, so go directly to the next step.
if (isMissingLocationDialog) {
isMissingLocationDialog = false;
onNextButtonClicked();
}
} else {
// If camera position is null means location is removed by the user
removeLocation();
}
}
if (requestCode == REQUEST_CODE_FOR_EDIT_ACTIVITY && resultCode == RESULT_OK) {
@ -673,6 +680,46 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
}
}
/**
* Removes the location data from the image, by setting them to null
*/
public void removeLocation() {
editableUploadItem.getGpsCoords().setDecimalCoords(null);
try {
ExifInterface sourceExif = new ExifInterface(uploadableFile.getFilePath());
String[] exifTags = {
ExifInterface.TAG_GPS_LATITUDE,
ExifInterface.TAG_GPS_LATITUDE_REF,
ExifInterface.TAG_GPS_LONGITUDE,
ExifInterface.TAG_GPS_LONGITUDE_REF,
};
for (String tag : exifTags) {
sourceExif.setAttribute(tag, null);
}
sourceExif.saveAttributes();
Drawable mapQuestion = getResources().getDrawable(R.drawable.ic_map_not_available_20dp);
if (binding != null) {
binding.locationImageView.setImageDrawable(mapQuestion);
binding.locationTextView.setText(R.string.add_location);
}
editableUploadItem.getGpsCoords().setDecLatitude(0.0);
editableUploadItem.getGpsCoords().setDecLongitude(0.0);
editableUploadItem.getGpsCoords().setImageCoordsExists(false);
hasUserRemovedLocation = true;
Toast.makeText(getContext(), getString(R.string.location_removed), Toast.LENGTH_LONG)
.show();
} catch (Exception e) {
Timber.d(e);
Toast.makeText(getContext(), "Location could not be removed due to internal error",
Toast.LENGTH_LONG).show();
}
}
/**
* Update the old coordinates with new one
* @param latitude new latitude
@ -694,7 +741,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
binding.locationTextView.setText(R.string.edit_location);
}
Toast.makeText(getContext(), "Location Updated", Toast.LENGTH_LONG).show();
Toast.makeText(getContext(), getString(R.string.location_updated), Toast.LENGTH_LONG).show();
}

View file

@ -71,13 +71,16 @@ public interface UploadMediaDetailsContract {
boolean getImageQuality(int uploadItemIndex, LatLng inAppPictureLocation, Activity activity);
/**
* Checks if the image has a location or not. Displays a dialog alerting user that no
* location has been to added to the image and asking them to add one
* Checks if the image has a location. Displays a dialog alerting user that no location has
* been to added to the image and asking them to add one, if location was not removed by the
* user
*
* @param uploadItemIndex Index of the uploadItem which has no location
* @param inAppPictureLocation In app picture location (if any)
* @param hasUserRemovedLocation True if user has removed location from the image
*/
void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation);
void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation,
boolean hasUserRemovedLocation);
/**
* Used to check image quality from stored qualities and display dialogs

View file

@ -202,17 +202,21 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt
}
/**
* Checks if the image has a location or not. Displays a dialog alerting user that no
* location has been to added to the image and asking them to add one
* Checks if the image has a location. Displays a dialog alerting user that no
* location has been to added to the image and asking them to add one, if location was not
* removed by the user
*
* @param uploadItemIndex Index of the uploadItem which has no location
* @param inAppPictureLocation In app picture location (if any)
* @param hasUserRemovedLocation True if user has removed location from the image
*/
@Override
public void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation) {
public void displayLocDialog(int uploadItemIndex, LatLng inAppPictureLocation,
boolean hasUserRemovedLocation) {
final List<UploadItem> uploadItems = repository.getUploads();
UploadItem uploadItem = uploadItems.get(uploadItemIndex);
if (uploadItem.getGpsCoords().getDecimalCoords() == null && inAppPictureLocation == null) {
if (uploadItem.getGpsCoords().getDecimalCoords() == null && inAppPictureLocation == null
&& !hasUserRemovedLocation) {
final Runnable onSkipClicked = () -> {
verifyCaptionQuality(uploadItem);
};

View file

@ -54,6 +54,28 @@
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent">
<Button
android:id="@+id/remove_location"
style="@style/Widget.AppCompat.Button"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_margin="5dp"
android:backgroundTint="@color/deleteRed"
android:text="@string/remove_location"
android:textColor="@color/white"
android:visibility="gone"
app:layout_constraintBottom_toBottomOf="@id/map_bottom_layout"
app:layout_constraintEnd_toStartOf="@+id/guideline2"
app:layout_constraintStart_toStartOf="@id/map_bottom_layout"
app:layout_constraintTop_toTopOf="@id/map_bottom_layout" />
<androidx.constraintlayout.widget.Guideline
android:id="@+id/guideline2"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="vertical"
app:layout_constraintGuide_percent="0.35" />
<Button
android:id="@+id/modify_location"
android:layout_width="0dp"
@ -64,7 +86,7 @@
android:layout_margin="5dp"
app:layout_constraintBottom_toBottomOf="@id/map_bottom_layout"
app:layout_constraintEnd_toStartOf="@+id/guideline3"
app:layout_constraintStart_toStartOf="@id/map_bottom_layout"
app:layout_constraintStart_toEndOf="@+id/guideline2"
app:layout_constraintTop_toTopOf="@id/map_bottom_layout" />
<androidx.constraintlayout.widget.Guideline
@ -72,7 +94,7 @@
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="vertical"
app:layout_constraintGuide_percent="0.5" />
app:layout_constraintGuide_percent="0.7" />
<TextView
android:id="@+id/show_in_map"

View file

@ -790,6 +790,11 @@ Upload your first media by tapping on the add button.</string>
<string name="see_your_achievements">See your achievements</string>
<string name="edit_image">Edit Image</string>
<string name="edit_location">Edit Location</string>
<string name="location_updated">Location updated!</string>
<string name="remove_location">Remove Location</string>
<string name="remove_location_warning_title">Remove Location Warning</string>
<string name="remove_location_warning_desc">Location makes pictures more useful and findable. Do you really wish to remove location from this picture?</string>
<string name="location_removed">Location removed!</string>
<string name="send_thanks_to_author">Thank the author</string>
<string name="error_sending_thanks">Error sending thanks to author.</string>
<string name="invalid_login_message">Your log-in has expired. Please log in again.</string>

View file

@ -2,7 +2,6 @@ package fr.free.nrw.commons.locationpicker
import android.content.Context
import android.os.Looper
import android.util.Log
import android.view.View
import android.widget.Button
import android.widget.ImageView
@ -11,10 +10,8 @@ import androidx.appcompat.widget.AppCompatTextView
import com.google.android.material.floatingactionbutton.FloatingActionButton
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import fr.free.nrw.commons.CameraPosition
import fr.free.nrw.commons.LocationPicker.LocationPickerActivity
import fr.free.nrw.commons.Media
import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.kvstore.JsonKvStore
import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.LAST_LOCATION
@ -29,7 +26,6 @@ import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.*
import org.mockito.MockitoAnnotations
import org.osmdroid.api.IMapController
import org.osmdroid.util.GeoPoint
import org.powermock.reflect.Whitebox
import org.robolectric.Robolectric
@ -38,7 +34,6 @@ import org.robolectric.RuntimeEnvironment
import org.robolectric.Shadows
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import java.lang.reflect.InvocationTargetException
import java.lang.reflect.Method
@RunWith(RobolectricTestRunner::class)
@ -61,6 +56,9 @@ class LocationPickerActivityUnitTests {
@Mock
private lateinit var modifyLocationButton: Button
@Mock
private lateinit var removeLocationButton: Button
@Mock
private lateinit var placeSelectedButton: FloatingActionButton
@ -96,6 +94,7 @@ class LocationPickerActivityUnitTests {
Whitebox.setInternalState(activity, "applicationKvStore", applicationKvStore)
Whitebox.setInternalState(activity, "cameraPosition", cameraPosition)
Whitebox.setInternalState(activity, "modifyLocationButton", modifyLocationButton)
Whitebox.setInternalState(activity, "removeLocationButton", removeLocationButton)
Whitebox.setInternalState(activity, "placeSelectedButton", placeSelectedButton)
Whitebox.setInternalState(activity, "showInMapButton", showInMapButton)
Whitebox.setInternalState(activity, "markerImage", markerImage)
@ -134,6 +133,7 @@ class LocationPickerActivityUnitTests {
method.invoke(activity)
verify(placeSelectedButton, times(1)).visibility = View.VISIBLE
verify(modifyLocationButton, times(1)).visibility = View.GONE
verify(removeLocationButton, times(1)).visibility = View.GONE
verify(showInMapButton, times(1)).visibility = View.GONE
verify(markerImage, times(1)).visibility = View.VISIBLE
verify(shadow, times(1)).visibility = View.VISIBLE
@ -142,6 +142,16 @@ class LocationPickerActivityUnitTests {
verify(fabCenterOnLocation, times(1)).visibility = View.VISIBLE
}
@Test
@Throws(Exception::class)
fun testOnClickRemoveLocation() {
val method: Method = LocationPickerActivity::class.java.getDeclaredMethod(
"onClickRemoveLocation"
)
method.isAccessible = true
method.invoke(activity)
}
@Test
@Throws(Exception::class)
fun testPlaceSelected() {

View file

@ -382,7 +382,7 @@ class UploadMediaDetailFragmentUnitTest {
`when`(latLng.longitude).thenReturn(0.0)
`when`(uploadItem.gpsCoords).thenReturn(imageCoordinates)
fragment.onActivityResult(1211, Activity.RESULT_OK, intent)
Mockito.verify(presenter, Mockito.times(1)).displayLocDialog(0, null)
Mockito.verify(presenter, Mockito.times(1)).displayLocDialog(0, null, false)
}
@Test