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

Store currently selected elements in undo checkpoint #2755

Merged
merged 1 commit into from
Jan 4, 2025
Merged

Store currently selected elements in undo checkpoint #2755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 33 additions & 0 deletions src/androidTest/java/de/blau/android/easyedit/NodeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,39 @@ public void unjoinMergeNodes() {
assertEquals(600181872L, node.getOsmId());
assertEquals(4, delegator.getCurrentStorage().getWays(node).size());
}

/**
* Select, unjoin, then undo, node should be selected again
*/
// @SdkSuppress(minSdkVersion = 26)
@Test
public void unjoinNodesUndo() {
TestUtils.zoomToLevel(device, main, 21);
TestUtils.unlock(device);
TestUtils.clickAtCoordinates(device, map, 8.3866386, 47.3904394, true);
TestUtils.clickAwayTip(device, main);
assertTrue(TestUtils.findText(device, false, context.getString(R.string.actionmode_nodeselect)));
Node node = App.getLogic().getSelectedNode();
assertNotNull(node);
assertEquals(600181872L, node.getOsmId());
final StorageDelegator delegator = App.getDelegator();
assertEquals(4, delegator.getCurrentStorage().getWays(node).size());

int apiNodeCount = delegator.getApiNodeCount();
assertTrue(TestUtils.clickMenuButton(device, context.getString(R.string.menu_unjoin), false, true));
assertEquals(apiNodeCount + 3, delegator.getApiNodeCount());

assertTrue(TestUtils.clickMenuButton(device, context.getString(R.string.undo), false, false));
TestUtils.clickAwayTip(device, context);

assertTrue(TestUtils.findText(device, false, context.getString(R.string.actionmode_nodeselect), 5000));
assertEquals(apiNodeCount, delegator.getApiNodeCount());

node = App.getLogic().getSelectedNode();
assertNotNull(node);
assertEquals(600181872L, node.getOsmId());
assertEquals(4, delegator.getCurrentStorage().getWays(node).size());
}

/**
* Select node that is member of a way, append to it
Expand Down
76 changes: 52 additions & 24 deletions src/main/java/de/blau/android/Logic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.app.AppCompatActivity;
import androidx.fragment.app.FragmentActivity;
import de.blau.android.Selection.Ids;
import de.blau.android.contract.HttpStatusCodes;
import de.blau.android.contract.Urls;
import de.blau.android.dialogs.AttachedObjectWarning;
Expand All @@ -64,6 +65,7 @@
import de.blau.android.dialogs.Progress;
import de.blau.android.dialogs.ProgressDialog;
import de.blau.android.dialogs.UploadConflict;
import de.blau.android.easyedit.EasyEditManager;
import de.blau.android.easyedit.ElementSelectionActionModeCallback;
import de.blau.android.exception.DataConflictException;
import de.blau.android.exception.IllegalOperationException;
Expand Down Expand Up @@ -106,6 +108,7 @@
import de.blau.android.osm.StorageDelegator;
import de.blau.android.osm.Tags;
import de.blau.android.osm.UndoStorage;
import de.blau.android.osm.UndoStorage.Checkpoint;
import de.blau.android.osm.UserDetails;
import de.blau.android.osm.ViewBox;
import de.blau.android.osm.Way;
Expand Down Expand Up @@ -470,41 +473,57 @@ public UndoStorage getUndo() {
}

/**
* Wrapper to ensure the dirty flag is set
* Undo the last checkpoint
*
* @return checkpoint name or null if none available
*/
@Nullable
public String undo() {
final StorageDelegator delegator = getDelegator();
String name = delegator.getUndo().undo();
checkClipboard(delegator);
delegator.dirty();
return name;
return undo(getDelegator(), getDelegator().getUndo().undo());
}

