Refactor AppMenu background

This CL replaces the 9-patch background with an xml and uses elevation to draw shadows. Bug: 1165077 Change-Id: Ica3ffd0790a695ca7261e63c27f3f55977ed1c39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2705291 Reviewed-by: Theresa <twellington@chromium.org> Reviewed-by: Gang Wu <gangwu@chromium.org> Commit-Queue: Sinan Sahin <sinansahin@google.com> Cr-Commit-Position: refs/heads/master@{#861038}
marcosholgado · Mar 9, 2021 · f6d3aec · f6d3aec
1 parent 95f5c06
commit f6d3aec
Show file tree

Hide file tree

Showing 15 changed files with 97 additions and 37 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeBaseAppCompatActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeBaseAppCompatActivity.java
@@ -15,7 +15,10 @@
 import androidx.appcompat.app.AppCompatActivity;
 
 import org.chromium.base.ContextUtils;
+import org.chromium.chrome.R;
 import org.chromium.chrome.browser.base.SplitCompatUtils;
+import org.chromium.chrome.browser.flags.CachedFeatureFlags;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.language.GlobalAppLocaleController;
 import org.chromium.chrome.browser.night_mode.GlobalNightModeStateProviderHolder;
 import org.chromium.chrome.browser.night_mode.NightModeStateProvider;
@@ -51,6 +54,9 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
 
         initializeNightModeStateProvider();
         mNightModeStateProvider.addObserver(this);
+        if (CachedFeatureFlags.isEnabled(ChromeFeatureList.THEME_REFACTOR_ANDROID)) {
+            setTheme(R.style.ThemeRefactorAppThemeOverlay);
+        }
         super.onCreate(savedInstanceState);
 
         // Activity level locale overrides must be done in onCreate.

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java b/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java
@@ -82,6 +82,7 @@ public void cacheNativeFlags() {
                 ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID,
                 ChromeFeatureList.TAB_TO_GTS_ANIMATION,
                 ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_THREE_BUTTON_ACTIONBAR,
+                ChromeFeatureList.THEME_REFACTOR_ANDROID,
                 ChromeFeatureList.TOOLBAR_USE_HARDWARE_BITMAP_DRAW,
                 ChromeFeatureList.USE_CHIME_ANDROID_SDK,
                 ChromeFeatureList.READ_LATER,

diff --git a/.../browser/flags/android/java/src/org/chromium/chrome/browser/flags/CachedFeatureFlags.java b/.../browser/flags/android/java/src/org/chromium/chrome/browser/flags/CachedFeatureFlags.java
@@ -73,13 +73,13 @@ public class CachedFeatureFlags {
             put(ChromeFeatureList.CLOSE_TAB_SUGGESTIONS, false);
             put(ChromeFeatureList.CRITICAL_PERSISTED_TAB_DATA, false);
             put(ChromeFeatureList.INSTANT_START, false);
-            put(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID, false);
             put(ChromeFeatureList.TAB_TO_GTS_ANIMATION, false);
             put(ChromeFeatureList.TEST_DEFAULT_DISABLED, false);
             put(ChromeFeatureList.TEST_DEFAULT_ENABLED, true);
             put(ChromeFeatureList.REPORT_FEED_USER_ACTIONS, false);
             put(ChromeFeatureList.INTEREST_FEED_V2, true);
             put(ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_THREE_BUTTON_ACTIONBAR, false);
+            put(ChromeFeatureList.THEME_REFACTOR_ANDROID, false);
             put(ChromeFeatureList.USE_CHIME_ANDROID_SDK, false);
             put(ChromeFeatureList.CCT_INCOGNITO_AVAILABLE_TO_THIRD_PARTY, false);
             put(ChromeFeatureList.READ_LATER, false);

diff --git a/chrome/browser/ui/android/appmenu/internal/java/res/drawable/menu_action_bar_bg.xml b/chrome/browser/ui/android/appmenu/internal/java/res/drawable/menu_action_bar_bg.xml
@@ -8,10 +8,10 @@
     android:shape="rectangle">
     <corners
         android:radius="1dp"
-        android:topLeftRadius="@dimen/popup_bg_corner_radius"
-        android:topRightRadius="@dimen/popup_bg_corner_radius"
+        android:topLeftRadius="@dimen/app_menu_corner_size"
+        android:topRightRadius="@dimen/app_menu_corner_size"
         android:bottomLeftRadius="0dp"
         android:bottomRightRadius="0dp" />
     <solid
         android:color="@color/default_bg_color_secondary" />
-</shape>
+</shape>
diff --git a/chrome/browser/ui/android/appmenu/internal/java/res/values/dimens.xml b/chrome/browser/ui/android/appmenu/internal/java/res/values/dimens.xml
@@ -11,4 +11,7 @@
 
     <!-- Menu button dragging related dimensions -->
     <dimen name="auto_scroll_full_velocity">500dp</dimen>
+
+    <!-- Misc. menu dimens -->
+    <dimen name="menu_elevation">@dimen/default_elevation_2</dimen>
 </resources>
diff --git a/.../ui/android/appmenu/internal/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenu.java b/.../ui/android/appmenu/internal/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenu.java
@@ -37,7 +37,6 @@
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
-import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.SysUtils;
 import org.chromium.base.metrics.RecordHistogram;
@@ -212,10 +211,7 @@ void show(Context context, final View anchorView, boolean isByPermanentButton,
         // drawable here even though our style says @null we should use this padding instead...
         Drawable originalBgDrawable = mPopup.getBackground();
 
-        // Need to explicitly set the background here.  Relying on it being set in the style caused
-        // an incorrectly drawn background.
-        mPopup.setBackgroundDrawable(ApiCompatibilityUtils.getDrawable(
-                context.getResources(), R.drawable.popup_bg_tinted));
+        mPopup.setBackgroundDrawable(null);
         // Make sure that the popup window will be closed when touch outside of it.
         mPopup.setOutsideTouchable(true);
 
@@ -224,14 +220,6 @@ void show(Context context, final View anchorView, boolean isByPermanentButton,
         // Turn off window animations for low end devices.
         if (SysUtils.isLowEndDevice()) mPopup.setAnimationStyle(0);
 
-        Rect bgPadding = new Rect();
-        mPopup.getBackground().getPadding(bgPadding);
-
-        int menuWidth = context.getResources().getDimensionPixelSize(R.dimen.menu_width);
-        int popupWidth = menuWidth + bgPadding.left + bgPadding.right;
-
-        mPopup.setWidth(popupWidth);
-
         mCurrentScreenRotation = screenRotation;
         mIsByPermanentButton = isByPermanentButton;
 
@@ -246,6 +234,25 @@ void show(Context context, final View anchorView, boolean isByPermanentButton,
             }
         }
 
+        // A List adapter for visible items in the Menu. The first row is added as a header to the
+        // list view.
+        mAdapter = new AppMenuAdapter(this, menuItems, LayoutInflater.from(context),
+                highlightedItemId, customViewBinders, mIconBeforeItem);
+
+        ViewGroup contentView =
+                (ViewGroup) LayoutInflater.from(context).inflate(R.layout.app_menu_layout, null);
+        // Setting android:clipToOutline in xml causes an "attribute not found" error.
+        contentView.setClipToOutline(true);
+        mPopup.setElevation(context.getResources().getDimensionPixelSize(R.dimen.menu_elevation));
+
+        Rect bgPadding = new Rect();
+        contentView.getBackground().getPadding(bgPadding);
+
+        int menuWidth = context.getResources().getDimensionPixelSize(R.dimen.menu_width);
+        int popupWidth = menuWidth + bgPadding.left + bgPadding.right;
+
+        mPopup.setWidth(popupWidth);
+
         Rect sizingPadding = new Rect(bgPadding);
         if (isByPermanentButton && originalBgDrawable != null) {
             Rect originalPadding = new Rect();
@@ -254,13 +261,6 @@ void show(Context context, final View anchorView, boolean isByPermanentButton,
             sizingPadding.bottom = originalPadding.bottom;
         }
 
-        // A List adapter for visible items in the Menu. The first row is added as a header to the
-        // list view.
-        mAdapter = new AppMenuAdapter(this, menuItems, LayoutInflater.from(context),
-                highlightedItemId, customViewBinders, mIconBeforeItem);
-
-        ViewGroup contentView =
-                (ViewGroup) LayoutInflater.from(context).inflate(R.layout.app_menu_layout, null);
         mListView = (ListView) contentView.findViewById(R.id.app_menu_list);
 
         int footerHeight = inflateFooter(footerResourceId, contentView, menuWidth);

diff --git a/...android/appmenu/internal/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenuTest.java b/...android/appmenu/internal/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenuTest.java
@@ -860,16 +860,14 @@ private View getViewAtPosition(int index) {
     private Rect getPopupLocationRect() {
         View contentView = mAppMenuHandler.getAppMenu().getPopup().getContentView();
         CriteriaHelper.pollUiThread(() -> contentView.getHeight() != 0);
-        Rect bgPadding = new Rect();
-        mAppMenuHandler.getAppMenu().getPopup().getBackground().getPadding(bgPadding);
 
         Rect popupRect = new Rect();
         int[] popupLocation = new int[2];
         contentView.getLocationOnScreen(popupLocation);
-        popupRect.left = popupLocation[0] - bgPadding.left;
-        popupRect.top = popupLocation[1] - bgPadding.top;
-        popupRect.right = popupLocation[0] + contentView.getWidth() + bgPadding.right;
-        popupRect.bottom = popupLocation[1] + contentView.getHeight() + bgPadding.bottom;
+        popupRect.left = popupLocation[0];
+        popupRect.top = popupLocation[1];
+        popupRect.right = popupLocation[0] + contentView.getWidth();
+        popupRect.bottom = popupLocation[1] + contentView.getHeight();
         return popupRect;
     }
 

diff --git a/components/browser_ui/styles/android/BUILD.gn b/components/browser_ui/styles/android/BUILD.gn
@@ -182,5 +182,8 @@ android_resources("java_resources") {
     sources += [ "java/res-arcore/drawable/ic_ar_core_install.xml" ]
   }
 
-  deps = [ "//ui/android:ui_java_resources" ]
+  deps = [
+    "//third_party/android_deps:material_design_java",
+    "//ui/android:ui_java_resources",
+  ]
 }
diff --git a/components/browser_ui/styles/android/java/res/values/styles.xml b/components/browser_ui/styles/android/java/res/values/styles.xml
@@ -42,6 +42,9 @@
             @style/TextAppearance.TextLarge.Primary
         </item>
         <item name="android:contextPopupMenuStyle" tools:targetApi="24">@style/PopupMenuStyle</item>
+
+        <!-- Shape -->
+        <item name="menuCornerSize">@dimen/popup_bg_corner_radius</item>
     </style>
 
     <!-- Popup and long-press context popup menu style -->
@@ -152,4 +155,9 @@
         <item name="colorAccent">@color/default_control_color_active</item>
     </style>
     <style name="Theme.Chromium.Fullscreen" parent="Base.Theme.Chromium.Fullscreen" />
+
+    <!-- Refactored styles -->
+    <style name="ThemeRefactorAppThemeOverlay">
+        <item name="menuCornerSize">@dimen/refactor_menu_rounded_corner_radius</item>
+    </style>
 </resources>
diff --git a/components/browser_ui/widget/android/BUILD.gn b/components/browser_ui/widget/android/BUILD.gn
@@ -118,6 +118,7 @@ android_library("java") {
     "//components/embedder_support/android:util_java",
     "//third_party/android_deps:android_support_v4_java",
     "//third_party/android_deps:android_support_v7_appcompat_java",
+    "//third_party/android_deps:material_design_java",
     "//third_party/androidx:androidx_annotation_annotation_java",
     "//third_party/androidx:androidx_appcompat_appcompat_resources_java",
     "//third_party/androidx:androidx_coordinatorlayout_coordinatorlayout_java",
@@ -176,6 +177,7 @@ android_resources("java_resources") {
     "java/res/drawable-xxxhdpi/ic_drag_handle_grey600_24dp.png",
     "java/res/drawable-xxxhdpi/ic_more_vert_24dp_on_dark_bg.png",
     "java/res/drawable-xxxhdpi/ic_more_vert_24dp_on_light_bg.png",
+    "java/res/drawable/app_menu_bg.xml",
     "java/res/drawable/async_image_view_unavailable.xml",
     "java/res/drawable/async_image_view_waiting.xml",
     "java/res/drawable/hairline_border_card_background.xml",
@@ -233,6 +235,7 @@ android_resources("java_resources") {
   deps = [
     "//components/browser_ui/strings/android:browser_ui_strings_grd",
     "//components/browser_ui/styles/android:java_resources",
+    "//third_party/android_deps:material_design_java",
     "//ui/android:ui_java_resources",
   ]
 }

diff --git a/components/browser_ui/widget/android/java/res/drawable/app_menu_bg.xml b/components/browser_ui/widget/android/java/res/drawable/app_menu_bg.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2021 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. -->
+
+<layer-list
+    xmlns:android="http://schemas.android.com/apk/res/android">
+    <item
+        android:top="@dimen/app_menu_shadow_length"
+        android:left="@dimen/app_menu_shadow_length"
+        android:bottom="@dimen/app_menu_shadow_length"
+        android:right="@dimen/app_menu_shadow_length">
+        <shape
+            android:shape="rectangle">
+            <corners android:radius="@dimen/app_menu_corner_size" />
+            <solid android:color="@color/popup_bg_color" />
+            <padding
+                android:top="@dimen/app_menu_shadow_length"
+                android:left="@dimen/app_menu_shadow_length"
+                android:bottom="@dimen/app_menu_shadow_length"
+                android:right="@dimen/app_menu_shadow_length" />
+        </shape>
+    </item>
+</layer-list>
diff --git a/components/browser_ui/widget/android/java/res/layout/app_menu_layout.xml b/components/browser_ui/widget/android/java/res/layout/app_menu_layout.xml
@@ -7,7 +7,8 @@
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
-    android:orientation="vertical" >
+    android:orientation="vertical"
+    android:background="@drawable/app_menu_bg">
 
     <ListView
         android:id="@+id/app_menu_list"
@@ -20,5 +21,4 @@
         android:inflatedId="@+id/app_menu_footer"
         android:layout_width="match_parent"
         android:layout_height="wrap_content" />
-
 </LinearLayout>
diff --git a/components/browser_ui/widget/android/java/res/values/dimens.xml b/components/browser_ui/widget/android/java/res/values/dimens.xml
@@ -96,11 +96,11 @@
     <dimen name="swipe_vertical_drag_threshold">5dp</dimen>
     <dimen name="swipe_horizontal_drag_threshold">10dp</dimen>
 
-  <!-- Query tile dimensions -->
-  <dimen name="tile_ideal_width">80dp</dimen>
+    <!-- Query tile dimensions -->
+    <dimen name="tile_ideal_width">80dp</dimen>
 
-  <!-- Query tile container dimensions -->
-  <dimen name="tile_grid_inter_tile_padding">12dp</dimen>
+    <!-- Query tile container dimensions -->
+    <dimen name="tile_grid_inter_tile_padding">12dp</dimen>
 
     <!-- Tile view dimensions -->
     <dimen name="tile_view_bg_corner_radius">2dp</dimen>
@@ -129,4 +129,10 @@
     <dimen name="chip_list_inter_chip_padding">2.5dp</dimen>
     <dimen name="chip_list_side_padding">-16dp</dimen>
 
+    <!-- Misc. app menu dimens -->
+    <dimen name="app_menu_corner_size">?attr/menuCornerSize</dimen>
+    <dimen name="app_menu_shadow_length">@dimen/default_shadow_length_elev_2</dimen>
+
+    <!-- Misc refactor dimensions -->
+    <dimen name="refactor_menu_rounded_corner_radius">28dp</dimen>
 </resources>
diff --git a/ui/android/java/res/values/attrs.xml b/ui/android/java/res/values/attrs.xml
@@ -7,6 +7,8 @@
     <attr name="rippleColor" format="color"/>
     <!-- The top or bottom inset of a drawable. -->
     <attr name="verticalInset" format="reference|dimension"/>
+    <!-- The rounded corner radius for the menus. -->
+    <attr name="menuCornerSize" format="reference|dimension"/>
 
     <declare-styleable name="ButtonCompat">
         <!-- The color of the button background. -->

diff --git a/ui/android/java/res/values/dimens.xml b/ui/android/java/res/values/dimens.xml
@@ -90,4 +90,10 @@
     -->
     <!-- TODO(crbug.com/900912): Fix and remove lint ignore -->
     <dimen name="config_min_scaling_span" tools:ignore="InOrMmUsage">12.0mm</dimen>
+
+    <!-- Elevation levels -->
+    <dimen name="default_elevation_2">3dp</dimen>
+
+    <!-- Arbitrary shadow length for elevation levels -->
+    <dimen name="default_shadow_length_elev_2">6dp</dimen>
 </resources>