[Android Bulk Tab Edit] Update SelectionAction Icons

Update Select/Deselect All icons and cleanup any state related to the old animated version. Bug: 1385431 Change-Id: I249466940c061286b602fecf3d5f87aa907aa937 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4031010 Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org> Reviewed-by: Fred Mello <fredmello@chromium.org> Cr-Commit-Position: refs/heads/main@{#1073393}
chromium · Nov 18, 2022 · f5c6f37 · f5c6f37
1 parent dc98944
commit f5c6f37
Show file tree

Hide file tree

Showing 18 changed files with 96 additions and 195 deletions.
diff --git a/chrome/android/features/tab_ui/BUILD.gn b/chrome/android/features/tab_ui/BUILD.gn
@@ -33,11 +33,13 @@ android_resources("java_resources") {
     "java/res/drawable/fake_search_box_text_box_bg_incognito.xml",
     "java/res/drawable/ic_check_googblue_20dp_animated.xml",
     "java/res/drawable/ic_close_tabs_24dp.xml",
+    "java/res/drawable/ic_deselect_all_24dp.xml",
     "java/res/drawable/ic_group_icon_16dp.xml",
     "java/res/drawable/ic_price_alert_blue.xml",
     "java/res/drawable/ic_rating_star_full.xml",
     "java/res/drawable/ic_rating_star_half.xml",
     "java/res/drawable/ic_rating_star_outline.xml",
+    "java/res/drawable/ic_select_all_24dp.xml",
     "java/res/drawable/iph_drag_and_drop_animated_drawable.xml",
     "java/res/drawable/iph_drag_and_drop_drawable.xml",
     "java/res/drawable/price_card_background.xml",

diff --git a/chrome/android/features/tab_ui/java/res/drawable/ic_deselect_all_24dp.xml b/chrome/android/features/tab_ui/java/res/drawable/ic_deselect_all_24dp.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2022 The Chromium Authors. All rights reserved.
+     Use of this source code is governed by a BSD-style license that can be
+     found in the LICENSE file. -->
+
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+    android:width="24dp"
+    android:height="24dp"
+    android:viewportWidth="24"
+    android:viewportHeight="24"
+    android:tint="@macro/default_icon_color">
+  <path
+      android:fillColor="@android:color/white"
+      android:pathData="M7.94,5.12L6.49,3.66C8.07,2.61 9.96,2 12,2c5.52,0 10,4.48 10,10c0,2.04 -0.61,3.93 -1.66,5.51l-1.46,-1.46C19.59,14.86 20,13.48 20,12c0,-4.41 -3.59,-8 -8,-8C10.52,4 9.14,4.41 7.94,5.12zM17.66,9.53l-1.41,-1.41l-2.65,2.65l1.41,1.41L17.66,9.53zM19.78,22.61l-2.27,-2.27C15.93,21.39 14.04,22 12,22C6.48,22 2,17.52 2,12c0,-2.04 0.61,-3.93 1.66,-5.51L1.39,4.22l1.41,-1.41l18.38,18.38L19.78,22.61zM16.06,18.88l-3.88,-3.88l-1.59,1.59l-4.24,-4.24l1.41,-1.41l2.83,2.83l0.18,-0.18L5.12,7.94C4.41,9.14 4,10.52 4,12c0,4.41 3.59,8 8,8C13.48,20 14.86,19.59 16.06,18.88z"/>
+</vector>
diff --git a/chrome/android/features/tab_ui/java/res/drawable/ic_select_all_24dp.xml b/chrome/android/features/tab_ui/java/res/drawable/ic_select_all_24dp.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2022 The Chromium Authors. All rights reserved.
+     Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file. -->
+
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+    android:width="24dp"
+    android:height="24dp"
+    android:viewportWidth="24"
+    android:viewportHeight="24"
+    android:tint="@macro/default_icon_color">
+  <path
+      android:fillColor="@android:color/white"
+      android:pathData="M12,2C6.48,2 2,6.48 2,12s4.48,10 10,10 10,-4.48 10,-10S17.52,2 12,2zM12,20c-4.41,0 -8,-3.59 -8,-8s3.59,-8 8,-8 8,3.59 8,8 -3.59,8 -8,8zM10,14.2l-2.6,-2.6L6,13l4,4 8,-8 -1.4,-1.4z"/>
+</vector>
diff --git a/chrome/android/features/tab_ui/java/res/values/dimens.xml b/chrome/android/features/tab_ui/java/res/values/dimens.xml
@@ -81,6 +81,5 @@ found in the LICENSE file.
 
     <!-- Dimens for TabSelectionEditorV2 -->
     <dimen name="tab_selection_editor_action_view_padding">14dp</dimen>
-    <dimen name="tab_selection_editor_selection_action_inset">2dp</dimen>
     <dimen name="tab_selection_editor_share_sheet_preview_thumbnail_padding">10dp</dimen>
 </resources>
diff --git a/...b_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGridDialogMediator.java b/...b_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGridDialogMediator.java
@@ -556,9 +556,8 @@ private void setupDialogSelectionEditor() {
 
         if (TabUiFeatureUtilities.isTabSelectionEditorV2Enabled(mContext)) {
             List<TabSelectionEditorAction> actions = new ArrayList<>();
-            actions.add(TabSelectionEditorSelectionAction.createAction(mContext, ShowMode.MENU_ONLY,
-                    ButtonType.ICON_AND_TEXT, IconPosition.END,
-                    mTabModelSelector.getCurrentModel().isIncognito()));
+            actions.add(TabSelectionEditorSelectionAction.createAction(
+                    mContext, ShowMode.MENU_ONLY, ButtonType.ICON_AND_TEXT, IconPosition.END));
             actions.add(TabSelectionEditorCloseAction.createAction(
                     mContext, ShowMode.MENU_ONLY, ButtonType.ICON_AND_TEXT, IconPosition.START));
             actions.add(TabSelectionEditorUngroupAction.createAction(

diff --git a/...i/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorAction.java b/...i/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorAction.java
@@ -159,13 +159,10 @@ public TabSelectionEditorAction(int menuItemId, @ShowMode int showMode,
                                 ColorStateList.valueOf(Color.TRANSPARENT))
                         .with(TabSelectionEditorActionProperties.ICON_TINT,
                                 ColorStateList.valueOf(Color.TRANSPARENT))
-                        .with(TabSelectionEditorActionProperties.SKIP_ICON_TINT, false)
                         .with(TabSelectionEditorActionProperties.ON_CLICK_LISTENER, this::perform)
                         .with(TabSelectionEditorActionProperties.SHOULD_DISMISS_MENU, true)
                         .with(TabSelectionEditorActionProperties.ON_SELECTION_STATE_CHANGE,
                                 this::onSelectionStateChange)
-                        .with(TabSelectionEditorActionProperties.ON_SHOWN_IN_MENU,
-                                this::onShownInMenu)
                         .build();
 
         if (contentDescriptionResourceId == null) return;
@@ -199,8 +196,6 @@ public boolean shouldNotifyObserversOfAction() {
         return true;
     }
 
-    public void onShownInMenu() {}
-
     /**
      * @return Whether the TabSelectionEditor supports applying the actions to related tabs.
      */

diff --git a/.../org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorActionProperties.java b/.../org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorActionProperties.java
@@ -41,38 +41,29 @@ public class TabSelectionEditorActionProperties {
             new WritableObjectPropertyKey<>();
 
     /**
-     * Tint for the icon. This should be null if {@code SKIP_ICON_TINT} is true.
+     * Tint for the icon.
      */
     public static final WritableObjectPropertyKey<ColorStateList> ICON_TINT =
             new WritableObjectPropertyKey<>();
 
-    /**
-     * If true skip usage of the {@code ICON_TINT} property. Used if the icon tint is handled
-     * directly by the {@link TabSelectionEditorAction}.
-     */
-    public static final WritableBooleanPropertyKey SKIP_ICON_TINT =
-            new WritableBooleanPropertyKey();
-
     public static final WritableObjectPropertyKey<Runnable> ON_CLICK_LISTENER =
             new WritableObjectPropertyKey<>();
     public static final WritableBooleanPropertyKey SHOULD_DISMISS_MENU =
             new WritableBooleanPropertyKey();
     public static final WritableObjectPropertyKey<Callback<List<Integer>>>
             ON_SELECTION_STATE_CHANGE = new WritableObjectPropertyKey<>();
-    public static final WritableObjectPropertyKey<Runnable> ON_SHOWN_IN_MENU =
-            new WritableObjectPropertyKey<>();
 
     /**
      * Keys for the {@link TabSelectionEditorAction}.
      */
     public static final PropertyKey[] ACTION_KEYS = {MENU_ITEM_ID, SHOW_MODE, BUTTON_TYPE,
             ICON_POSITION, TITLE_RESOURCE_ID, TITLE_IS_PLURAL, CONTENT_DESCRIPTION_RESOURCE_ID,
-            ICON, ENABLED, ITEM_COUNT, TEXT_TINT, ICON_TINT, SKIP_ICON_TINT, ON_CLICK_LISTENER,
-            SHOULD_DISMISS_MENU, ON_SELECTION_STATE_CHANGE, ON_SHOWN_IN_MENU};
+            ICON, ENABLED, ITEM_COUNT, TEXT_TINT, ICON_TINT, ON_CLICK_LISTENER, SHOULD_DISMISS_MENU,
+            ON_SELECTION_STATE_CHANGE};
 
     /**
      * Keys for the {@link TabSelectionEditorMenuItem}.
      */
-    public static final PropertyKey[] MENU_ITEM_KEYS = {MENU_ITEM_ID, TITLE, CONTENT_DESCRIPTION,
-            ICON, ICON_TINT, ENABLED, ITEM_COUNT, ON_SHOWN_IN_MENU};
+    public static final PropertyKey[] MENU_ITEM_KEYS = {
+            MENU_ITEM_ID, TITLE, CONTENT_DESCRIPTION, ICON, ICON_TINT, ENABLED, ITEM_COUNT};
 }
diff --git a/.../org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorActionViewLayout.java b/.../org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorActionViewLayout.java
@@ -80,12 +80,9 @@ ListMenuButton getListMenuButtonForTesting() {
     }
 
     /**
-     * @param popupListener for handling show events.
      * @param delegate for handling menu button presses.
      */
-    public void setListMenuButtonDelegate(
-            ListMenuButton.PopupMenuShownListener popupListener, ListMenuButtonDelegate delegate) {
-        mMenuButton.addPopupListener(popupListener);
+    public void setListMenuButtonDelegate(ListMenuButtonDelegate delegate) {
         mMenuButton.setDelegate(delegate);
     }
 

diff --git a/...java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMediator.java b/...java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMediator.java
@@ -183,10 +183,7 @@ private void updateColors(boolean isIncognito) {
         if (mActionListModel == null) return;
 
         for (PropertyModel model : mActionListModel) {
-            // TODO(ckitagawa): update these tints with input from UX.
             model.set(TabSelectionEditorActionProperties.TEXT_TINT, toolbarTintColorList);
-            if (model.get(TabSelectionEditorActionProperties.SKIP_ICON_TINT)) continue;
-
             model.set(TabSelectionEditorActionProperties.ICON_TINT, toolbarTintColorList);
         }
     }

diff --git a/..._ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenu.java b/..._ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenu.java
@@ -16,7 +16,6 @@
 import org.chromium.chrome.browser.tasks.tab_management.TabSelectionEditorActionViewLayout.ActionViewLayoutDelegate;
 import org.chromium.chrome.tab_ui.R;
 import org.chromium.components.browser_ui.widget.listmenu.ListMenu;
-import org.chromium.components.browser_ui.widget.listmenu.ListMenuButton;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectionDelegate;
 import org.chromium.ui.modelutil.LayoutViewBuilder;
 import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
@@ -36,9 +35,9 @@
  * {@link TabSelectionEditorActionViewLayout} for Action views. The menu contains a list of
  * {@link TabSelectionEditorMenuItem}s which hold optional action views if room is available.
  */
-public class TabSelectionEditorMenu
-        implements ListMenu, OnItemClickListener, SelectionDelegate.SelectionObserver<Integer>,
-                   ActionViewLayoutDelegate, ListMenuButton.PopupMenuShownListener {
+public class TabSelectionEditorMenu implements ListMenu, OnItemClickListener,
+                                               SelectionDelegate.SelectionObserver<Integer>,
+                                               ActionViewLayoutDelegate {
     @Retention(RetentionPolicy.SOURCE)
     @IntDef({ListItemType.MENU_ITEM})
     public static @interface ListItemType {
@@ -85,7 +84,7 @@ public boolean isEnabled(int position) {
         mListView.setDivider(null);
         mListView.setOnItemClickListener(this);
 
-        mActionViewLayout.setListMenuButtonDelegate(this, () -> this);
+        mActionViewLayout.setListMenuButtonDelegate(() -> this);
     }
 
     private void registerItemTypes() {
@@ -96,13 +95,6 @@ private void registerItemTypes() {
         // clang-format on
     }
 
-    @Override
-    public void onPopupMenuShown() {
-        for (ListItem listItem : mModelList) {
-            listItem.model.get(TabSelectionEditorActionProperties.ON_SHOWN_IN_MENU).run();
-        }
-    }
-
     private ListItem buildListItem(int menuItemId) {
         // Model values are populated while configuring the TabSelectionEditorMenuItem.
         return new ListItem(ListItemType.MENU_ITEM,

diff --git a/...a/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenuAdapter.java b/...a/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenuAdapter.java
@@ -109,9 +109,6 @@ private void bindMenuItemProperty(
         } else if (key == TabSelectionEditorActionProperties.ON_SELECTION_STATE_CHANGE) {
             menuItem.setOnSelectionStateChange(
                     actionModel.get(TabSelectionEditorActionProperties.ON_SELECTION_STATE_CHANGE));
-        } else if (key == TabSelectionEditorActionProperties.ON_SHOWN_IN_MENU) {
-            menuItem.setOnShownInMenu(
-                    actionModel.get(TabSelectionEditorActionProperties.ON_SHOWN_IN_MENU));
         }
     }
 

diff --git a/...java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenuItem.java b/...java/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorMenuItem.java
@@ -154,15 +154,6 @@ public void setTextTint(ColorStateList colorStateList) {
     }
 
     public void setIconTint(@Nullable ColorStateList colorStateList) {
-        // A null colorStateList is used with TabSelectionEditorActionProperties.SKIP_ICON_TINT
-        // = true to signal that a custom tint is used. Ignore null so that this custom tint is
-        // not overridden.
-        if (colorStateList == null) {
-            mIconTint = null;
-            mListItem.model.set(TabSelectionEditorActionProperties.ICON_TINT, null);
-            return;
-        }
-
         // mListItem uses the default icon tint whenever shown. Cache the tint to restore it when
         // the action view shown state is toggled.
         mListItem.model.set(TabSelectionEditorActionProperties.ICON_TINT,
@@ -201,10 +192,6 @@ public void setOnSelectionStateChange(Callback<List<Integer>> callback) {
         mOnSelectionStateChange = callback;
     }
 
-    public void setOnShownInMenu(Runnable runnable) {
-        mListItem.model.set(TabSelectionEditorActionProperties.ON_SHOWN_IN_MENU, runnable);
-    }
-
     /**
      * Handler for click events on the menu item or action view.
      */