Add unit tests to GlobalActionsGridLayout.
Refactor GlobalActionsGridLayout, ListGridLayout, and related classes/XML files to clean things up and improve testability.
Test: Automated tests pass. (Hooray, they exist now!)
Fixes: 130808337
Change-Id: I89f1a90b07425a95ce600dd104ed3a4729c2215b
diff --git a/packages/SystemUI/src/com/android/systemui/HardwareUiLayout.java b/packages/SystemUI/src/com/android/systemui/HardwareUiLayout.java
index 822538b..802903d 100644
--- a/packages/SystemUI/src/com/android/systemui/HardwareUiLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/HardwareUiLayout.java
@@ -32,13 +32,10 @@
import android.view.ViewTreeObserver;
import android.widget.LinearLayout;
-import com.android.systemui.globalactions.GlobalActionsDialog;
import com.android.systemui.tuner.TunerService;
import com.android.systemui.tuner.TunerService.Tunable;
import com.android.systemui.util.leak.RotationUtils;
-import java.util.ArrayList;
-
/**
* Layout for placing two containers at a specific physical position on the device, relative to the
* device's hardware, regardless of screen rotation.
@@ -258,24 +255,16 @@
@Override
public void onUpdateList() {
super.onUpdateList();
- ArrayList<GlobalActionsDialog.Action> separatedActions =
- mAdapter.getSeparatedItems();
- ArrayList<GlobalActionsDialog.Action> listActions = mAdapter.getListItems();
for (int i = 0; i < mAdapter.getCount(); i++) {
- Object action = mAdapter.getItem(i);
- int separatedIndex = separatedActions.indexOf(action);
ViewGroup parent;
- if (separatedIndex != -1) {
+ boolean separated = mAdapter.shouldBeSeparated(i);
+ if (separated) {
parent = getSeparatedView();
} else {
- int listIndex = listActions.indexOf(action);
parent = getListView();
}
View v = mAdapter.getView(i, null, parent);
- final int pos = i;
- v.setOnClickListener(view -> mAdapter.onClickItem(pos));
- v.setOnLongClickListener(view -> mAdapter.onLongClickItem(pos));
parent.addView(v);
}
}
@@ -421,7 +410,9 @@
@Override
protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
super.onLayout(changed, left, top, right, bottom);
+
post(() -> updatePosition());
+
}
private void animateChild(int oldHeight, int newHeight) {
diff --git a/packages/SystemUI/src/com/android/systemui/MultiListLayout.java b/packages/SystemUI/src/com/android/systemui/MultiListLayout.java
index a30b681..f8287a4 100644
--- a/packages/SystemUI/src/com/android/systemui/MultiListLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/MultiListLayout.java
@@ -26,16 +26,12 @@
import com.android.systemui.util.leak.RotationUtils;
-import java.util.ArrayList;
-
/**
* Layout class representing the Global Actions menu which appears when the power button is held.
*/
public abstract class MultiListLayout extends LinearLayout {
protected boolean mHasOutsideTouch;
protected MultiListAdapter mAdapter;
- protected boolean mSnapToEdge;
-
protected int mRotation;
protected RotationListener mRotationListener;
@@ -51,7 +47,7 @@
/**
* Removes all child items from the separated and list views, if they exist.
*/
- public abstract void removeAllItems();
+ protected abstract void removeAllItems();
/**
* Sets the divided view, which may have a differently-colored background.
@@ -70,13 +66,6 @@
}
/**
- * Sets whether the GlobalActions view should snap to the edge of the screen.
- */
- public void setSnapToEdge(boolean snap) {
- mSnapToEdge = snap;
- }
-
- /**
* Sets the adapter used to inflate items.
*/
public void setAdapter(MultiListAdapter adapter) {
@@ -122,6 +111,7 @@
}
protected void onUpdateList() {
+ removeAllItems();
setSeparatedViewVisibility(mAdapter.hasSeparatedItems());
}
@@ -163,16 +153,14 @@
*/
public abstract static class MultiListAdapter extends BaseAdapter {
/**
- * Creates an ArrayList of items which should be rendered in the separated view.
- * @param useSeparatedView is true if the separated view will be used, false otherwise.
+ * Counts the number of items to be rendered in the separated view.
*/
- public abstract ArrayList getSeparatedItems();
+ public abstract int countSeparatedItems();
/**
- * Creates an ArrayList of items which should be rendered in the list view.
- * @param useSeparatedView True if the separated view will be used, false otherwise.
+ * Counts the number of items be rendered in the list view.
*/
- public abstract ArrayList getListItems();
+ public abstract int countListItems();
/**
* Callback to run when an individual item is clicked or pressed.
@@ -192,7 +180,13 @@
* or not to hide the separated list from view.
*/
public boolean hasSeparatedItems() {
- return getSeparatedItems().size() > 0;
+ return countSeparatedItems() > 0;
}
+
+ /**
+ * Determines whether the item at the given index should be rendered in the separarted view.
+ * @param position The index of the item.
+ */
+ public abstract boolean shouldBeSeparated(int position);
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
index 411536c..9b69dc5 100644
--- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
+++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
@@ -919,73 +919,59 @@
* via {@link com.android.systemui.globalactions.GlobalActionsDialog#mDeviceProvisioned}.
*/
public class MyAdapter extends MultiListAdapter {
- @Override
- public int getCount() {
+ private int countItems(boolean separated) {
int count = 0;
for (int i = 0; i < mItems.size(); i++) {
final Action action = mItems.get(i);
- if (mKeyguardShowing && !action.showDuringKeyguard()) {
- continue;
+ if (shouldBeShown(action) && action.shouldBeSeparated() == separated) {
+ count++;
}
- if (!mDeviceProvisioned && !action.showBeforeProvisioning()) {
- continue;
- }
- count++;
}
return count;
}
+ private boolean shouldBeShown(Action action) {
+ if (mKeyguardShowing && !action.showDuringKeyguard()) {
+ return false;
+ }
+ if (!mDeviceProvisioned && !action.showBeforeProvisioning()) {
+ return false;
+ }
+ return true;
+ }
+
+ @Override
+ public int countSeparatedItems() {
+ return countItems(true);
+ }
+
+ @Override
+ public int countListItems() {
+ return countItems(false);
+ }
+
+ @Override
+ public int getCount() {
+ return countSeparatedItems() + countListItems();
+ }
+
@Override
public boolean isEnabled(int position) {
return getItem(position).isEnabled();
}
@Override
- public ArrayList<Action> getSeparatedItems() {
- ArrayList<Action> separatedActions = new ArrayList<Action>();
- if (!shouldUseSeparatedView()) {
- return separatedActions;
- }
- for (int i = 0; i < mItems.size(); i++) {
- final Action action = mItems.get(i);
- if (action.shouldBeSeparated()) {
- separatedActions.add(action);
- }
- }
- return separatedActions;
- }
-
- @Override
- public ArrayList<Action> getListItems() {
- if (!shouldUseSeparatedView()) {
- return new ArrayList<Action>(mItems);
- }
- ArrayList<Action> listActions = new ArrayList<Action>();
- for (int i = 0; i < mItems.size(); i++) {
- final Action action = mItems.get(i);
- if (!action.shouldBeSeparated()) {
- listActions.add(action);
- }
- }
- return listActions;
- }
-
- @Override
public boolean areAllItemsEnabled() {
return false;
}
@Override
public Action getItem(int position) {
-
int filteredPos = 0;
for (int i = 0; i < mItems.size(); i++) {
final Action action = mItems.get(i);
- if (mKeyguardShowing && !action.showDuringKeyguard()) {
- continue;
- }
- if (!mDeviceProvisioned && !action.showBeforeProvisioning()) {
+ if (!shouldBeShown(action)) {
continue;
}
if (filteredPos == position) {
@@ -1010,10 +996,8 @@
public View getView(int position, View convertView, ViewGroup parent) {
Action action = getItem(position);
View view = action.create(mContext, convertView, parent, LayoutInflater.from(mContext));
- // Everything but screenshot, the last item, gets white background.
- if (position == getCount() - 1) {
- MultiListLayout.get(parent).setDivisionView(view);
- }
+ view.setOnClickListener(v -> onClickItem(position));
+ view.setOnLongClickListener(v -> onLongClickItem(position));
return view;
}
@@ -1035,6 +1019,11 @@
}
item.onPress();
}
+
+ @Override
+ public boolean shouldBeSeparated(int position) {
+ return getItem(position).shouldBeSeparated();
+ }
}
// note: the scheme below made more sense when we were planning on having
@@ -1589,7 +1578,6 @@
com.android.systemui.R.drawable.global_action_panel_scrim);
mScrimAlpha = 1f;
}
- mGlobalActionsLayout.setSnapToEdge(true);
getWindow().setBackgroundDrawable(mBackgroundDrawable);
}
diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsGridLayout.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsGridLayout.java
index 669348e..554ed73 100644
--- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsGridLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsGridLayout.java
@@ -21,19 +21,16 @@
import static com.android.systemui.util.leak.RotationUtils.ROTATION_SEASCAPE;
import android.content.Context;
-import android.text.TextUtils;
import android.util.AttributeSet;
-import android.view.Gravity;
+import android.util.Log;
import android.view.View;
import android.view.ViewGroup;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.HardwareBgDrawable;
import com.android.systemui.MultiListLayout;
import com.android.systemui.util.leak.RotationUtils;
-import java.util.ArrayList;
-import java.util.Locale;
-
/**
* Grid-based implementation of the button layout created by the global actions dialog.
*/
@@ -69,67 +66,65 @@
}
}
- /**
- * Sets the number of items expected to be rendered in the list container. This allows the
- * layout to correctly determine which parent containers will be used for items before they have
- * beenadded to the layout.
- * @param count The number of items expected.
- */
- public void setExpectedListItemCount(int count) {
- getListView().setExpectedCount(count);
+ @VisibleForTesting
+ protected int getCurrentRotation() {
+ return RotationUtils.getRotation(mContext);
+ }
+
+ @VisibleForTesting
+ protected void setupListView(ListGridLayout listView, int itemCount) {
+ listView.setExpectedCount(itemCount);
+ listView.setReverseSublists(shouldReverseSublists());
+ listView.setReverseItems(shouldReverseListItems());
+ listView.setSwapRowsAndColumns(shouldSwapRowsAndColumns());
}
@Override
public void onUpdateList() {
super.onUpdateList();
- ArrayList<GlobalActionsDialog.Action> separatedActions =
- mAdapter.getSeparatedItems();
- ArrayList<GlobalActionsDialog.Action> listActions = mAdapter.getListItems();
- setExpectedListItemCount(listActions.size());
- int rotation = RotationUtils.getRotation(mContext);
- boolean reverse = false; // should we add items to parents in the reverse order?
- if (rotation == ROTATION_NONE
- || rotation == ROTATION_SEASCAPE) {
- reverse = !reverse; // if we're in portrait or seascape, reverse items
- }
- if (TextUtils.getLayoutDirectionFromLocale(Locale.getDefault())
- == View.LAYOUT_DIRECTION_RTL) {
- reverse = !reverse; // if we're in an RTL language, reverse items (again)
- }
+ ViewGroup separatedView = getSeparatedView();
+ ListGridLayout listView = getListView();
+ setupListView(listView, mAdapter.countListItems());
for (int i = 0; i < mAdapter.getCount(); i++) {
- Object action = mAdapter.getItem(i);
- int separatedIndex = separatedActions.indexOf(action);
- ViewGroup parent;
- if (separatedIndex != -1) {
- parent = getParentView(true, separatedIndex, rotation);
+ // generate the view item
+ View v;
+ boolean separated = mAdapter.shouldBeSeparated(i);
+ if (separated) {
+ v = mAdapter.getView(i, null, separatedView);
} else {
- int listIndex = listActions.indexOf(action);
- parent = getParentView(false, listIndex, rotation);
+ v = mAdapter.getView(i, null, listView);
}
- View v = mAdapter.getView(i, null, parent);
- final int pos = i;
- v.setOnClickListener(view -> mAdapter.onClickItem(pos));
- v.setOnLongClickListener(view -> mAdapter.onLongClickItem(pos));
- if (reverse) {
- parent.addView(v, 0); // reverse order of items
+ Log.d("GlobalActionsGridLayout", "View: " + v);
+
+ if (separated) {
+ separatedView.addView(v);
} else {
- parent.addView(v);
+ listView.addItem(v);
}
- parent.setVisibility(View.VISIBLE);
}
- updateSnapPosition();
- updateSeparatedButtonSize();
+ updateSeparatedItemSize();
}
- private void updateSeparatedButtonSize() {
+ /**
+ * If the separated view contains only one item, expand the bounds of that item to take up the
+ * entire view, so that the whole thing is touch-able.
+ */
+ private void updateSeparatedItemSize() {
ViewGroup separated = getSeparatedView();
+ if (separated.getChildCount() == 0) {
+ return;
+ }
+ View firstChild = separated.getChildAt(0);
+ ViewGroup.LayoutParams childParams = firstChild.getLayoutParams();
+
if (separated.getChildCount() == 1) {
- View onlyChild = separated.getChildAt(0);
- ViewGroup.LayoutParams childParams = onlyChild.getLayoutParams();
childParams.width = ViewGroup.LayoutParams.MATCH_PARENT;
childParams.height = ViewGroup.LayoutParams.MATCH_PARENT;
+ } else {
+ childParams.width = ViewGroup.LayoutParams.WRAP_CONTENT;
+ childParams.height = ViewGroup.LayoutParams.WRAP_CONTENT;
}
}
@@ -138,19 +133,6 @@
return findViewById(com.android.systemui.R.id.separated_button);
}
- private void updateSnapPosition() {
- if (mSnapToEdge) {
- setPadding(0, 0, 0, 0);
- if (mRotation == ROTATION_LANDSCAPE) {
- setGravity(Gravity.RIGHT);
- } else if (mRotation == ROTATION_SEASCAPE) {
- setGravity(Gravity.LEFT);
- } else {
- setGravity(Gravity.BOTTOM);
- }
- }
- }
-
@Override
protected ListGridLayout getListView() {
return findViewById(android.R.id.list);
@@ -168,19 +150,47 @@
}
}
- public ViewGroup getParentView(boolean separated, int index, int rotation) {
- if (separated) {
- return getSeparatedView();
- } else {
- switch (rotation) {
- case ROTATION_LANDSCAPE:
- return getListView().getParentView(index, false, true);
- case ROTATION_SEASCAPE:
- return getListView().getParentView(index, true, true);
- default:
- return getListView().getParentView(index, false, false);
- }
+ /**
+ * Determines whether the ListGridLayout should fill sublists in the reverse order.
+ * Used to account for sublist ordering changing between landscape and seascape views.
+ */
+ @VisibleForTesting
+ protected boolean shouldReverseSublists() {
+ if (getCurrentRotation() == ROTATION_SEASCAPE) {
+ return true;
}
+ return false;
+ }
+
+ /**
+ * Determines whether the ListGridLayout should fill rows first instead of columns.
+ * Used to account for vertical/horizontal changes due to landscape or seascape rotations.
+ */
+ @VisibleForTesting
+ protected boolean shouldSwapRowsAndColumns() {
+ if (getCurrentRotation() == ROTATION_NONE) {
+ return false;
+ }
+ return true;
+ }
+
+ /**
+ * Determines whether the ListGridLayout should reverse the ordering of items within sublists.
+ * Used for RTL languages to ensure that items appear in the same positions, without having to
+ * override layoutDirection, which breaks Talkback ordering.
+ */
+ @VisibleForTesting
+ protected boolean shouldReverseListItems() {
+ int rotation = getCurrentRotation();
+ boolean reverse = false; // should we add items to parents in the reverse order?
+ if (rotation == ROTATION_NONE
+ || rotation == ROTATION_SEASCAPE) {
+ reverse = !reverse; // if we're in portrait or seascape, reverse items
+ }
+ if (getLayoutDirection() == View.LAYOUT_DIRECTION_RTL) {
+ reverse = !reverse; // if we're in an RTL language, reverse items (again)
+ }
+ return reverse;
}
/**
@@ -191,7 +201,7 @@
// do nothing
}
- private float getAnimationDistance() {
+ protected float getAnimationDistance() {
int rows = getListView().getRowCount();
float gridItemSize = getContext().getResources().getDimension(
com.android.systemui.R.dimen.global_actions_grid_item_height);
@@ -200,7 +210,7 @@
@Override
public float getAnimationOffsetX() {
- switch (RotationUtils.getRotation(getContext())) {
+ switch (getCurrentRotation()) {
case ROTATION_LANDSCAPE:
return getAnimationDistance();
case ROTATION_SEASCAPE:
@@ -212,7 +222,7 @@
@Override
public float getAnimationOffsetY() {
- if (RotationUtils.getRotation(mContext) == ROTATION_NONE) {
+ if (getCurrentRotation() == ROTATION_NONE) {
return getAnimationDistance();
}
return 0;
diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/ListGridLayout.java b/packages/SystemUI/src/com/android/systemui/globalactions/ListGridLayout.java
index 6bc975a..8c93b13 100644
--- a/packages/SystemUI/src/com/android/systemui/globalactions/ListGridLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/globalactions/ListGridLayout.java
@@ -22,6 +22,8 @@
import android.view.ViewGroup;
import android.widget.LinearLayout;
+import com.android.internal.annotations.VisibleForTesting;
+
/**
* Layout which uses nested LinearLayouts to create a grid with the following behavior:
*
@@ -40,6 +42,10 @@
public class ListGridLayout extends LinearLayout {
private static final String TAG = "ListGridLayout";
private int mExpectedCount;
+ private int mCurrentCount = 0;
+ private boolean mSwapRowsAndColumns;
+ private boolean mReverseSublists;
+ private boolean mReverseItems;
// number of rows and columns to use for different numbers of items
private final int[][] mConfigs = {
@@ -61,16 +67,60 @@
}
/**
+ * Sets whether this grid should prioritize filling rows or columns first.
+ */
+ public void setSwapRowsAndColumns(boolean swap) {
+ mSwapRowsAndColumns = swap;
+ }
+
+ /**
+ * Sets whether this grid should fill sublists in reverse order.
+ */
+ public void setReverseSublists(boolean reverse) {
+ mReverseSublists = reverse;
+ }
+
+ /**
+ * Sets whether this grid should add items to sublists in reverse order.
+ * @param reverse
+ */
+ public void setReverseItems(boolean reverse) {
+ mReverseItems = reverse;
+ }
+
+ /**
* Remove all items from this grid.
*/
public void removeAllItems() {
for (int i = 0; i < getChildCount(); i++) {
- ViewGroup subList = (ViewGroup) getChildAt(i);
+ ViewGroup subList = getSublist(i);
if (subList != null) {
subList.removeAllViews();
subList.setVisibility(View.GONE);
}
}
+ mCurrentCount = 0;
+ }
+
+ /**
+ * Adds a view item to this grid, placing it in the correct sublist and ensuring that the
+ * sublist is visible.
+ *
+ * This function is stateful, since it tracks how many items have been added thus far, to
+ * determine which sublist they should be added to. To ensure that this works correctly, call
+ * removeAllItems() instead of removing views individually with removeView() to ensure that the
+ * counter gets reset correctly.
+ * @param item
+ */
+ public void addItem(View item) {
+ ViewGroup parent = getParentView(mCurrentCount, mReverseSublists, mSwapRowsAndColumns);
+ if (mReverseItems) {
+ parent.addView(item, 0);
+ } else {
+ parent.addView(item);
+ }
+ parent.setVisibility(View.VISIBLE);
+ mCurrentCount++;
}
/**
@@ -83,13 +133,20 @@
* true will cause rows to fill first, adding one item to each column.
* @return
*/
- public ViewGroup getParentView(int index, boolean reverseSublists, boolean swapRowsAndColumns) {
+ @VisibleForTesting
+ protected ViewGroup getParentView(int index, boolean reverseSublists,
+ boolean swapRowsAndColumns) {
if (getRowCount() == 0 || index < 0) {
return null;
}
int targetIndex = Math.min(index, getMaxElementCount() - 1);
int row = getParentViewIndex(targetIndex, reverseSublists, swapRowsAndColumns);
- return (ViewGroup) getChildAt(row);
+ return getSublist(row);
+ }
+
+ @VisibleForTesting
+ protected ViewGroup getSublist(int index) {
+ return (ViewGroup) getChildAt(index);
}
private int reverseSublistIndex(int index) {