Allow Drag and Drop to form new selection with CTRL held down.
Bug: 31992361
Change-Id: I90036b1b9b3573de289a7bce2d2fb07f69ef5651
diff --git a/src/com/android/documentsui/dirlist/DragStartListener.java b/src/com/android/documentsui/dirlist/DragStartListener.java
index 2bca5b2..5bf41d7 100644
--- a/src/com/android/documentsui/dirlist/DragStartListener.java
+++ b/src/com/android/documentsui/dirlist/DragStartListener.java
@@ -97,18 +97,18 @@
@Override
public final boolean onMouseDragEvent(InputEvent event) {
assert(Events.isMouseDragEvent(event));
- return startDrag(mViewFinder.findView(event.getX(), event.getY()));
+ return startDrag(mViewFinder.findView(event.getX(), event.getY()), event);
}
@Override
public final boolean onTouchDragEvent(InputEvent event) {
- return startDrag(mViewFinder.findView(event.getX(), event.getY()));
+ return startDrag(mViewFinder.findView(event.getX(), event.getY()), event);
}
/**
* May be called externally when drag is initiated from other event handling code.
*/
- private final boolean startDrag(@Nullable View view) {
+ private final boolean startDrag(@Nullable View view, InputEvent event) {
if (view == null) {
if (DEBUG) Log.d(TAG, "Ignoring drag event, null view.");
@@ -121,22 +121,7 @@
return false;
}
-
- Selection selection = new Selection();
-
- // User can drag an unselected item. Ideally if CTRL key was pressed
- // we'd extend the selection, if not, the selection would be cleared.
- // Buuuuuut, there's an impedance mismatch between event-handling policies,
- // and drag and drop. So we only initiate drag of a single item when
- // drag starts on an item that is unselected. This behavior
- // would look like a bug, if it were not for the implicitly coupled
- // behavior where we clear the selection in the UI (finish action mode)
- // in DirectoryFragment#onDragStart.
- if (!mSelectionMgr.getSelection().contains(modelId)) {
- selection.add(modelId);
- } else {
- mSelectionMgr.getSelection(selection);
- }
+ Selection selection = getSelectionToBeCopied(modelId, event);
final List<DocumentInfo> invalidDest = mDocsConverter.apply(selection);
invalidDest.add(mState.stack.peek());
@@ -159,6 +144,29 @@
}
/**
+ * Given the InputEvent (for CTRL case) and modelId of the view associated with the
+ * coordinates of the event, return a valid selection for drag and drop operation
+ */
+ @VisibleForTesting
+ Selection getSelectionToBeCopied(String modelId, InputEvent event) {
+ Selection selection = new Selection();
+ // If CTRL-key is held down and there's other existing selection, add item to
+ // selection (if not already selected)
+ if (event.isCtrlKeyDown() && !mSelectionMgr.getSelection().contains(modelId)
+ && mSelectionMgr.hasSelection()) {
+ mSelectionMgr.toggleSelection(modelId);
+ }
+
+ if (mSelectionMgr.getSelection().contains(modelId)) {
+ mSelectionMgr.getSelection(selection);
+ } else {
+ selection.add(modelId);
+ mSelectionMgr.clearSelection();
+ }
+ return selection;
+ }
+
+ /**
* This exists as a testing workaround since {@link View#startDragAndDrop} is final.
*/
@VisibleForTesting
diff --git a/tests/unit/com/android/documentsui/dirlist/DragStartListenerTest.java b/tests/unit/com/android/documentsui/dirlist/DragStartListenerTest.java
index c2bd556..f94e10f 100644
--- a/tests/unit/com/android/documentsui/dirlist/DragStartListenerTest.java
+++ b/tests/unit/com/android/documentsui/dirlist/DragStartListenerTest.java
@@ -23,6 +23,7 @@
import android.view.View;
import com.android.documentsui.base.State;
+import com.android.documentsui.dirlist.DragStartListener.ActiveListener;
import com.android.documentsui.base.Events.InputEvent;
import com.android.documentsui.selection.SelectionManager;
import com.android.documentsui.selection.Selection;
@@ -35,7 +36,7 @@
@SmallTest
public class DragStartListenerTest extends AndroidTestCase {
- private DragStartListener mListener;
+ private ActiveListener mListener;
private TestEvent.Builder mEvent;
private SelectionManager mMultiSelectManager;
private String mViewModelId;
@@ -91,6 +92,11 @@
.primary();
}
+ @Override
+ protected void tearDown() throws Exception {
+ mMultiSelectManager.clearSelection();
+ }
+
public void testDragStarted_OnMouseMove() {
assertTrue(mListener.onMouseDragEvent(mEvent.build()));
assertTrue(mDragStarted);
@@ -121,6 +127,51 @@
assertThrows(mEvent.at(-1).build());
}
+ public void testDragStart_nonSelectedItem() {
+ Selection selection = mListener.getSelectionToBeCopied("1234",
+ mEvent.action(MotionEvent.ACTION_MOVE).build());
+ assertTrue(selection.size() == 1);
+ assertTrue(selection.contains("1234"));
+ }
+
+ public void testDragStart_selectedItem() {
+ Selection selection = new Selection();
+ selection.add("1234");
+ selection.add("5678");
+ mMultiSelectManager.replaceSelection(selection);
+
+ selection = mListener.getSelectionToBeCopied("1234",
+ mEvent.action(MotionEvent.ACTION_MOVE).build());
+ assertTrue(selection.size() == 2);
+ assertTrue(selection.contains("1234"));
+ assertTrue(selection.contains("5678"));
+ }
+
+ public void testDragStart_newNonSelectedItem() {
+ Selection selection = new Selection();
+ selection.add("5678");
+ mMultiSelectManager.replaceSelection(selection);
+
+ selection = mListener.getSelectionToBeCopied("1234",
+ mEvent.action(MotionEvent.ACTION_MOVE).build());
+ assertTrue(selection.size() == 1);
+ assertTrue(selection.contains("1234"));
+ // After this, selection should be cleared
+ assertFalse(mMultiSelectManager.hasSelection());
+ }
+
+ public void testCtrlDragStart_newNonSelectedItem() {
+ Selection selection = new Selection();
+ selection.add("5678");
+ mMultiSelectManager.replaceSelection(selection);
+
+ selection = mListener.getSelectionToBeCopied("1234",
+ mEvent.action(MotionEvent.ACTION_MOVE).ctrl().build());
+ assertTrue(selection.size() == 2);
+ assertTrue(selection.contains("1234"));
+ assertTrue(selection.contains("5678"));
+ }
+
private void assertThrows(InputEvent e) {
try {
assertFalse(mListener.onMouseDragEvent(e));