UserInputHandler cleanup.
Obtain DocumentDetails directly from Event...fixing stack trace.
Don't try to do stuff in UserIntputHandler on events w/o details.
Don't call TouchDelegate#onLongPress for mouse events.
Don't clear selection when clicking in mime type icon (w/o CTRL).
Bug: 31350922,31381545
Change-Id: Ibc4e5335cdb41569f9863d87f82dafca58659974
diff --git a/src/com/android/documentsui/Events.java b/src/com/android/documentsui/Events.java
index c878a5e..0767bd9 100644
--- a/src/com/android/documentsui/Events.java
+++ b/src/com/android/documentsui/Events.java
@@ -26,6 +26,11 @@
import android.view.MotionEvent;
import android.view.View;
+import com.android.documentsui.dirlist.DocumentDetails;
+import com.android.documentsui.dirlist.DocumentHolder;
+
+import javax.annotation.Nullable;
+
/**
* Utility code for dealing with MotionEvents.
*/
@@ -142,19 +147,23 @@
/** Returns the adapter position of the item under the finger/cursor. */
int getItemPosition();
+
+ /** Returns the DocumentDetails for the item under the event, or null. */
+ @Nullable DocumentDetails getDocumentDetails();
}
public static final class MotionInputEvent implements InputEvent {
private static final String TAG = "MotionInputEvent";
+ private static final int UNSET_POSITION = RecyclerView.NO_POSITION - 1;
+
private static final Pools.SimplePool<MotionInputEvent> sPool = new Pools.SimplePool<>(1);
private MotionEvent mEvent;
- interface PositionProvider {
- int get(MotionEvent e);
- }
+ private @Nullable RecyclerView mRecView;
- private int mPosition;
+ private int mPosition = UNSET_POSITION;
+ private @Nullable DocumentDetails mDocDetails;
private MotionInputEvent() {
if (DEBUG) Log.i(TAG, "Created a new instance.");
@@ -167,25 +176,7 @@
instance = (instance != null ? instance : new MotionInputEvent());
instance.mEvent = event;
-
- // Consider determining position lazily as an optimization.
- View child = view.findChildViewUnder(event.getX(), event.getY());
- instance.mPosition = (child != null)
- ? view.getChildAdapterPosition(child)
- : RecyclerView.NO_POSITION;
-
- return instance;
- }
-
- public static MotionInputEvent obtain(
- MotionEvent event, PositionProvider positionProvider) {
- Shared.checkMainLoop();
-
- MotionInputEvent instance = sPool.acquire();
- instance = (instance != null ? instance : new MotionInputEvent());
-
- instance.mEvent = event;
- instance.mPosition = positionProvider.get(event);
+ instance.mRecView = view;
return instance;
}
@@ -194,7 +185,9 @@
Shared.checkMainLoop();
mEvent = null;
- mPosition = -1;
+ mRecView = null;
+ mPosition = UNSET_POSITION;
+ mDocDetails = null;
boolean released = sPool.release(this);
// This assert is used to guarantee we won't generate too many instances that can't be
@@ -282,10 +275,27 @@
@Override
public int getItemPosition() {
+ if (mPosition == UNSET_POSITION) {
+ View child = mRecView.findChildViewUnder(mEvent.getX(), mEvent.getY());
+ mPosition = (child != null)
+ ? mRecView.getChildAdapterPosition(child)
+ : RecyclerView.NO_POSITION;
+ }
return mPosition;
}
@Override
+ public @Nullable DocumentDetails getDocumentDetails() {
+ if (mDocDetails == null) {
+ View childView = mRecView.findChildViewUnder(mEvent.getX(), mEvent.getY());
+ mDocDetails = (childView != null)
+ ? (DocumentHolder) mRecView.getChildViewHolder(childView)
+ : null;
+ }
+ return mDocDetails;
+ }
+
+ @Override
public String toString() {
return new StringBuilder()
.append("MotionInputEvent {")
@@ -298,6 +308,7 @@
.append(" getOrigin=").append(getOrigin())
.append(" isOverItem=").append(isOverItem())
.append(" getItemPosition=").append(getItemPosition())
+ .append(" getDocumentDetails=").append(getDocumentDetails())
.append("}")
.toString();
}
diff --git a/src/com/android/documentsui/dirlist/DirectoryFragment.java b/src/com/android/documentsui/dirlist/DirectoryFragment.java
index d93a241..367a134 100644
--- a/src/com/android/documentsui/dirlist/DirectoryFragment.java
+++ b/src/com/android/documentsui/dirlist/DirectoryFragment.java
@@ -340,7 +340,6 @@
mSelectionMgr,
mFocusManager,
(MotionEvent t) -> MotionInputEvent.obtain(t, mRecView),
- this::getTarget,
this::canSelect,
this::onRightClick,
(DocumentDetails doc) -> handleViewItem(doc.getModelId()), // activate handler
@@ -475,8 +474,8 @@
protected boolean onRightClick(InputEvent e) {
if (e.getItemPosition() != RecyclerView.NO_POSITION) {
final DocumentHolder doc = getTarget(e);
- if (!mSelectionMgr.getSelection().contains(doc.modelId)) {
- mSelectionMgr.replaceSelection(Collections.singleton(doc.modelId));
+ if (!mSelectionMgr.getSelection().contains(doc.getModelId())) {
+ mSelectionMgr.replaceSelection(Collections.singleton(doc.getModelId()));
}
// We are registering for context menu here so long-press doesn't trigger this
@@ -1419,11 +1418,9 @@
private @Nullable DocumentHolder getTarget(InputEvent e) {
View childView = mRecView.findChildViewUnder(e.getX(), e.getY());
- if (childView != null) {
- return (DocumentHolder) mRecView.getChildViewHolder(childView);
- } else {
- return null;
- }
+ return (childView != null)
+ ? (DocumentHolder) mRecView.getChildViewHolder(childView)
+ : null;
}
/**
@@ -1437,7 +1434,7 @@
if (itemView != null) {
RecyclerView.ViewHolder vh = mRecView.getChildViewHolder(itemView);
if (vh instanceof DocumentHolder) {
- return ((DocumentHolder) vh).modelId;
+ return ((DocumentHolder) vh).getModelId();
}
}
return null;
diff --git a/src/com/android/documentsui/dirlist/DocumentDetails.java b/src/com/android/documentsui/dirlist/DocumentDetails.java
index 27a2234..5dac633 100644
--- a/src/com/android/documentsui/dirlist/DocumentDetails.java
+++ b/src/com/android/documentsui/dirlist/DocumentDetails.java
@@ -22,6 +22,7 @@
* Interface providing a loose coupling between DocumentHolder.
*/
public interface DocumentDetails {
+ boolean hasModelId();
String getModelId();
int getAdapterPosition();
boolean isInSelectionHotspot(InputEvent event);
diff --git a/src/com/android/documentsui/dirlist/DocumentHolder.java b/src/com/android/documentsui/dirlist/DocumentHolder.java
index a9830f2..8c93595 100644
--- a/src/com/android/documentsui/dirlist/DocumentHolder.java
+++ b/src/com/android/documentsui/dirlist/DocumentHolder.java
@@ -22,6 +22,7 @@
import android.graphics.Rect;
import android.support.annotation.Nullable;
import android.support.v7.widget.RecyclerView;
+import android.text.TextUtils;
import android.view.KeyEvent;
import android.view.LayoutInflater;
import android.view.View;
@@ -39,8 +40,7 @@
static final float DISABLED_ALPHA = 0.3f;
- @Deprecated // Public access is deprecated, use #getModelId.
- public @Nullable String modelId;
+ protected @Nullable String modelId;
final Context mContext;
final @ColorInt int mDefaultBgColor;
@@ -77,6 +77,11 @@
public abstract void bind(Cursor cursor, String modelId, State state);
@Override
+ public boolean hasModelId() {
+ return !TextUtils.isEmpty(modelId);
+ }
+
+ @Override
public String getModelId() {
return modelId;
}
diff --git a/src/com/android/documentsui/dirlist/UserInputHandler.java b/src/com/android/documentsui/dirlist/UserInputHandler.java
index 7304a8f..363055b 100644
--- a/src/com/android/documentsui/dirlist/UserInputHandler.java
+++ b/src/com/android/documentsui/dirlist/UserInputHandler.java
@@ -31,6 +31,8 @@
import java.util.function.Function;
import java.util.function.Predicate;
+import javax.annotation.Nullable;
+
/**
* Grand unified-ish gesture/event listener for items in the directory list.
*/
@@ -43,7 +45,6 @@
private final MultiSelectManager mSelectionMgr;
private final FocusHandler mFocusHandler;
private final Function<MotionEvent, T> mEventConverter;
- private final Function<T, DocumentDetails> mDocFinder;
private final Predicate<DocumentDetails> mSelectable;
private final EventHandler mRightClickHandler;
private final DocumentHandler mActivateHandler;
@@ -58,7 +59,6 @@
MultiSelectManager selectionMgr,
FocusHandler focusHandler,
Function<MotionEvent, T> eventConverter,
- Function<T, DocumentDetails> docFinder,
Predicate<DocumentDetails> selectable,
EventHandler rightClickHandler,
DocumentHandler activateHandler,
@@ -69,7 +69,6 @@
mSelectionMgr = selectionMgr;
mFocusHandler = focusHandler;
mEventConverter = eventConverter;
- mDocFinder = docFinder;
mSelectable = selectable;
mRightClickHandler = rightClickHandler;
mActivateHandler = activateHandler;
@@ -164,8 +163,9 @@
void onLongPress(T event) {
if (event.isMouseEvent()) {
mMouseDelegate.onLongPress(event);
+ } else {
+ mTouchDelegate.onLongPress(event);
}
- mTouchDelegate.onLongPress(event);
}
// Only events from RecyclerView are fed into UserInputHandler#onDown.
@@ -188,21 +188,24 @@
private boolean selectDocument(DocumentDetails doc) {
assert(doc != null);
+ assert(doc.hasModelId());
mSelectionMgr.toggleSelection(doc.getModelId());
mSelectionMgr.setSelectionRangeBegin(doc.getAdapterPosition());
return true;
}
+ private void extendSelectionRange(DocumentDetails doc) {
+ mSelectionMgr.snapRangeSelection(doc.getAdapterPosition());
+ }
+
boolean isRangeExtension(T event) {
return event.isShiftKeyDown() && mSelectionMgr.isRangeSelectionActive();
}
- private void extendSelectionRange(T event) {
- mSelectionMgr.snapRangeSelection(event.getItemPosition());
- }
-
private boolean shouldClearSelection(T event, DocumentDetails doc) {
- return !event.isCtrlKeyDown() && !mSelectionMgr.getSelection().contains(doc.getModelId());
+ return !event.isCtrlKeyDown()
+ && !doc.isInSelectionHotspot(event)
+ && !mSelectionMgr.getSelection().contains(doc.getModelId());
}
private final class TouchInputDelegate {
@@ -223,26 +226,26 @@
return false;
}
+ @Nullable DocumentDetails doc = event.getDocumentDetails();
+ if (doc == null || !doc.hasModelId()) {
+ Log.w(TAG, "Ignoring Single Tap Up. No document details associated w/ event.");
+ return false;
+ }
+
if (mSelectionMgr.hasSelection()) {
if (isRangeExtension(event)) {
- mSelectionMgr.snapRangeSelection(event.getItemPosition());
+ extendSelectionRange(doc);
} else {
- selectDocument(mDocFinder.apply(event));
+ selectDocument(doc);
}
return true;
}
- // Give the DocumentHolder a crack at the event.
- DocumentDetails doc = mDocFinder.apply(event);
- if (doc != null) {
- // Touch events select if they occur in the selection hotspot,
- // otherwise they activate.
- return doc.isInSelectionHotspot(event)
- ? selectDocument(doc)
- : activateDocument(doc);
- }
-
- return false;
+ // Touch events select if they occur in the selection hotspot,
+ // otherwise they activate.
+ return doc.isInSelectionHotspot(event)
+ ? selectDocument(doc)
+ : activateDocument(doc);
}
boolean onSingleTapConfirmed(T event) {
@@ -255,15 +258,21 @@
final void onLongPress(T event) {
if (!event.isOverItem()) {
+ if (DEBUG) Log.d(TAG, "Ignoring Long Press on non-item.");
+ return;
+ }
+
+ @Nullable DocumentDetails doc = event.getDocumentDetails();
+ if (doc == null || !doc.hasModelId()) {
+ Log.w(TAG, "Ignoring Long Press. No document details associated w/ event.");
return;
}
if (isRangeExtension(event)) {
- extendSelectionRange(event);
+ extendSelectionRange(doc);
} else {
- DocumentDetails doc = mDocFinder.apply(event);
if (!mSelectionMgr.getSelection().contains(doc.getModelId())) {
- selectDocument(mDocFinder.apply(event));
+ selectDocument(doc);
mGestureSelectHandler.apply(event);
} else {
// We only initiate drag and drop on long press for touch to allow regular
@@ -305,15 +314,21 @@
}
if (!event.isOverItem()) {
+ if (DEBUG) Log.d(TAG, "Tap on non-item. Clearing selection.");
mSelectionMgr.clearSelection();
return false;
}
+ @Nullable DocumentDetails doc = event.getDocumentDetails();
+ if (doc == null || !doc.hasModelId()) {
+ Log.w(TAG, "Ignoring Single Tap Up. No document details associated w/ event.");
+ return false;
+ }
+
if (mSelectionMgr.hasSelection()) {
if (isRangeExtension(event)) {
- extendSelectionRange(event);
+ extendSelectionRange(doc);
} else {
- DocumentDetails doc = mDocFinder.apply(event);
if (shouldClearSelection(event, doc)) {
mSelectionMgr.clearSelection();
}
@@ -323,19 +338,7 @@
return true;
}
- // We'll toggle selection in onSingleTapConfirmed
- // This avoids flickering on/off action mode when an item is double clicked.
- if (!mSelectionMgr.hasSelection()) {
- return false;
- }
-
- DocumentDetails doc = mDocFinder.apply(event);
- if (doc == null) {
- return false;
- }
-
- mHandledTapUp = true;
- return selectDocument(doc);
+ return false;
}
boolean onSingleTapConfirmed(T event) {
@@ -348,8 +351,14 @@
return false; // should have been handled by onSingleTapUp.
}
- DocumentDetails doc = mDocFinder.apply(event);
- if (doc == null) {
+ if (!event.isOverItem()) {
+ if (DEBUG) Log.d(TAG, "Ignoring Confirmed Tap on non-item.");
+ return false;
+ }
+
+ @Nullable DocumentDetails doc = event.getDocumentDetails();
+ if (doc == null || !doc.hasModelId()) {
+ Log.w(TAG, "Ignoring Confirmed Tap. No document details associated w/ event.");
return false;
}
@@ -358,25 +367,25 @@
boolean onDoubleTap(T event) {
mHandledTapUp = false;
- DocumentDetails doc = mDocFinder.apply(event);
- if (doc != null) {
- return mSelectionMgr.hasSelection()
- ? selectDocument(doc)
- : activateDocument(doc);
+
+ if (!event.isOverItem()) {
+ if (DEBUG) Log.d(TAG, "Ignoring DoubleTap on non-item.");
+ return false;
}
- return false;
+
+ @Nullable DocumentDetails doc = event.getDocumentDetails();
+ if (doc == null || !doc.hasModelId()) {
+ Log.w(TAG, "Ignoring Double Tap. No document details associated w/ event.");
+ return false;
+ }
+
+ return mSelectionMgr.hasSelection()
+ ? selectDocument(doc)
+ : activateDocument(doc);
}
final void onLongPress(T event) {
- if (!event.isOverItem()) {
- return;
- }
-
- if (isRangeExtension(event)) {
- extendSelectionRange(event);
- } else {
- selectDocument(mDocFinder.apply(event));
- }
+ return;
}
private boolean onRightClick(T event) {
diff --git a/tests/src/com/android/documentsui/TestInputEvent.java b/tests/src/com/android/documentsui/TestInputEvent.java
index 70a4308..bcbb4bf 100644
--- a/tests/src/com/android/documentsui/TestInputEvent.java
+++ b/tests/src/com/android/documentsui/TestInputEvent.java
@@ -3,6 +3,8 @@
import android.graphics.Point;
import android.support.v7.widget.RecyclerView;
+import com.android.documentsui.dirlist.DocumentDetails;
+
public class TestInputEvent implements Events.InputEvent {
public boolean mouseEvent;
@@ -16,6 +18,7 @@
public Point location;
public Point rawLocation;
public int position = Integer.MIN_VALUE;
+ public DocumentDetails details;
public TestInputEvent() {}
@@ -99,6 +102,11 @@
}
@Override
+ public DocumentDetails getDocumentDetails() {
+ return details;
+ }
+
+ @Override
public void close() {}
public static TestInputEvent tap(int position) {
diff --git a/tests/src/com/android/documentsui/dirlist/UserInputHandler_MouseTest.java b/tests/src/com/android/documentsui/dirlist/UserInputHandler_MouseTest.java
index 6a50c3c..bda4569 100644
--- a/tests/src/com/android/documentsui/dirlist/UserInputHandler_MouseTest.java
+++ b/tests/src/com/android/documentsui/dirlist/UserInputHandler_MouseTest.java
@@ -17,7 +17,6 @@
package com.android.documentsui.dirlist;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.assertFalse;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
@@ -76,9 +75,6 @@
(MotionEvent event) -> {
throw new UnsupportedOperationException("Not exercised in tests.");
},
- (TestEvent event) -> {
- return event.getDocument();
- },
mCanSelect,
mRightClickHandler::test,
mActivateHandler::test,
diff --git a/tests/src/com/android/documentsui/dirlist/UserInputHandler_RangeTest.java b/tests/src/com/android/documentsui/dirlist/UserInputHandler_RangeTest.java
index 53f3540..4172d1b 100644
--- a/tests/src/com/android/documentsui/dirlist/UserInputHandler_RangeTest.java
+++ b/tests/src/com/android/documentsui/dirlist/UserInputHandler_RangeTest.java
@@ -75,9 +75,6 @@
(MotionEvent event) -> {
throw new UnsupportedOperationException("Not exercised in tests.");
},
- (TestEvent event) -> {
- return event.getDocument();
- },
mCanSelect,
mRightClickHandler::test,
mActivateHandler::test,
diff --git a/tests/src/com/android/documentsui/dirlist/UserInputHandler_TouchTest.java b/tests/src/com/android/documentsui/dirlist/UserInputHandler_TouchTest.java
index 23076d7..898c642 100644
--- a/tests/src/com/android/documentsui/dirlist/UserInputHandler_TouchTest.java
+++ b/tests/src/com/android/documentsui/dirlist/UserInputHandler_TouchTest.java
@@ -75,9 +75,6 @@
(MotionEvent event) -> {
throw new UnsupportedOperationException("Not exercised in tests.");
},
- (TestEvent event) -> {
- return event.getDocument();
- },
mCanSelect,
mRightClickHandler::test,
mActivateHandler::test,
diff --git a/tests/src/com/android/documentsui/testing/TestEvent.java b/tests/src/com/android/documentsui/testing/TestEvent.java
index ed726ab..de107f5 100644
--- a/tests/src/com/android/documentsui/testing/TestEvent.java
+++ b/tests/src/com/android/documentsui/testing/TestEvent.java
@@ -29,6 +29,15 @@
private String modelId;
private boolean inHotspot;
+ public TestEvent() {
+ details = this;
+ }
+
+ @Override
+ public boolean hasModelId() {
+ return modelId != null;
+ }
+
@Override
public String getModelId() {
return modelId;
@@ -65,10 +74,6 @@
return this;
}
- public DocumentDetails getDocument() {
- return this;
- }
-
@Override
public int hashCode() {
return modelId != null ? modelId.hashCode() : -1;