/**
* Check the clipboard for consistency post undo
* Undo a specific checkpoint
*
* @param checkpoint index of the checkpoint to undo
* @return checkpoint name or null if none available
*/
private void checkClipboard(@NonNull StorageDelegator delegator) {
if (!delegator.clipboardIsEmpty()) {
delegator.checkClipboard();
}
@Nullable
public String undo(int checkpoint) {
return undo(getDelegator(), getDelegator().getUndo().undo(checkpoint));
}

/**
* Wrapper to ensure the dirty flag is set
* Undo a checkpoint
*
* @param checkpoint index of the checkpoint to undo
* @param delegator the current StorageDelegator instance
* @param toUndo the Checkpoint to undo
* @return checkpoint name or null if none available
*/
@Nullable
public String undo(int checkpoint) {
final StorageDelegator delegator = getDelegator();
String name = delegator.getUndo().undo(checkpoint);
private String undo(@NonNull final StorageDelegator delegator, @Nullable Checkpoint toUndo) {
Selection.Ids ids = toUndo != null ? toUndo.getSelection() : null;
if (ids != null && map != null && map.getContext() instanceof Main) {
Main main = (Main) map.getContext();
final EasyEditManager easyEditManager = main.getEasyEditManager();
easyEditManager.finish();
final Selection currentSelection = selectionStack.getFirst();
currentSelection.reset();
currentSelection.fromIds(main, delegator, ids);
selectFromTop();
easyEditManager.editElements();
}
checkClipboard(delegator);
delegator.dirty();
return name;
return toUndo != null ? toUndo.getName() : null;
}

/**
* Check the clipboard for consistency post undo
*/
private void checkClipboard(@NonNull StorageDelegator delegator) {
if (!delegator.clipboardIsEmpty()) {
delegator.checkClipboard();
}
}

/**
Expand Down Expand Up @@ -656,7 +675,7 @@ public void createCheckpoint(@Nullable Activity activity, int stringId) {
Resources r = activity != null ? activity.getResources() : App.resources();
final UndoStorage undo = getDelegator().getUndo();
boolean firstCheckpoint = !undo.canUndo();
undo.createCheckpoint(r.getString(stringId));
undo.createCheckpoint(r.getString(stringId), getSelectedIds());
getDelegator().recordImagery(map);
if (firstCheckpoint && activity instanceof AppCompatActivity) {
((AppCompatActivity) activity).invalidateOptionsMenu();
Expand Down Expand Up @@ -2290,7 +2309,7 @@ public synchronized List<Result> performMerge(@Nullable final FragmentActivity a
createCheckpoint(activity, R.string.undo_action_merge_ways);
try {
displayAttachedObjectWarning(activity, mergeInto, mergeFrom, true); // needs to be done before merge
MergeAction action = new MergeAction(getDelegator(), mergeInto, mergeFrom);
MergeAction action = new MergeAction(getDelegator(), mergeInto, mergeFrom, getSelectedIds());
List<Result> result = action.mergeWays();
invalidateMap();
return result;
Expand Down Expand Up @@ -2327,7 +2346,7 @@ public synchronized List<Result> performMerge(@Nullable FragmentActivity activit
result.setElement(previousWay);
for (int i = 1; i < sortedWays.size(); i++) {
Way nextWay = (Way) sortedWays.get(i);
MergeAction action = new MergeAction(getDelegator(), previousWay, nextWay);
MergeAction action = new MergeAction(getDelegator(), previousWay, nextWay, getSelectedIds());
List<Result> tempResult = action.mergeWays();
final Result newMergeResult = tempResult.get(0);
if (!(newMergeResult.getElement() instanceof Way)) {
Expand Down Expand Up @@ -2364,7 +2383,7 @@ public synchronized List<Result> performPolygonMerge(@Nullable FragmentActivity
throw new OsmIllegalOperationException("No mergeable polygons");
}
try {
MergeAction action = new MergeAction(getDelegator(), ways.get(0), ways.get(1));
MergeAction action = new MergeAction(getDelegator(), ways.get(0), ways.get(1), getSelectedIds());
return action.mergeSimplePolygons(map);
} catch (OsmIllegalOperationException | StorageException ex) {
handleDelegatorException(activity, ex);
Expand Down Expand Up @@ -2534,7 +2553,7 @@ public synchronized List<Result> performMergeNodes(@Nullable FragmentActivity ac
throw new OsmIllegalOperationException("Trying to join node to itself");
}
displayAttachedObjectWarning(activity, element, nodeToJoin); // needs to be done before join
MergeAction action = new MergeAction(getDelegator(), element, nodeToJoin);
MergeAction action = new MergeAction(getDelegator(), element, nodeToJoin, getSelectedIds());
try {
List<Result> tempResult = action.mergeNodes();
if (overallResult.isEmpty()) {
Expand Down Expand Up @@ -2617,7 +2636,7 @@ public synchronized List<Result> performJoinNodeToWays(@Nullable FragmentActivit
} else {
displayAttachedObjectWarning(activity, node, nodeToJoin); // needs to be done before join
// merge node into target Node
MergeAction action = new MergeAction(getDelegator(), node, nodeToJoin);
MergeAction action = new MergeAction(getDelegator(), node, nodeToJoin, getSelectedIds());
tempResult = action.mergeNodes();
}
} catch (OsmIllegalOperationException | StorageException ex) {
Expand Down Expand Up @@ -4980,6 +4999,15 @@ public synchronized List<OsmElement> getSelectedElements() {
return result;
}

/**
* Get the ids of currently selected objects
*
* @return an Selection.Ids object containing the ids of currently selected objects
*/
public Ids getSelectedIds() {
return selectionStack.getFirst().getIds();
}

/**
* Check is all selected elements exist, return true if we actually had to remove something
*
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/de/blau/android/easyedit/PathCreationActionModeCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public boolean onCreateActionMode(ActionMode mode, Menu menu) {
} else {
mode.setSubtitle(R.string.actionmode_createpath);
}
logic.createCheckpoint(main, R.string.undo_action_append);
snap = logic.getPrefs().isWaySnapEnabled();
logic.setSelectedWay(null);
logic.setSelectedNode(appendTargetNode);
Expand Down Expand Up @@ -373,13 +374,13 @@ private synchronized void pathCreateNode(float x, float y) {
final boolean firstNode = addedNodes.isEmpty();
Node clicked = logic.getClickedNode(x, y);
if (appendTargetNode != null) {
logic.performAppendAppend(main, x, y, firstNode, snap);
logic.performAppendAppend(main, x, y, false, snap);
appendTargetNode = logic.getSelectedNode();
if (firstNode) {
checkpointName = R.string.undo_action_append;
}
} else {
logic.performAdd(main, x, y, firstNode, snap);
logic.performAdd(main, x, y, false, snap);
if (firstNode) {
checkpointName = R.string.undo_action_add;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/blau/android/easyedit/ReplaceGeometryActionModeCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public boolean handleElementClick(OsmElement element) { // NOSONAR
Node toReplace = findYoungestUntaggedNode((Way) element);
logic.setTags(main, Node.NAME, target.getOsmId(), null, false);
logic.performSetPosition(main, (Node) target, toReplace.getLon(), toReplace.getLat(), false);
MergeAction merge = new MergeAction(delegator, target, toReplace, false);
MergeAction merge = new MergeAction(delegator, target, toReplace, false, logic.getSelectedIds());
List<Result> mergeResult = merge.mergeNodes();
java.util.Map<String, String> mergedTags = MergeAction.mergeTags(element, targetTags);
Result r = mergeResult.get(0);
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/de/blau/android/easyedit/WaySelectPartActionModeCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ public boolean handleElementClick(OsmElement element) {
try {
List<Result> result = logic.performSplit(main, way, node, fromEnd);
checkSplitResult(way, result);
manager.finish();
logic.setSelectedWay((Way) result.get(0).getElement());
manager.editElements();
} catch (OsmIllegalOperationException | StorageException ex) {
// toast has already been displayed
} finally {
manager.finish();
}
});
Expand All @@ -96,7 +98,7 @@ public void saveState(SerializableState state) {
state.putLong(WAY_ID_KEY, way.getOsmId());
state.putLong(NODE_ID_KEY, node.getOsmId());
}

@Override
public void onDestroyActionMode(ActionMode mode) {
logic.setClickableElements(null);
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/de/blau/android/easyedit/WaySelectionActionModeCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,11 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
Way way = (Way) element;
switch (item.getItemId()) {
case MENUITEM_SPLIT:
deselect = false;
main.startSupportActionMode(new WaySplittingActionModeCallback(manager, way, false));
break;
case MENUITEM_MERGE:
deselect = false;
main.startSupportActionMode(new WayMergingActionModeCallback(manager, way, cachedMergeableWays));
break;
case MENUITEM_REVERSE:
Expand All @@ -247,9 +249,11 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
findConnectedWays(way);
break;
case MENUITEM_APPEND:
deselect = false;
main.startSupportActionMode(new WayAppendingActionModeCallback(manager, way, cachedAppendableNodes));
break;
case MENUITEM_RESTRICTION:
deselect = false;
main.startSupportActionMode(new FromElementActionModeCallback(manager, way, cachedViaElements));
break;
case MENUITEM_ROUTE:
Expand Down Expand Up @@ -282,6 +286,7 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
main.performTagEdit(element, null, true, false);
break;
case MENUITEM_REMOVE_NODE:
deselect = false;
main.startSupportActionMode(new RemoveNodeFromWayActionModeCallback(manager, way));
break;
case MENUITEM_UNJOIN:
Expand All @@ -292,6 +297,7 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
Util.sharePosition(main, Geometry.centroidLonLat(way), main.getMap().getZoomLevel());
break;
case MENUITEM_EXTRACT_SEGMENT:
deselect = false;
main.startSupportActionMode(new WaySegmentActionModeCallback(manager, way));
break;
case MENUITEM_SELECT_WAY_NODES:
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/de/blau/android/easyedit/WaySplittingActionModeCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ public boolean onCreateActionMode(ActionMode mode, Menu menu) {
}

@Override
public boolean handleElementClick(OsmElement element) { // due to clickableElements, only valid nodes can be
// clicked
public boolean handleElementClick(OsmElement element) { // due to clickableElements, only valid nodes can be clicked
super.handleElementClick(element);
// protect against race conditions
if (!(element instanceof Node)) {
Expand All @@ -102,9 +101,11 @@ public boolean handleElementClick(OsmElement element) { // due to clickableEleme
try {
List<Result> result = logic.performSplit(main, way, (Node) element, true);
checkSplitResult(way, result);
manager.finish();
logic.setSelectedWay((Way) result.get(0).getElement());
manager.editElements();
} catch (OsmIllegalOperationException | StorageException ex) {
// toast has already been displayed
} finally {
manager.finish();
}
});
Expand Down
16 changes: 12 additions & 4 deletions src/main/java/de/blau/android/osm/MergeAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import de.blau.android.R;
import de.blau.android.Selection;
import de.blau.android.Selection.Ids;
import de.blau.android.exception.OsmIllegalOperationException;
import de.blau.android.util.ACRAHelper;
import de.blau.android.util.Coordinates;
Expand All @@ -40,6 +42,8 @@ public class MergeAction {
private final OsmElement mergeFrom;
private final List<Result> overallResult;

private final Selection.Ids selection;

/**
* Initialize a new MergeAction
*
Expand All @@ -48,9 +52,10 @@ public class MergeAction {
* @param delegator the StorageDelegator to use
* @param mergeInto the OsmElement that we are going to merge into
* @param mergeFrom the OsmElement that is going to be removed
* @param selection the current selection in the UI
*/
public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElement mergeInto, @NonNull OsmElement mergeFrom) {
this(delegator, mergeInto, mergeFrom, true);
public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElement mergeInto, @NonNull OsmElement mergeFrom, Ids selection) {
this(delegator, mergeInto, mergeFrom, true, selection);
}

/**
Expand All @@ -62,8 +67,10 @@ public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElemen
* @param mergeInto the OsmElement that we are going to merge into
* @param mergeFrom the OsmElement that is going to be removed
* @param swappable if true the elements can be swapped to maintain history
* @param selection the current selection in the UI
*/
public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElement mergeInto, @NonNull OsmElement mergeFrom, boolean swappable) {
public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElement mergeInto, @NonNull OsmElement mergeFrom, boolean swappable,
Ids selection) {
this.delegator = delegator;
// first determine if one of the elements already has a valid id, if it is not and other node has valid id swap
// else check version numbers, then choose the older element. The point of this is to preserve as much history
Expand All @@ -79,6 +86,7 @@ public MergeAction(final @NonNull StorageDelegator delegator, @NonNull OsmElemen
}
this.mergeInto = mergeInto;
this.mergeFrom = mergeFrom;
this.selection = selection;
overallResult = roleConflict(mergeInto, mergeFrom);
}

Expand Down Expand Up @@ -540,7 +548,7 @@ public List<Result> mergeSimplePolygons(@NonNull de.blau.android.Map map) throws
delegator.setTags(result, tags);
try {
delegator.lock();
delegator.getUndo().createCheckpoint(map.getContext().getString(R.string.undo_action_move_tags));
delegator.getUndo().createCheckpoint(map.getContext().getString(R.string.undo_action_move_tags), selection);
delegator.recordImagery(map);
} finally {
delegator.unlock();
Expand Down
Loading
Loading