Merge "Fix band selection problems." into nyc-dev

This commit is contained in:
Ben Kwa
2016-03-16 20:06:12 +00:00
committed by Android (Google) Code Review
3 changed files with 224 additions and 107 deletions

View File

@@ -903,7 +903,7 @@ public final class MultiSelectManager {
public Selection createFromParcel(Parcel in, ClassLoader loader) { public Selection createFromParcel(Parcel in, ClassLoader loader) {
return new Selection( return new Selection(
in.readString(), in.readString(),
(ArrayList<String>) in.readArrayList(loader)); in.readArrayList(loader));
} }
@Override @Override
@@ -931,7 +931,6 @@ public final class MultiSelectManager {
Rect getAbsoluteRectForChildViewAt(int index); Rect getAbsoluteRectForChildViewAt(int index);
int getAdapterPositionAt(int index); int getAdapterPositionAt(int index);
int getColumnCount(); int getColumnCount();
int getRowCount();
int getChildCount(); int getChildCount();
int getVisibleChildCount(); int getVisibleChildCount();
/** /**
@@ -1007,13 +1006,6 @@ public final class MultiSelectManager {
return 1; return 1;
} }
@Override
public int getRowCount() {
int numFullColumns = getChildCount() / getColumnCount();
boolean hasPartiallyFullColumn = getChildCount() % getColumnCount() != 0;
return numFullColumns + (hasPartiallyFullColumn ? 1 : 0);
}
@Override @Override
public int getHeight() { public int getHeight() {
return mView.getHeight(); return mView.getHeight();
@@ -1202,6 +1194,7 @@ public final class MultiSelectManager {
} }
mCurrentPosition = input.getOrigin(); mCurrentPosition = input.getOrigin();
mModel.resizeSelection(input.getOrigin());
scrollViewIfNecessary(); scrollViewIfNecessary();
resizeBandSelectRectangle(); resizeBandSelectRectangle();
} }
@@ -1549,11 +1542,7 @@ public final class MultiSelectManager {
mColumnBounds, new Limits(absoluteChildRect.left, absoluteChildRect.right)); mColumnBounds, new Limits(absoluteChildRect.left, absoluteChildRect.right));
} }
if (mRowBounds.size() != mHelper.getRowCount()) { recordLimits(mRowBounds, new Limits(absoluteChildRect.top, absoluteChildRect.bottom));
// If not all y-limits have been recorded, record this one.
recordLimits(
mRowBounds, new Limits(absoluteChildRect.top, absoluteChildRect.bottom));
}
SparseIntArray columnList = mColumns.get(absoluteChildRect.left); SparseIntArray columnList = mColumns.get(absoluteChildRect.left);
if (columnList == null) { if (columnList == null) {
@@ -1747,6 +1736,11 @@ public final class MultiSelectManager {
return ((Limits) other).lowerLimit == lowerLimit && return ((Limits) other).lowerLimit == lowerLimit &&
((Limits) other).upperLimit == upperLimit; ((Limits) other).upperLimit == upperLimit;
} }
@Override
public String toString() {
return "(" + lowerLimit + ", " + upperLimit + ")";
}
} }
/** /**

View File

@@ -27,6 +27,7 @@ import android.test.suitebuilder.annotation.SmallTest;
import com.android.documentsui.dirlist.MultiSelectManager.GridModel; import com.android.documentsui.dirlist.MultiSelectManager.GridModel;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List;
import java.util.Set; import java.util.Set;
@SmallTest @SmallTest
@@ -42,6 +43,17 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
private Set<String> lastSelection; private Set<String> lastSelection;
private int viewWidth; private int viewWidth;
// TLDR: Don't call model.{start|resize}Selection; use the local #startSelection and
// #resizeSelection methods instead.
//
// The reason for this is that selection is stateful and involves operations that take the
// current UI state (e.g scrolling) into account. This test maintains its own copy of the
// selection bounds as control data for verifying selections. Keep this data in sync by calling
// #startSelection and
// #resizeSelection.
private Point mSelectionOrigin;
private Point mSelectionPoint;
private void initData(final int numChildren, int numColumns) { private void initData(final int numChildren, int numColumns) {
env = new TestEnvironment(numChildren, numColumns); env = new TestEnvironment(numChildren, numColumns);
adapter = new TestDocumentsAdapter(new ArrayList<String>()) { adapter = new TestDocumentsAdapter(new ArrayList<String>()) {
@@ -76,139 +88,241 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
public void testSelectionLeftOfItems() { public void testSelectionLeftOfItems() {
initData(20, 5); initData(20, 5);
model.startSelection(new Point(0, 10)); startSelection(new Point(0, 10));
model.resizeSelection(new Point(1, 11)); resizeSelection(new Point(1, 11));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testSelectionRightOfItems() { public void testSelectionRightOfItems() {
initData(20, 4); initData(20, 4);
model.startSelection(new Point(viewWidth - 1, 10)); startSelection(new Point(viewWidth - 1, 10));
model.resizeSelection(new Point(viewWidth - 2, 11)); resizeSelection(new Point(viewWidth - 2, 11));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testSelectionAboveItems() { public void testSelectionAboveItems() {
initData(20, 4); initData(20, 4);
model.startSelection(new Point(10, 0)); startSelection(new Point(10, 0));
model.resizeSelection(new Point(11, 1)); resizeSelection(new Point(11, 1));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testSelectionBelowItems() { public void testSelectionBelowItems() {
initData(5, 4); initData(5, 4);
model.startSelection(new Point(10, VIEWPORT_HEIGHT - 1)); startSelection(new Point(10, VIEWPORT_HEIGHT - 1));
model.resizeSelection(new Point(11, VIEWPORT_HEIGHT - 2)); resizeSelection(new Point(11, VIEWPORT_HEIGHT - 2));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testVerticalSelectionBetweenItems() { public void testVerticalSelectionBetweenItems() {
initData(20, 4); initData(20, 4);
model.startSelection(new Point(106, 0)); startSelection(new Point(106, 0));
model.resizeSelection(new Point(107, 200)); resizeSelection(new Point(107, 200));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testHorizontalSelectionBetweenItems() { public void testHorizontalSelectionBetweenItems() {
initData(20, 4); initData(20, 4);
model.startSelection(new Point(0, 105)); startSelection(new Point(0, 105));
model.resizeSelection(new Point(200, 106)); resizeSelection(new Point(200, 106));
assertSelected(); assertNoSelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testGrowingAndShrinkingSelection() { public void testGrowingAndShrinkingSelection() {
initData(20, 4); initData(20, 4);
model.startSelection(new Point(0, 0)); startSelection(new Point(0, 0));
model.resizeSelection(new Point(5, 5));
assertSelected(0); resizeSelection(new Point(5, 5));
model.resizeSelection(new Point(109, 109)); verifySelection();
assertSelected(0);
model.resizeSelection(new Point(110, 109)); resizeSelection(new Point(109, 109));
assertSelected(0, 1); verifySelection();
model.resizeSelection(new Point(110, 110));
assertSelected(0, 1, 4, 5); resizeSelection(new Point(110, 109));
model.resizeSelection(new Point(214, 214)); verifySelection();
assertSelected(0, 1, 4, 5);
model.resizeSelection(new Point(215, 214)); resizeSelection(new Point(110, 110));
assertSelected(0, 1, 2, 4, 5, 6); verifySelection();
model.resizeSelection(new Point(214, 214));
assertSelected(0, 1, 4, 5); resizeSelection(new Point(214, 214));
model.resizeSelection(new Point(110, 110)); verifySelection();
assertSelected(0, 1, 4, 5);
model.resizeSelection(new Point(110, 109)); resizeSelection(new Point(215, 214));
assertSelected(0, 1); verifySelection();
model.resizeSelection(new Point(109, 109));
assertSelected(0); resizeSelection(new Point(214, 214));
model.resizeSelection(new Point(5, 5)); verifySelection();
assertSelected(0);
model.resizeSelection(new Point(0, 0)); resizeSelection(new Point(110, 110));
assertSelected(); verifySelection();
resizeSelection(new Point(110, 109));
verifySelection();
resizeSelection(new Point(109, 109));
verifySelection();
resizeSelection(new Point(5, 5));
verifySelection();
resizeSelection(new Point(0, 0));
verifySelection();
assertEquals(NOT_SET, model.getPositionNearestOrigin()); assertEquals(NOT_SET, model.getPositionNearestOrigin());
} }
public void testSelectionMovingAroundOrigin() { public void testSelectionMovingAroundOrigin() {
initData(16, 4); initData(16, 4);
model.startSelection(new Point(210, 210));
model.resizeSelection(new Point(viewWidth - 1, 0)); startSelection(new Point(210, 210));
assertSelected(2, 3, 6, 7); resizeSelection(new Point(viewWidth - 1, 0));
model.resizeSelection(new Point(0, 0)); verifySelection();
assertSelected(0, 1, 4, 5);
model.resizeSelection(new Point(0, 420)); resizeSelection(new Point(0, 0));
assertSelected(8, 9, 12, 13); verifySelection();
model.resizeSelection(new Point(viewWidth - 1, 420));
assertSelected(10, 11, 14, 15); resizeSelection(new Point(0, 420));
assertEquals(10, model.getPositionNearestOrigin()); verifySelection();
resizeSelection(new Point(viewWidth - 1, 420));
verifySelection();
// This is manually figured and will need to be adjusted if the separator position is
// changed.
assertEquals(7, model.getPositionNearestOrigin());
} }
public void testScrollingBandSelect() { public void testScrollingBandSelect() {
initData(40, 4); initData(40, 4);
model.startSelection(new Point(0, 0));
model.resizeSelection(new Point(100, VIEWPORT_HEIGHT - 1)); startSelection(new Point(0, 0));
assertSelected(0, 4, 8, 12, 16); resizeSelection(new Point(100, VIEWPORT_HEIGHT - 1));
verifySelection();
scroll(CHILD_VIEW_EDGE_PX); scroll(CHILD_VIEW_EDGE_PX);
assertSelected(0, 4, 8, 12, 16, 20); verifySelection();
model.resizeSelection(new Point(200, VIEWPORT_HEIGHT - 1));
assertSelected(0, 1, 4, 5, 8, 9, 12, 13, 16, 17, 20, 21); resizeSelection(new Point(200, VIEWPORT_HEIGHT - 1));
verifySelection();
scroll(CHILD_VIEW_EDGE_PX); scroll(CHILD_VIEW_EDGE_PX);
assertSelected(0, 1, 4, 5, 8, 9, 12, 13, 16, 17, 20, 21, 24, 25); verifySelection();
scroll(-2 * CHILD_VIEW_EDGE_PX); scroll(-2 * CHILD_VIEW_EDGE_PX);
assertSelected(0, 1, 4, 5, 8, 9, 12, 13, 16, 17); verifySelection();
model.resizeSelection(new Point(100, VIEWPORT_HEIGHT - 1));
assertSelected(0, 4, 8, 12, 16); resizeSelection(new Point(100, VIEWPORT_HEIGHT - 1));
verifySelection();
assertEquals(0, model.getPositionNearestOrigin()); assertEquals(0, model.getPositionNearestOrigin());
} }
private void assertSelected(int... selectedPositions) { /** Returns the current selection area as a Rect. */
assertEquals(selectedPositions.length, lastSelection.size()); private Rect getSelectionArea() {
for (int position : selectedPositions) { // Construct a rect from the two selection points.
assertTrue(lastSelection.contains(Integer.toString(position))); Rect selectionArea = new Rect(
mSelectionOrigin.x, mSelectionOrigin.y, mSelectionOrigin.x, mSelectionOrigin.y);
selectionArea.union(mSelectionPoint.x, mSelectionPoint.y);
// Rect intersection tests are exclusive of bounds, while the MSM's selection code is
// inclusive. Expand the rect by 1 pixel in all directions to account for this.
selectionArea.inset(-1, -1);
return selectionArea;
}
/** Asserts that the selection is currently empty. */
private void assertNoSelection() {
assertEquals("Unexpected items " + lastSelection + " in selection " + getSelectionArea(),
0, lastSelection.size());
}
/** Verifies the selection using actual bbox checks. */
private void verifySelection() {
Rect selectionArea = getSelectionArea();
for (TestEnvironment.Item item: env.items) {
if (Rect.intersects(selectionArea, item.rect)) {
assertTrue("Expected item " + item + " was not in selection " + selectionArea,
lastSelection.contains(item.name));
} else {
assertFalse("Unexpected item " + item + " in selection" + selectionArea,
lastSelection.contains(item.name));
}
} }
} }
private void startSelection(Point p) {
model.startSelection(p);
mSelectionOrigin = env.createAbsolutePoint(p);
}
private void resizeSelection(Point p) {
model.resizeSelection(p);
mSelectionPoint = env.createAbsolutePoint(p);
}
private void scroll(int dy) { private void scroll(int dy) {
assertTrue(env.verticalOffset + VIEWPORT_HEIGHT + dy <= env.getTotalHeight()); assertTrue(env.verticalOffset + VIEWPORT_HEIGHT + dy <= env.getTotalHeight());
env.verticalOffset += dy; env.verticalOffset += dy;
// Correct the cached selection point as well.
mSelectionPoint.y += dy;
model.onScrolled(null, 0, dy); model.onScrolled(null, 0, dy);
} }
private static final class TestEnvironment implements MultiSelectManager.SelectionEnvironment { private static final class TestEnvironment implements MultiSelectManager.SelectionEnvironment {
public int horizontalOffset = 0;
public int verticalOffset = 0;
private final int mNumColumns; private final int mNumColumns;
private final int mNumRows; private final int mNumRows;
private final int mNumChildren; private final int mNumChildren;
private final int mSeparatorPosition;
public int horizontalOffset = 0;
public int verticalOffset = 0;
private List<Item> items = new ArrayList<>();
public TestEnvironment(int numChildren, int numColumns) { public TestEnvironment(int numChildren, int numColumns) {
mNumChildren = numChildren; mNumChildren = numChildren;
mNumColumns = numColumns; mNumColumns = numColumns;
mNumRows = (int) Math.ceil((double) numChildren / mNumColumns); mSeparatorPosition = mNumColumns + 1;
mNumRows = setupGrid();
}
private int setupGrid() {
// Split the input set into folders and documents. Do this such that there is a
// partially-populated row in the middle of the grid, to test corner cases in layout
// code.
int y = VIEW_PADDING_PX;
int i = 0;
int numRows = 0;
while (i < mNumChildren) {
int top = y;
int height = CHILD_VIEW_EDGE_PX;
int width = CHILD_VIEW_EDGE_PX;
for (int j = 0; j < mNumColumns && i < mNumChildren; j++) {
int left = VIEW_PADDING_PX + (j * (width + VIEW_PADDING_PX));
items.add(new Item(
Integer.toString(i),
new Rect(
left,
top,
left + width - 1,
top + height - 1)));
// Create a partially populated row at the separator position.
if (++i == mSeparatorPosition) {
break;
}
}
y += height + VIEW_PADDING_PX;
numRows++;
}
return numRows;
} }
private int getTotalHeight() { private int getTotalHeight() {
@@ -227,8 +341,16 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
private int getNumItemsInRow(int index) { private int getNumItemsInRow(int index) {
assertTrue(index >= 0 && index < mNumRows); assertTrue(index >= 0 && index < mNumRows);
if (index == mNumRows - 1 && mNumChildren % mNumColumns != 0) { int mod = mSeparatorPosition % mNumColumns;
return mNumChildren % mNumColumns; if (index == (mSeparatorPosition / mNumColumns)) {
// The row containing the separator may be incomplete
return mod > 0 ? mod : mNumColumns;
}
// Account for the partial separator row in the final row tally.
if (index == mNumRows - 1) {
// The last row may be incomplete
int finalRowCount = (mNumChildren - mod) % mNumColumns;
return finalRowCount > 0 ? finalRowCount : mNumColumns;
} }
return mNumColumns; return mNumColumns;
@@ -257,21 +379,18 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
@Override @Override
public int getAdapterPositionAt(int index) { public int getAdapterPositionAt(int index) {
return index + mNumColumns * (getFirstVisibleRowIndex()); // Account for partial rows by actually tallying up the items in hidden rows.
int hiddenCount = 0;
for (int i = 0; i < getFirstVisibleRowIndex(); i++) {
hiddenCount += getNumItemsInRow(i);
}
return index + hiddenCount;
} }
@Override @Override
public Rect getAbsoluteRectForChildViewAt(int index) { public Rect getAbsoluteRectForChildViewAt(int index) {
int adapterPosition = (getFirstVisibleRowIndex() * mNumColumns) + index; int adapterPosition = getAdapterPositionAt(index);
int rowIndex = adapterPosition / mNumColumns; return items.get(adapterPosition).rect;
int columnIndex = adapterPosition % mNumColumns;
Rect rect = new Rect();
rect.top = VIEW_PADDING_PX + rowIndex * (CHILD_VIEW_EDGE_PX + VIEW_PADDING_PX);
rect.bottom = rect.top + CHILD_VIEW_EDGE_PX - 1;
rect.left = VIEW_PADDING_PX + columnIndex * (CHILD_VIEW_EDGE_PX + VIEW_PADDING_PX);
rect.right = rect.left + CHILD_VIEW_EDGE_PX - 1;
return rect;
} }
@Override @Override
@@ -284,11 +403,6 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
return mNumColumns; return mNumColumns;
} }
@Override
public int getRowCount() {
return mNumRows;
}
@Override @Override
public void showBand(Rect rect) { public void showBand(Rect rect) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
@@ -328,5 +442,19 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase {
public boolean isLayoutItem(int adapterPosition) { public boolean isLayoutItem(int adapterPosition) {
return false; return false;
} }
public static final class Item {
public String name;
public Rect rect;
public Item(String n, Rect r) {
name = n;
rect = r;
}
public String toString() {
return name + ": " + rect;
}
}
} }
} }

View File

@@ -86,11 +86,6 @@ public class TestSelectionEnvironment implements SelectionEnvironment {
return 0; return 0;
} }
@Override
public int getRowCount() {
return 0;
}
@Override @Override
public int getChildCount() { public int getChildCount() {
return 0; return 0;