[Density] Adjust navigation popup for desktop density Bug: 414869569 Change-Id: Ie76e06f5d3a61ad3dc18957036c90e583771e798 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7004768 Commit-Queue: Sinan Sahin <sinansahin@google.com> Reviewed-by: Wenyu Fu <wenyufu@chromium.org> Cr-Commit-Position: refs/heads/main@{#1524481}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationSheetCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationSheetCoordinator.java index c6d78a7e..668f5384 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationSheetCoordinator.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationSheetCoordinator.java
@@ -33,6 +33,7 @@ import org.chromium.ui.modelutil.ModelListAdapter; import org.chromium.ui.modelutil.PropertyKey; import org.chromium.ui.modelutil.PropertyModel; +import org.chromium.ui.util.AttrUtils; import java.util.function.Supplier; @@ -151,7 +152,8 @@ context.getResources().getDisplayMetrics().density * LONG_SWIPE_PEEK_THRESHOLD_DP, parent.getWidth() / 2f); - mItemHeight = getSizePx(context, R.dimen.navigation_popup_item_height); + mItemHeight = AttrUtils.getDimensionPixelSize(context, R.attr.listItemHeightLarge); + assert mItemHeight >= 0; mContentPadding = getSizePx(context, R.dimen.navigation_sheet_content_top_padding) + getSizePx(context, R.dimen.navigation_sheet_content_bottom_padding);
diff --git a/chrome/browser/ui/android/toolbar/java/res/layout/navigation_popup_item.xml b/chrome/browser/ui/android/toolbar/java/res/layout/navigation_popup_item.xml index e9a9910..f46a8ade5 100644 --- a/chrome/browser/ui/android/toolbar/java/res/layout/navigation_popup_item.xml +++ b/chrome/browser/ui/android/toolbar/java/res/layout/navigation_popup_item.xml
@@ -10,8 +10,8 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:paddingStart="@dimen/navigation_popup_default_padding" - android:paddingEnd="@dimen/navigation_popup_default_padding" + android:paddingStart="@dimen/navigation_popup_start_padding" + android:paddingEnd="@dimen/navigation_popup_end_padding" android:gravity="center_vertical" android:orientation="horizontal" android:background="?attr/selectableItemBackground"> @@ -19,13 +19,16 @@ <FrameLayout android:layout_width="wrap_content" android:layout_height="match_parent"> - <View + <ImageView android:id="@+id/favicon_bg" - android:layout_width="@dimen/navigation_popup_favicon_bg_size" - android:layout_height="@dimen/navigation_popup_favicon_bg_size" + android:layout_width="?attr/minInteractTargetSize" + android:layout_height="?attr/minInteractTargetSize" android:layout_gravity="center" - android:background="@drawable/oval_shape" - android:backgroundTint="@color/context_menu_header_circle_bg_color" /> + android:padding="@dimen/navigation_popup_favicon_padding" + android:src="@drawable/oval_shape" + android:tint="@color/context_menu_header_circle_bg_color" + android:scaleType="fitCenter" + android:importantForAccessibility="no" /> <ImageView android:id="@+id/favicon_img" android:layout_width="@dimen/default_favicon_size" @@ -39,8 +42,8 @@ android:gravity="center_vertical" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:layout_marginStart="@dimen/navigation_popup_default_padding" - android:minHeight="@dimen/navigation_popup_item_height" - android:textAppearance="@style/TextAppearance.TextLarge.Primary" + android:layout_marginStart="@dimen/navigation_popup_icon_text_spacing" + android:minHeight="?attr/listItemHeightLarge" + android:textAppearance="@style/TextAppearance.DensityAdaptive.TextLarge.Primary" android:singleLine="true"/> </LinearLayout>
diff --git a/chrome/browser/ui/android/toolbar/java/res/values/dimens.xml b/chrome/browser/ui/android/toolbar/java/res/values/dimens.xml index 9028892..7bdacb3 100644 --- a/chrome/browser/ui/android/toolbar/java/res/values/dimens.xml +++ b/chrome/browser/ui/android/toolbar/java/res/values/dimens.xml
@@ -50,10 +50,12 @@ <!-- Navigation history popup dimensions --> <dimen name="navigation_popup_width">312dp</dimen> - <dimen name="navigation_popup_item_height">56dp</dimen> - <dimen name="navigation_popup_default_padding">16dp</dimen> + <dimen name="navigation_popup_end_padding">16dp</dimen> + <!-- There is extra padding coming from navigation_popup_favicon_padding for the next two. --> + <dimen name="navigation_popup_start_padding">10dp</dimen> + <dimen name="navigation_popup_icon_text_spacing">6dp</dimen> <dimen name="navigation_popup_top_padding">8dp</dimen> - <dimen name="navigation_popup_favicon_bg_size">36dp</dimen> + <dimen name="navigation_popup_favicon_padding">6dp</dimen> <dimen name="navigation_popup_tablet_min_width">200dp</dimen> <dimen name="navigation_popup_tablet_max_width">500dp</dimen> <dimen name="navigation_popup_tablet_width_margin">48dp</dimen>
diff --git a/components/browser_ui/theme/android/templates/res/values/themes.xml b/components/browser_ui/theme/android/templates/res/values/themes.xml index c24fa54..69daaee 100644 --- a/components/browser_ui/theme/android/templates/res/values/themes.xml +++ b/components/browser_ui/theme/android/templates/res/values/themes.xml
@@ -153,6 +153,7 @@ <!-- List item dimensions. --> <item name="listItemHeight">?attr/minInteractTargetSize</item> + <item name="listItemHeightLarge">@dimen/list_menu_item_min_height_large</item> <item name="listItemIconSize">@dimen/list_menu_item_icon_size</item> <item name="listItemIconPadding">@dimen/list_menu_item_icon_padding</item> <item name="listItemControlPadding">@dimen/list_menu_item_control_padding</item> @@ -343,6 +344,7 @@ <item name="popupBgCornerRadius">@dimen/popup_bg_corner_radius_16dp</item> <!-- List item dimensions. --> + <item name="listItemHeightLarge">@dimen/list_menu_item_min_height_large_desktop</item> <item name="listItemIconSize">@dimen/list_menu_item_icon_size_desktop</item> <item name="listItemIconPadding">@dimen/list_menu_item_icon_padding_desktop</item> <item name="listItemControlPadding">@dimen/list_menu_item_control_padding_desktop</item>
diff --git a/ui/android/java/res/values/attrs.xml b/ui/android/java/res/values/attrs.xml index 3f510b83..4fd1aae3 100644 --- a/ui/android/java/res/values/attrs.xml +++ b/ui/android/java/res/values/attrs.xml
@@ -77,6 +77,7 @@ <attr name="minInteractTargetSize" format="dimension" /> <attr name="listItemHeight" format="dimension" /> + <attr name="listItemHeightLarge" format="dimension" /> <attr name="listItemIconSize" format="dimension" /> <attr name="listItemIconPadding" format="dimension" /> <!-- For views that contain a control icon, e.g. checkbox, which includes built-in padding. -->
diff --git a/ui/android/java/res/values/dimens.xml b/ui/android/java/res/values/dimens.xml index a52c2665..dd628bdf 100644 --- a/ui/android/java/res/values/dimens.xml +++ b/ui/android/java/res/values/dimens.xml
@@ -113,12 +113,14 @@ <dimen name="list_menu_item_horizontal_padding">16dp</dimen> <dimen name="list_menu_elevation">3dp</dimen> <dimen name="list_menu_item_min_height">48dp</dimen> + <dimen name="list_menu_item_min_height_large">56dp</dimen> <dimen name="list_menu_item_icon_size_desktop">20dp</dimen> <dimen name="list_menu_item_icon_padding_desktop">8dp</dimen> <dimen name="list_menu_item_control_padding_desktop">5dp</dimen> <dimen name="list_menu_item_start_icon_end_margin_desktop">12dp</dimen> <dimen name="list_divider_layout_height_desktop">10dp</dimen> + <dimen name="list_menu_item_min_height_large_desktop">36dp</dimen> <dimen name="list_menu_flyout_popup_horizontal_overlap">8dp</dimen>
diff --git a/ui/android/java/res/values/themes.xml b/ui/android/java/res/values/themes.xml index 526c818..e74956c 100644 --- a/ui/android/java/res/values/themes.xml +++ b/ui/android/java/res/values/themes.xml
@@ -14,6 +14,7 @@ <!-- List item dimensions. --> <item name="listItemHeight">?attr/minInteractTargetSize</item> + <item name="listItemHeightLarge">@dimen/list_menu_item_min_height_large</item> <item name="listItemIconSize">@dimen/list_menu_item_icon_size</item> <item name="listItemIconPadding">@dimen/list_menu_item_icon_padding</item> <item name="listItemControlPadding">@dimen/list_menu_item_control_padding</item>
diff --git a/ui/android/java/src/org/chromium/ui/util/AttrUtils.java b/ui/android/java/src/org/chromium/ui/util/AttrUtils.java index 0f98f052..78949db 100644 --- a/ui/android/java/src/org/chromium/ui/util/AttrUtils.java +++ b/ui/android/java/src/org/chromium/ui/util/AttrUtils.java
@@ -4,11 +4,13 @@ package org.chromium.ui.util; +import android.content.Context; import android.content.res.Resources.Theme; import android.util.TypedValue; import androidx.annotation.AttrRes; import androidx.annotation.ColorInt; +import androidx.annotation.Px; import org.chromium.build.annotations.NullMarked; @@ -51,4 +53,22 @@ return defaultColor; } } + + /** + * Resolves a dimension attribute from the theme and returns its value in pixels. + * + * @param context The context to resolve the theme attribute from. + * @param dimenAttr The dimension attribute to resolve. + * @return The dimension value in pixels, or -1 if the attribute is not defined in the theme. + */ + public static @Px int getDimensionPixelSize(Context context, @AttrRes int dimenAttr) { + var typedValue = new TypedValue(); + + if (context.getTheme().resolveAttribute(dimenAttr, typedValue, true)) { + return TypedValue.complexToDimensionPixelSize( + typedValue.data, context.getResources().getDisplayMetrics()); + } + + return -1; + } }