Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 936a7fc6 authored by Ben Kwa's avatar Ben Kwa
Browse files

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
parent b8a5e081

src/com/android/documentsui/dirlist/DirectoryFragment.java

+2 −4
Original line number Diff line number Diff line
@@ -1003,10 +1003,9 @@ public class DirectoryFragment extends Fragment { 
        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 @@ public class DirectoryFragment extends Fragment { 


        @Override

        public void onModelUpdate(Model model) {

            // TODO(stable-id): Sort model IDs, categorize by dir/file, etc

            mModelIds = Lists.newArrayList(model.getModelIds());

        }

src/com/android/documentsui/dirlist/MultiSelectManager.java

+56 −59
Original line number Diff line number Diff line
@@ -30,6 +30,8 @@ import android.support.v7.widget.GridLayoutManager; 
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.Events.MotionInputEvent; 
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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        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 final class MultiSelectManager implements View.OnKeyListener { 
                    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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        }



        @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
@@ -848,11 +841,6 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            return mAdapter.getModelId(position);

        }



        @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 final class MultiSelectManager implements View.OnKeyListener { 
        public void registerDataObserver(RecyclerView.AdapterDataObserver observer) {

            mAdapter.registerAdapterDataObserver(observer);

        }



        @Override

        public List<String> getModelIds() {

            return mAdapter.getModelIds();

        }

    }



    public interface Callback {
@@ -1179,15 +1172,14 @@ public final class MultiSelectManager implements View.OnKeyListener { 
            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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        // 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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
        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 @@ public final class MultiSelectManager implements View.OnKeyListener { 


        // 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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
         * @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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
         */

        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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
         * @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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
                        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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
                updateSelection(computeBounds());

            } else {

                mSelection.clear();

                mItemNearestOrigin = null;

                mPositionNearestOrigin = NOT_SET;

            }

        }
@@ -1552,11 +1544,11 @@ public final class MultiSelectManager implements View.OnKeyListener { 
                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 @@ public final class MultiSelectManager implements View.OnKeyListener { 
                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));

                    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;

                    }

                }

            }

tests/src/com/android/documentsui/dirlist/MultiSelectManager_GridModelTest.java

+21 −20
Original line number Diff line number Diff line
@@ -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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        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) {
@@ -242,6 +244,11 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
            return childCount;

        }



        @Override

        public int getAdapterPositionAt(int index) {

            return index + mNumColumns * (getFirstVisibleRowIndex());

        }



        @Override

        public Rect getAbsoluteRectForChildViewAt(int index) {

            int adapterPosition = (getFirstVisibleRowIndex() * mNumColumns) + index;
@@ -322,19 +329,13 @@ public class MultiSelectManager_GridModelTest extends AndroidTestCase { 
        }



        @Override

        public String getModelIdAt(int index) {

            int firstVisibleChildIndex = getFirstVisibleRowIndex() * mNumColumns;

            return Integer.toString(firstVisibleChildIndex + index);

        }



        @Override

        public String getModelIdForChildView(View view) {

            throw new UnsupportedOperationException();

        public int getItemCount() {

            return mNumChildren;

        }



        @Override

        public int getItemCount() {

            return mNumChildren;

        public List<String> getModelIds() {

            return null;

        }



        @Override

tests/src/com/android/documentsui/dirlist/MultiSelectManager_SelectionTest.java

+70 −0
Original line number Diff line number Diff line
@@ -20,6 +20,10 @@ import android.test.AndroidTestCase; 
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 @@ public class MultiSelectManager_SelectionTest extends AndroidTestCase { 
        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));

tests/src/com/android/documentsui/dirlist/TestSelectionEnvironment.java

+9 −8
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.view.View; 
import com.android.documentsui.dirlist.MultiSelectManager.SelectionEnvironment;



import java.util.List;

import java.util.Set;



public class TestSelectionEnvironment implements SelectionEnvironment {
@@ -81,6 +82,11 @@ public class TestSelectionEnvironment implements SelectionEnvironment { 
        return null;

    }



    @Override

    public int getAdapterPositionAt(int index) {

        return 0;

    }



    @Override

    public int getAdapterPositionForChildView(View view) {

        return 0;
@@ -116,20 +122,15 @@ public class TestSelectionEnvironment implements SelectionEnvironment { 
    }



    @Override

    public String getModelIdAt(int index) {

        return null;

    public int getItemCount() {

        return mItems.size();

    }



    @Override

    public String getModelIdForChildView(View view) {

    public List<String> getModelIds() {

        return null;

    }



    @Override

    public int getItemCount() {

        return mItems.size();

    }



    @Override

    public void notifyItemChanged(String id, String selectionChangedMarker) {

    }