FloatingToolbar: Rules for ordering menu items.
- The assist menu item should always appear first
- Only the assist menu item should have an icon + text label
The description above enforces the expected UX without breaking
existing apps. Previously, apps couldn't have icons + text for a
menu item (we only rendered text for such items). If we don't
enforce that the assist menu item (android.R.id.textAssist) appears
first, then this item could potentially be rendered in a different
position for apps that specify an order number lower than that
specified for the assist menu item.
Test: Included in this cl.
Bug: 35989798
Change-Id: I56c71450c850e1d8cd80d54d4e17a6390823485f
diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java
index 4fb7b19..0d3baa8 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -146,7 +146,7 @@
private static final String UNDO_OWNER_TAG = "Editor";
// Ordering constants used to place the Action Mode or context menu items in their menu.
- private static final int MENU_ITEM_ORDER_ASSIST = 1;
+ private static final int MENU_ITEM_ORDER_ASSIST = 0;
private static final int MENU_ITEM_ORDER_UNDO = 2;
private static final int MENU_ITEM_ORDER_REDO = 3;
private static final int MENU_ITEM_ORDER_CUT = 4;
@@ -156,8 +156,8 @@
private static final int MENU_ITEM_ORDER_PASTE_AS_PLAIN_TEXT = 8;
private static final int MENU_ITEM_ORDER_SELECT_ALL = 9;
private static final int MENU_ITEM_ORDER_REPLACE = 10;
- private static final int MENU_ITEM_ORDER_PROCESS_TEXT_INTENT_ACTIONS_START = 11;
- private static final int MENU_ITEM_ORDER_AUTOFILL = 12;
+ private static final int MENU_ITEM_ORDER_AUTOFILL = 11;
+ private static final int MENU_ITEM_ORDER_PROCESS_TEXT_INTENT_ACTIONS_START = 100;
// Each Editor manages its own undo stack.
private final UndoManager mUndoManager = new UndoManager();
@@ -6322,9 +6322,10 @@
* Adds "PROCESS_TEXT" menu items to the specified menu.
*/
public void onInitializeMenu(Menu menu) {
- int i = 0;
+ final int size = mSupportedActivities.size();
loadSupportedActivities();
- for (ResolveInfo resolveInfo : mSupportedActivities) {
+ for (int i = 0; i < size; i++) {
+ final ResolveInfo resolveInfo = mSupportedActivities.get(i);
menu.add(Menu.NONE, Menu.NONE,
Editor.MENU_ITEM_ORDER_PROCESS_TEXT_INTENT_ACTIONS_START + i++,
getLabel(resolveInfo))
diff --git a/core/java/com/android/internal/widget/FloatingToolbar.java b/core/java/com/android/internal/widget/FloatingToolbar.java
index 818cc2c..8c71cf7 100644
--- a/core/java/com/android/internal/widget/FloatingToolbar.java
+++ b/core/java/com/android/internal/widget/FloatingToolbar.java
@@ -233,6 +233,7 @@
private void doShow() {
List<MenuItem> menuItems = getVisibleAndEnabledMenuItems(mMenu);
+ tidy(menuItems);
if (!isCurrentlyShowing(menuItems) || mWidthChanged) {
mPopup.dismiss();
mPopup.layoutMenuItems(menuItems, mMenuItemClickListener, mSuggestedWidth);
@@ -274,6 +275,36 @@
return menuItems;
}
+ /**
+ * Update the list of menu items to conform to certain requirements.
+ */
+ private void tidy(List<MenuItem> menuItems) {
+ int assistItemIndex = -1;
+ Drawable assistItemDrawable = null;
+
+ final int size = menuItems.size();
+ for (int i = 0; i < size; i++) {
+ final MenuItem menuItem = menuItems.get(i);
+
+ if (menuItem.getItemId() == android.R.id.textAssist) {
+ assistItemIndex = i;
+ assistItemDrawable = menuItem.getIcon();
+ }
+
+ // Remove icons for all menu items with text.
+ if (!TextUtils.isEmpty(menuItem.getTitle())) {
+ menuItem.setIcon(null);
+ }
+ }
+ if (assistItemIndex > -1) {
+ final MenuItem assistMenuItem = menuItems.remove(assistItemIndex);
+ // Ensure the assist menu item preserves its icon.
+ assistMenuItem.setIcon(assistItemDrawable);
+ // Ensure the assist menu item is always the first item.
+ menuItems.add(0, assistMenuItem);
+ }
+ }
+
private List<Object> getShowingMenuItemsReferences(List<MenuItem> menuItems) {
List<Object> references = new ArrayList<Object>();
for (MenuItem menuItem : menuItems) {
diff --git a/core/tests/coretests/src/android/widget/TextViewActivityTest.java b/core/tests/coretests/src/android/widget/TextViewActivityTest.java
index 3029134..2203b6a 100644
--- a/core/tests/coretests/src/android/widget/TextViewActivityTest.java
+++ b/core/tests/coretests/src/android/widget/TextViewActivityTest.java
@@ -28,6 +28,7 @@
import static android.widget.espresso.TextViewActions.longPressOnTextAtIndex;
import static android.widget.espresso.TextViewAssertions.hasInsertionPointerAtIndex;
import static android.widget.espresso.TextViewAssertions.hasSelection;
+import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarItemIndex;
import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarIsDisplayed;
import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarIsNotDisplayed;
import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarContainsItem;
@@ -46,6 +47,11 @@
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.is;
+import android.view.ActionMode;
+import android.view.Menu;
+import android.view.MenuItem;
+import android.view.textclassifier.TextClassificationManager;
+import android.view.textclassifier.TextClassifier;
import android.widget.espresso.CustomViewActions.RelativeCoordinatesProvider;
import android.support.test.espresso.action.EspressoKey;
@@ -71,7 +77,8 @@
@Override
public void setUp() throws Exception {
super.setUp();
- getActivity();
+ getActivity().getSystemService(TextClassificationManager.class)
+ .setTextClassifier(TextClassifier.NO_OP);
}
public void testTypedTextIsOnScreen() throws Exception {
@@ -676,4 +683,38 @@
// hasTransientState should return false when selection is created by API.
assertFalse(textView.hasTransientState());
}
+
+ public void testAssistItemIsAtIndexZero() throws Exception {
+ getActivity().getSystemService(TextClassificationManager.class).setTextClassifier(null);
+ final TextView textView = (TextView) getActivity().findViewById(R.id.textview);
+ textView.post(() -> textView.setCustomSelectionActionModeCallback(
+ new ActionMode.Callback() {
+ @Override
+ public boolean onCreateActionMode(ActionMode actionMode, Menu menu) {
+ // Create another item at order position 0 to confirm that it will never be
+ // placed before the textAssist item.
+ menu.add(Menu.NONE, 0 /* id */, 0 /* order */, "Test");
+ return true;
+ }
+
+ @Override
+ public boolean onPrepareActionMode(ActionMode actionMode, Menu menu) {
+ return true;
+ }
+
+ @Override
+ public boolean onActionItemClicked(ActionMode actionMode, MenuItem menuItem) {
+ return false;
+ }
+
+ @Override
+ public void onDestroyActionMode(ActionMode actionMode) {}
+ }));
+ final String text = "droid@android.com";
+
+ onView(withId(R.id.textview)).perform(replaceText(text));
+ onView(withId(R.id.textview)).perform(longPressOnTextAtIndex(text.indexOf('@')));
+ sleepForFloatingToolbarPopup();
+ assertFloatingToolbarItemIndex(android.R.id.textAssist, 0);
+ }
}
diff --git a/core/tests/coretests/src/android/widget/espresso/FloatingToolbarEspressoUtils.java b/core/tests/coretests/src/android/widget/espresso/FloatingToolbarEspressoUtils.java
index 838f4db..5206c9b 100644
--- a/core/tests/coretests/src/android/widget/espresso/FloatingToolbarEspressoUtils.java
+++ b/core/tests/coretests/src/android/widget/espresso/FloatingToolbarEspressoUtils.java
@@ -29,7 +29,13 @@
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.is;
+import android.view.MenuItem;
+import android.view.ViewGroup;
+import java.util.ArrayList;
+import java.util.List;
+import org.hamcrest.Description;
import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
import android.support.test.espresso.NoMatchingRootException;
import android.support.test.espresso.NoMatchingViewException;
@@ -123,6 +129,39 @@
}
/**
+ * Asserts that the floating toolbar contains a specified item at a specified index.
+ *
+ * @param menuItemId id of the menu item
+ * @param index expected index of the menu item in the floating toolbar
+ * @throws AssertionError if the assertion fails
+ */
+ public static void assertFloatingToolbarItemIndex(final int menuItemId, final int index) {
+ onFloatingToolBar().check(matches(new TypeSafeMatcher<View>() {
+ private List<Integer> menuItemIds = new ArrayList<>();
+
+ @Override
+ public boolean matchesSafely(View view) {
+ collectMenuItemIds(view);
+ return menuItemIds.size() > index && menuItemIds.get(index) == menuItemId;
+ }
+
+ @Override
+ public void describeTo(Description description) {}
+
+ private void collectMenuItemIds(View view) {
+ if (view.getTag() instanceof MenuItem) {
+ menuItemIds.add(((MenuItem) view.getTag()).getItemId());
+ } else if (view instanceof ViewGroup) {
+ ViewGroup viewGroup = (ViewGroup) view;
+ for (int i = 0; i < viewGroup.getChildCount(); i++) {
+ collectMenuItemIds(viewGroup.getChildAt(i));
+ }
+ }
+ }
+ }));
+ }
+
+ /**
* Asserts that the floating toolbar doesn't contain the specified item.
*
* @param itemLabel label of the item.