Handle shift+arrow correctly in single select mode.

Bug: 25603626
Change-Id: I2f71152b303ac218ecec59e8200acf8a716ea0ee

4 files changed, 178 insertions, 43 deletions

diff --git a/packages/DocumentsUI/src/com/android/documentsui/Events.java b/packages/DocumentsUI/src/com/android/documentsui/Events.java
index 49dae3d0402f..1b5b60de6f24 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/Events.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/Events.java
@@ -117,6 +117,8 @@ public final class Events {
         public MotionInputEvent(MotionEvent event, RecyclerView view) {
             mEvent = event;
             mView = view;
+
+            // Consider determining position lazily as an optimization.
             View child = mView.findChildViewUnder(mEvent.getX(), mEvent.getY());
             mPosition = (child != null)
                     ? mView.getChildAdapterPosition(child)
diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
index 9eafcc311f0c..65e1a2820442 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/MultiSelectManager.java
@@ -41,10 +41,9 @@ import android.view.KeyEvent;
 import android.view.MotionEvent;
 import android.view.View;
 
-import com.android.documentsui.Events;
-import com.android.documentsui.R;
 import com.android.documentsui.Events.InputEvent;
 import com.android.documentsui.Events.MotionInputEvent;
+import com.android.documentsui.R;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -88,9 +87,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
      * @param mode Selection mode
      */
     public MultiSelectManager(final RecyclerView recyclerView, int mode) {
-        this(recyclerView.getAdapter(), mode);
-
-        mEnvironment = new RuntimeSelectionEnvironment(recyclerView);
+        this(recyclerView.getAdapter(), new RuntimeSelectionEnvironment(recyclerView), mode);
 
         if (mode == MODE_MULTIPLE) {
             mBandManager = new BandController();
@@ -137,16 +134,15 @@ public final class MultiSelectManager implements View.OnKeyListener {
 
     /**
      * Constructs a new instance with {@code adapter} and {@code helper}.
+     * @param runtimeSelectionEnvironment
      * @hide
      */
     @VisibleForTesting
-    MultiSelectManager(Adapter<?> adapter, int mode) {
-        checkNotNull(adapter, "'adapter' cannot be null.");
-
+    MultiSelectManager(Adapter<?> adapter, SelectionEnvironment environment, int mode) {
+        mAdapter = checkNotNull(adapter, "'adapter' cannot be null.");
+        mEnvironment = checkNotNull(environment, "'environment' cannot be null.");
         mSingleSelect = mode == MODE_SINGLE;
 
-        mAdapter = adapter;
-
         mAdapter.registerAdapterDataObserver(
                 new AdapterDataObserver() {
 
@@ -880,7 +876,7 @@ public final class MultiSelectManager implements View.OnKeyListener {
         void focusItem(int position);
     }
 
-    /** RvFacade implementation backed by good ol' RecyclerView. */
+    /** Recycler view facade implementation backed by good ol' RecyclerView. */
     private static final class RuntimeSelectionEnvironment implements SelectionEnvironment {
 
         private final RecyclerView mView;
@@ -1960,11 +1956,50 @@ public final class MultiSelectManager implements View.OnKeyListener {
             return false;
         }
 
-        int target = RecyclerView.NO_POSITION;
+        // Here we unpack information from the event and pass it to an more
+        // easily tested method....basically eliminating the need to synthesize
+        // events and views and so on in our tests.
+        int position = findTargetPosition(view, keyCode);
+        if (position == RecyclerView.NO_POSITION) {
+            // If there is no valid navigation target, don't handle the keypress.
+            return false;
+        }
+
+        return attemptChangePosition(position, event.isShiftPressed());
+    }
+
+    @VisibleForTesting
+    boolean attemptChangePosition(int targetPosition, boolean isShiftPressed) {
+        // Focus the new file.
+        mEnvironment.focusItem(targetPosition);
+
+        if (isShiftPressed) {
+            if (!hasSelection()) {
+                // If there is no selection, start a selection when the user presses shift-arrow.
+                toggleSelection(targetPosition);
+            } else if (!mSingleSelect) {
+                mRanger.snapSelection(targetPosition);
+                notifySelectionChanged();
+            } else {
+                // We're in single select and have an existing selection.
+                // Our best guess as to what the user would expect is to advance the selection.
+                clearSelection();
+                toggleSelection(targetPosition);
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * Returns the adapter position that the key combo is targeted at.
+     */
+    private int findTargetPosition(View view, int keyCode) {
+        int position = RecyclerView.NO_POSITION;
         if (keyCode == KeyEvent.KEYCODE_MOVE_HOME) {
-            target = 0;
+            position = 0;
         } else if (keyCode == KeyEvent.KEYCODE_MOVE_END) {
-            target = mAdapter.getItemCount() - 1;
+            position = mAdapter.getItemCount() - 1;
         } else {
             // Find a navigation target based on the arrow key that the user pressed.  Ignore
             // navigation targets that aren't items in the recycler view.
@@ -1988,30 +2023,10 @@ public final class MultiSelectManager implements View.OnKeyListener {
                 // TargetView can be null, for example, if the user pressed <down> at the bottom of
                 // the list.
                 if (targetView != null) {
-                    target = mEnvironment.getAdapterPositionForChildView(targetView);
+                    position = mEnvironment.getAdapterPositionForChildView(targetView);
                 }
             }
         }
-
-        if (target == RecyclerView.NO_POSITION) {
-            // If there is no valid navigation target, don't handle the keypress.
-            return false;
-        }
-
-        // Focus the new file.
-        mEnvironment.focusItem(target);
-
-        if (event.isShiftPressed()) {
-            if (!hasSelection()) {
-                // If there is no selection, start a selection when the user presses shift-arrow.
-                toggleSelection(mEnvironment.getAdapterPositionForChildView(view));
-            }
-
-            mRanger.snapSelection(target);
-            notifySelectionChanged();
-        }
-
-        return true;
+        return position;
     }
-
 }
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManagerTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManagerTest.java
index 24f5c9e21f7a..d1ce56457a87 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManagerTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/MultiSelectManagerTest.java
@@ -23,7 +23,6 @@ import android.view.View;
 import android.view.ViewGroup;
 
 import com.android.documentsui.TestInputEvent;
-import com.android.documentsui.dirlist.MultiSelectManager;
 import com.android.documentsui.dirlist.MultiSelectManager.Selection;
 
 import org.mockito.Mockito;
@@ -49,11 +48,13 @@ public class MultiSelectManagerTest extends AndroidTestCase {
     private MultiSelectManager mManager;
     private TestAdapter mAdapter;
     private TestCallback mCallback;
+    private TestSelectionEnvironment mEnv;
 
     public void setUp() throws Exception {
         mAdapter = new TestAdapter(items);
         mCallback = new TestCallback();
-        mManager = new MultiSelectManager(mAdapter, MultiSelectManager.MODE_MULTIPLE);
+        mEnv = new TestSelectionEnvironment();
+        mManager = new MultiSelectManager(mAdapter, mEnv, MultiSelectManager.MODE_MULTIPLE);
         mManager.addCallback(mCallback);
     }
 
@@ -171,7 +172,7 @@ public class MultiSelectManagerTest extends AndroidTestCase {
     }
 
     public void testSingleSelectMode() {
-        mManager = new MultiSelectManager(mAdapter, MultiSelectManager.MODE_SINGLE);
+        mManager = new MultiSelectManager(mAdapter, mEnv, MultiSelectManager.MODE_SINGLE);
         mManager.addCallback(mCallback);
         longPress(20);
         tap(13);
@@ -179,13 +180,21 @@ public class MultiSelectManagerTest extends AndroidTestCase {
     }
 
     public void testSingleSelectMode_ShiftTap() {
-        mManager = new MultiSelectManager(mAdapter, MultiSelectManager.MODE_SINGLE);
+        mManager = new MultiSelectManager(mAdapter, mEnv, MultiSelectManager.MODE_SINGLE);
         mManager.addCallback(mCallback);
         longPress(13);
         shiftTap(20);
         assertSelection(20);
     }
 
+    public void testSingleSelectMode_ShiftDoesNotExtendSelection() {
+        mManager = new MultiSelectManager(mAdapter, mEnv, MultiSelectManager.MODE_SINGLE);
+        mManager.addCallback(mCallback);
+        longPress(20);
+        keyToPosition(22, true);
+        assertSelection(22);
+    }
+
     public void testProvisionalSelection() {
         Selection s = mManager.getSelection();
         assertSelection();
@@ -235,6 +244,10 @@ public class MultiSelectManagerTest extends AndroidTestCase {
         mManager.onSingleTapUp(TestInputEvent.shiftClick(position));
     }
 
+    private void keyToPosition(int position, boolean shift) {
+        mManager.attemptChangePosition(position, shift);
+    }
+
     private void assertSelected(int... expected) {
         for (int i = 0; i < expected.length; i++) {
             Selection selection = mManager.getSelection();
@@ -290,11 +303,8 @@ public class MultiSelectManagerTest extends AndroidTestCase {
 
     private static final class TestHolder extends RecyclerView.ViewHolder {
         // each data item is just a string in this case
-        public View view;
-        public String string;
         public TestHolder(View view) {
             super(view);
-            this.view = view;
         }
     }
 
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java
new file mode 100644
index 000000000000..b4324a825b4a
--- /dev/null
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.documentsui.dirlist;
+
+import android.graphics.Point;
+import android.graphics.Rect;
+import android.support.v7.widget.RecyclerView.OnScrollListener;
+import android.view.View;
+
+import com.android.documentsui.dirlist.MultiSelectManager.SelectionEnvironment;
+
+public class TestSelectionEnvironment implements SelectionEnvironment {
+
+    @Override
+    public void showBand(Rect rect) {
+    }
+
+    @Override
+    public void hideBand() {
+    }
+
+    @Override
+    public void addOnScrollListener(OnScrollListener listener) {
+    }
+
+    @Override
+    public void removeOnScrollListener(OnScrollListener listener) {
+    }
+
+    @Override
+    public void scrollBy(int dy) {
+    }
+
+    @Override
+    public int getHeight() {
+        return 0;
+    }
+
+    @Override
+    public void invalidateView() {
+    }
+
+    @Override
+    public void runAtNextFrame(Runnable r) {
+    }
+
+    @Override
+    public void removeCallback(Runnable r) {
+    }
+
+    @Override
+    public Point createAbsolutePoint(Point relativePoint) {
+        return null;
+    }
+
+    @Override
+    public Rect getAbsoluteRectForChildViewAt(int index) {
+        return null;
+    }
+
+    @Override
+    public int getAdapterPositionAt(int index) {
+        return 0;
+    }
+
+    @Override
+    public int getAdapterPositionForChildView(View view) {
+        return 0;
+    }
+
+    @Override
+    public int getColumnCount() {
+        return 0;
+    }
+
+    @Override
+    public int getRowCount() {
+        return 0;
+    }
+
+    @Override
+    public int getChildCount() {
+        return 0;
+    }
+
+    @Override
+    public int getVisibleChildCount() {
+        return 0;
+    }
+
+    @Override
+    public void focusItem(int position) {
+    }
+}