Wrap up the stable ID refactor.
- Rationalize band selection: make it internally a range selection
operation, that translates positions to IDs only when updating the
Selection.
- Clean up TODOs and comments.
- Fix selection adjustment when things are removed from the view.
Change-Id: If917eb9dd18e755c5a0ce83c84409902c4ef3d2e
diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
index ef71693..5eef9b7 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
@@ -1003,10 +1003,9 @@
static private final String TAG = "DocumentsAdapter";
private final Context mContext;
/**
- * Map of model IDs to adapter positions. This is the data structure that determines what
- * shows up in the UI, and where.
+ * An ordered list of model IDs. This is the data structure that determines what shows up in
+ * the UI, and where.
*/
- // TODO(stable-id): need to keep this up-to-date when items are added/removed
private List<String> mModelIds = new ArrayList<>();
public DocumentsAdapter(Context context) {
@@ -1225,7 +1224,6 @@
@Override
public void onModelUpdate(Model model) {
- // TODO(stable-id): Sort model IDs, categorize by dir/file, etc
mModelIds = Lists.newArrayList(model.getModelIds());
}
diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
index 83c8294..43023e1 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
@@ -30,6 +30,8 @@
import android.support.v7.widget.RecyclerView;
import android.util.Log;
import android.util.SparseArray;
+import android.util.SparseBooleanArray;
+import android.util.SparseIntArray;
import android.view.GestureDetector;
import android.view.KeyEvent;
import android.view.MotionEvent;
@@ -40,6 +42,7 @@
import com.android.documentsui.R;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -140,21 +143,13 @@
mEnvironment.registerDataObserver(
new RecyclerView.AdapterDataObserver() {
- private List<String> mModelIds = new ArrayList<>();
+ private List<String> mModelIds;
@Override
public void onChanged() {
- // TODO(stable-id): This is causing b/22765812
+ // TODO: This is causing b/22765812
mSelection.clear();
-
- // TODO(stable-id): Improve this. It's currently super-inefficient,
- // performing a bunch of lookups and inserting into a List. Maybe just add
- // another method to the SelectionEnvironment to just grab the whole list at
- // once.
- mModelIds.clear();
- for (int i = 0; i < mEnvironment.getItemCount(); ++i) {
- mModelIds.add(mEnvironment.getModelIdFromAdapterPosition(i));
- }
+ mModelIds = mEnvironment.getModelIds();
}
@Override
@@ -172,14 +167,10 @@
public void onItemRangeRemoved(int startPosition, int itemCount) {
checkState(startPosition >= 0);
checkState(itemCount > 0);
+
mSelection.cancelProvisionalSelection();
- int endPosition = startPosition + itemCount;
// Remove any disappeared IDs from the selection.
- for (int i = startPosition; i < endPosition; i++) {
- String id = mModelIds.get(i);
- mSelection.remove(id);
- mModelIds.remove(i);
- }
+ mSelection.intersect(mModelIds);
}
@Override
@@ -731,6 +722,14 @@
mTotalSelection.clear();
}
+ /**
+ * Trims this selection to be the intersection of itself with the set of given IDs.
+ */
+ public void intersect(Collection<String> ids) {
+ mSavedSelection.retainAll(ids);
+ mTotalSelection.retainAll(ids);
+ }
+
@VisibleForTesting
void copyFrom(Selection source) {
mSavedSelection = new HashSet<>(source.mSavedSelection);
@@ -794,6 +793,7 @@
void removeCallback(Runnable r);
Point createAbsolutePoint(Point relativePoint);
Rect getAbsoluteRectForChildViewAt(int index);
+ int getAdapterPositionAt(int index);
int getAdapterPositionForChildView(View view);
int getColumnCount();
int getRowCount();
@@ -801,9 +801,8 @@
int getVisibleChildCount();
void focusItem(int position);
String getModelIdFromAdapterPosition(int position);
- String getModelIdAt(int index);
- String getModelIdForChildView(View view);
int getItemCount();
+ List<String> getModelIds();
void notifyItemChanged(String id, String selectionChangedMarker);
void registerDataObserver(RecyclerView.AdapterDataObserver observer);
}
@@ -833,14 +832,8 @@
}
@Override
- public String getModelIdForChildView(View view) {
- if (view.getParent() == mView) {
- RecyclerView.ViewHolder vh = mView.getChildViewHolder(view);
- if (vh instanceof DirectoryFragment.DocumentHolder) {
- return ((DirectoryFragment.DocumentHolder) vh).modelId;
- }
- }
- return null;
+ public int getAdapterPositionAt(int index) {
+ return getAdapterPositionForChildView(mView.getChildAt(index));
}
@Override
@@ -849,11 +842,6 @@
}
@Override
- public String getModelIdAt(int index) {
- return getModelIdForChildView(mView.getChildAt(index));
- }
-
- @Override
public void addOnScrollListener(RecyclerView.OnScrollListener listener) {
mView.addOnScrollListener(listener);
}
@@ -997,6 +985,11 @@
public void registerDataObserver(RecyclerView.AdapterDataObserver observer) {
mAdapter.registerAdapterDataObserver(observer);
}
+
+ @Override
+ public List<String> getModelIds() {
+ return mAdapter.getModelIds();
+ }
}
public interface Callback {
@@ -1179,15 +1172,14 @@
mEnvironment.hideBand();
mSelection.applyProvisionalSelection();
mModel.endSelection();
- String firstSelected = mModel.getItemNearestOrigin();
- if (!mSelection.contains(firstSelected)) {
+ int firstSelected = mModel.getPositionNearestOrigin();
+ if (!mSelection.contains(mEnvironment.getModelIdFromAdapterPosition(firstSelected))) {
Log.w(TAG, "First selected by band is NOT in selection!");
// Sadly this is really happening. Need to figure out what's going on.
- } else if (firstSelected != null) {
- // TODO(stable-id): firstSelected should really be lastSelected, we want to anchor the
- // range where the mouse-up occurred. Also figure out how to translate the model ID
- // into a position.
- // setSelectionRangeBegin(firstSelected);
+ } else if (firstSelected != NOT_SET) {
+ // TODO: firstSelected should really be lastSelected, we want to anchor the item
+ // where the mouse-up occurred.
+ setSelectionRangeBegin(firstSelected);
}
mModel = null;
@@ -1349,7 +1341,7 @@
// by their y-offset. For example, if the first column of the view starts at an x-value of 5,
// mColumns.get(5) would return an array of positions in that column. Within that array, the
// value for key y is the adapter position for the item whose y-offset is y.
- private final SparseArray<SparseArray<String>> mColumns = new SparseArray<>();
+ private final SparseArray<SparseIntArray> mColumns = new SparseArray<>();
// List of limits along the x-axis (columns).
// This list is sorted from furthest left to furthest right.
@@ -1360,7 +1352,7 @@
private final List<Limits> mRowBounds = new ArrayList<>();
// The adapter positions which have been recorded so far.
- private final Set<String> mKnownIds = new HashSet<>();
+ private final SparseBooleanArray mKnownPositions = new SparseBooleanArray();
// Array passed to registered OnSelectionChangedListeners. One array is created and reused
// throughout the lifetime of the object.
@@ -1377,7 +1369,7 @@
// Tracks where the band select originated from. This is used to determine where selections
// should expand from when Shift+click is used.
- private String mItemNearestOrigin = null;
+ private int mPositionNearestOrigin = NOT_SET;
GridModel(SelectionEnvironment helper) {
mHelper = helper;
@@ -1434,8 +1426,8 @@
* @return The adapter position for the item nearest the origin corresponding to the latest
* band select operation, or NOT_SET if the selection did not cover any items.
*/
- String getItemNearestOrigin() {
- return mItemNearestOrigin;
+ int getPositionNearestOrigin() {
+ return mPositionNearestOrigin;
}
@Override
@@ -1455,10 +1447,10 @@
*/
private void recordVisibleChildren() {
for (int i = 0; i < mHelper.getVisibleChildCount(); i++) {
- String modelId = mHelper.getModelIdAt(i);
- if (!mKnownIds.contains(modelId)) {
- mKnownIds.add(modelId);
- recordItemData(mHelper.getAbsoluteRectForChildViewAt(i), modelId);
+ int adapterPosition = mHelper.getAdapterPositionAt(i);
+ if (!mKnownPositions.get(adapterPosition)) {
+ mKnownPositions.put(adapterPosition, true);
+ recordItemData(mHelper.getAbsoluteRectForChildViewAt(i), adapterPosition);
}
}
}
@@ -1468,7 +1460,7 @@
* @param absoluteChildRect The absolute rectangle for the child view being processed.
* @param adapterPosition The position of the child view being processed.
*/
- private void recordItemData(Rect absoluteChildRect, String modelId) {
+ private void recordItemData(Rect absoluteChildRect, int adapterPosition) {
if (mColumnBounds.size() != mHelper.getColumnCount()) {
// If not all x-limits have been recorded, record this one.
recordLimits(
@@ -1481,12 +1473,12 @@
mRowBounds, new Limits(absoluteChildRect.top, absoluteChildRect.bottom));
}
- SparseArray<String> columnList = mColumns.get(absoluteChildRect.left);
+ SparseIntArray columnList = mColumns.get(absoluteChildRect.left);
if (columnList == null) {
- columnList = new SparseArray<>();
+ columnList = new SparseIntArray();
mColumns.put(absoluteChildRect.left, columnList);
}
- columnList.put(absoluteChildRect.top, modelId);
+ columnList.put(absoluteChildRect.top, adapterPosition);
}
/**
@@ -1523,7 +1515,7 @@
updateSelection(computeBounds());
} else {
mSelection.clear();
- mItemNearestOrigin = null;
+ mPositionNearestOrigin = NOT_SET;
}
}
@@ -1552,11 +1544,11 @@
columnEndIndex = i;
}
- SparseArray<String> firstColumn =
+ SparseIntArray firstColumn =
mColumns.get(mColumnBounds.get(columnStartIndex).lowerLimit);
int rowStartIndex = firstColumn.indexOfKey(rect.top);
if (rowStartIndex < 0) {
- mItemNearestOrigin = null;
+ mPositionNearestOrigin = NOT_SET;
return;
}
@@ -1577,15 +1569,20 @@
int columnStartIndex, int columnEndIndex, int rowStartIndex, int rowEndIndex) {
mSelection.clear();
for (int column = columnStartIndex; column <= columnEndIndex; column++) {
- SparseArray<String> items = mColumns.get(mColumnBounds.get(column).lowerLimit);
+ SparseIntArray items = mColumns.get(mColumnBounds.get(column).lowerLimit);
for (int row = rowStartIndex; row <= rowEndIndex; row++) {
- String id = items.get(items.keyAt(row));
- mSelection.add(id);
+ int position = items.get(items.keyAt(row));
+ String id = mHelper.getModelIdFromAdapterPosition(position);
+ if (id != null) {
+ // The adapter inserts items for UI layout purposes that aren't associated
+ // with files. Those will have a null model ID. Don't select them.
+ mSelection.add(id);
+ }
if (isPossiblePositionNearestOrigin(column, columnStartIndex, columnEndIndex,
row, rowStartIndex, rowEndIndex)) {
// If this is the position nearest the origin, record it now so that it
// can be returned by endSelection() later.
- mItemNearestOrigin = id;
+ mPositionNearestOrigin = position;
}
}
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_GridModelTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_GridModelTest.java
index 153f58b..d6e8b55 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_GridModelTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_GridModelTest.java
@@ -16,17 +16,19 @@
package com.android.documentsui.dirlist;
+import static com.android.documentsui.dirlist.MultiSelectManager.GridModel.NOT_SET;
+
import android.graphics.Point;
import android.graphics.Rect;
import android.support.v7.widget.RecyclerView.AdapterDataObserver;
import android.support.v7.widget.RecyclerView.OnScrollListener;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
-import android.util.Log;
import android.view.View;
import com.android.documentsui.dirlist.MultiSelectManager.GridModel;
+import java.util.List;
import java.util.Set;
@SmallTest
@@ -66,7 +68,7 @@
model.startSelection(new Point(0, 10));
model.resizeSelection(new Point(1, 11));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testSelectionRightOfItems() {
@@ -74,7 +76,7 @@
model.startSelection(new Point(viewWidth - 1, 10));
model.resizeSelection(new Point(viewWidth - 2, 11));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testSelectionAboveItems() {
@@ -82,7 +84,7 @@
model.startSelection(new Point(10, 0));
model.resizeSelection(new Point(11, 1));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testSelectionBelowItems() {
@@ -90,7 +92,7 @@
model.startSelection(new Point(10, VIEWPORT_HEIGHT - 1));
model.resizeSelection(new Point(11, VIEWPORT_HEIGHT - 2));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testVerticalSelectionBetweenItems() {
@@ -98,7 +100,7 @@
model.startSelection(new Point(106, 0));
model.resizeSelection(new Point(107, 200));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testHorizontalSelectionBetweenItems() {
@@ -106,7 +108,7 @@
model.startSelection(new Point(0, 105));
model.resizeSelection(new Point(200, 106));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testGrowingAndShrinkingSelection() {
@@ -136,7 +138,7 @@
assertSelected(0);
model.resizeSelection(new Point(0, 0));
assertSelected();
- assertEquals(null, model.getItemNearestOrigin());
+ assertEquals(NOT_SET, model.getPositionNearestOrigin());
}
public void testSelectionMovingAroundOrigin() {
@@ -150,7 +152,7 @@
assertSelected(8, 9, 12, 13);
model.resizeSelection(new Point(viewWidth - 1, 420));
assertSelected(10, 11, 14, 15);
- assertEquals("10", model.getItemNearestOrigin());
+ assertEquals(10, model.getPositionNearestOrigin());
}
public void testScrollingBandSelect() {
@@ -168,7 +170,7 @@
assertSelected(0, 1, 4, 5, 8, 9, 12, 13, 16, 17);
model.resizeSelection(new Point(100, VIEWPORT_HEIGHT - 1));
assertSelected(0, 4, 8, 12, 16);
- assertEquals("0", model.getItemNearestOrigin());
+ assertEquals(0, model.getPositionNearestOrigin());
}
private static void assertSelected(int... selectedPositions) {
@@ -243,6 +245,11 @@
}
@Override
+ public int getAdapterPositionAt(int index) {
+ return index + mNumColumns * (getFirstVisibleRowIndex());
+ }
+
+ @Override
public Rect getAbsoluteRectForChildViewAt(int index) {
int adapterPosition = (getFirstVisibleRowIndex() * mNumColumns) + index;
int rowIndex = adapterPosition / mNumColumns;
@@ -322,22 +329,16 @@
}
@Override
- public String getModelIdAt(int index) {
- int firstVisibleChildIndex = getFirstVisibleRowIndex() * mNumColumns;
- return Integer.toString(firstVisibleChildIndex + index);
- }
-
- @Override
- public String getModelIdForChildView(View view) {
- throw new UnsupportedOperationException();
- }
-
- @Override
public int getItemCount() {
return mNumChildren;
}
@Override
+ public List<String> getModelIds() {
+ return null;
+ }
+
+ @Override
public void notifyItemChanged(String id, String selectionChangedMarker) {
throw new UnsupportedOperationException();
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_SelectionTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_SelectionTest.java
index 7c36d63..444b2dc 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_SelectionTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManager_SelectionTest.java
@@ -20,6 +20,10 @@
import android.test.suitebuilder.annotation.SmallTest;
import com.android.documentsui.dirlist.MultiSelectManager.Selection;
+import com.google.common.collect.Sets;
+
+import java.util.HashSet;
+import java.util.Set;
@SmallTest
public class MultiSelectManager_SelectionTest extends AndroidTestCase {
@@ -100,6 +104,72 @@
assertFalse(selection.equals(other));
}
+ public void testIntersection_empty0() {
+ Selection testSelection = new Selection();
+ testSelection.intersect(new HashSet<String>());
+ assertTrue(testSelection.isEmpty());
+ }
+
+ public void testIntersection_empty1() {
+ Selection testSelection = new Selection();
+ testSelection.intersect(Sets.newHashSet("foo"));
+ assertTrue(testSelection.isEmpty());
+ }
+
+ public void testIntersection_empty2() {
+ assertFalse(selection.isEmpty());
+ selection.intersect(new HashSet<String>());
+ assertTrue(selection.isEmpty());
+ }
+
+ public void testIntersection_exclusive() {
+ String[] ids0 = new String[]{"foo", "bar", "baz"};
+ String[] ids1 = new String[]{"0", "1", "2"};
+
+ Selection testSelection = new Selection();
+ testSelection.add(ids0[0]);
+ testSelection.add(ids0[1]);
+ testSelection.add(ids0[2]);
+
+ Set<String> set = Sets.newHashSet(ids1);
+ testSelection.intersect(set);
+
+ assertTrue(testSelection.isEmpty());
+ }
+
+ public void testIntersection_subset() {
+ String[] ids0 = new String[]{"foo", "bar", "baz"};
+ String[] ids1 = new String[]{"0", "baz", "1", "foo", "2"};
+
+ Selection testSelection = new Selection();
+ testSelection.add(ids0[0]);
+ testSelection.add(ids0[1]);
+ testSelection.add(ids0[2]);
+
+ testSelection.intersect(Sets.newHashSet(ids1));
+
+ assertTrue(testSelection.contains("foo"));
+ assertFalse(testSelection.contains("bar"));
+ assertTrue(testSelection.contains("baz"));
+ }
+
+ public void testIntersection_all() {
+ String[] ids0 = new String[]{"foo", "bar", "baz"};
+ String[] ids1 = new String[]{"0", "baz", "1", "foo", "2", "bar"};
+
+ Selection testSelection = new Selection();
+ testSelection.add(ids0[0]);
+ testSelection.add(ids0[1]);
+ testSelection.add(ids0[2]);
+
+ Selection control = new Selection();
+ control.copyFrom(testSelection);
+
+ testSelection.intersect(Sets.newHashSet(ids1));
+
+ assertTrue(testSelection.equals(control));
+ }
+
private void assertContains(String id) {
String err = String.format("Selection %s does not contain %s", selection, id);
assertTrue(err, selection.contains(id));
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java
index 9608e7e..6805644 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java
@@ -25,6 +25,7 @@
import com.android.documentsui.dirlist.MultiSelectManager.SelectionEnvironment;
import java.util.List;
+import java.util.Set;
public class TestSelectionEnvironment implements SelectionEnvironment {
@@ -82,6 +83,11 @@
}
@Override
+ public int getAdapterPositionAt(int index) {
+ return 0;
+ }
+
+ @Override
public int getAdapterPositionForChildView(View view) {
return 0;
}
@@ -116,21 +122,16 @@
}
@Override
- public String getModelIdAt(int index) {
- return null;
- }
-
- @Override
- public String getModelIdForChildView(View view) {
- return null;
- }
-
- @Override
public int getItemCount() {
return mItems.size();
}
@Override
+ public List<String> getModelIds() {
+ return null;
+ }
+
+ @Override
public void notifyItemChanged(String id, String selectionChangedMarker) {
}