Skip to content

Commit

Permalink
[Omnibox] Avoid adjusting dimens for revamp on tablets
Browse files Browse the repository at this point in the history 
(cherry picked from commit 52bc891)

Bug: 1450721
Change-Id: I5b9e35b554b8fba77efad6d1ec280a2865f2c214
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4580408
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1152210}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4594139
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#415}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent c00b036 commit 75f6f25
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 57 deletions.
8 changes: 3 additions & 5 deletions ...er/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
View file
Expand Up @@ -339,11 +339,9 @@ protected void setStatusViewLeftSpacePercent(float percent) {

// Set the left space expansion width.
ViewGroup.LayoutParams leftSpacingParams = mStatusViewLeftSpace.getLayoutParams();
int dimen = OmniboxResourceProvider.selectMarginDimen(
R.dimen.location_bar_status_view_left_space_width,
R.dimen.location_bar_status_view_left_space_width_bigger,
R.dimen.location_bar_status_view_left_space_width_bigger);
leftSpacingParams.width = (int) (getResources().getDimensionPixelSize(dimen) * percent);
int fullSpacing = OmniboxResourceProvider.getFocusedStatusViewLeftSpacing(getContext());

leftSpacingParams.width = (int) (fullSpacing * percent);
mStatusViewLeftSpace.setLayoutParams(leftSpacingParams);
}

Expand Down
18 changes: 6 additions & 12 deletions ...wser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/OmniboxFeatures.java
View file
Expand Up @@ -105,24 +105,18 @@ public static boolean shouldShowSmallBottomMargin() {
/**
* Returns whether smaller vertical and horizontal margins should be used in the visual update.
*/
public static boolean shouldShowSmallerMargins() {
return MODERNIZE_VISUAL_UPDATE_SMALLER_MARGINS.getValue();
public static boolean shouldShowSmallerMargins(Context context) {
return shouldShowModernizeVisualUpdate(context)
&& MODERNIZE_VISUAL_UPDATE_SMALLER_MARGINS.getValue();
}

/**
* Returns whether even smaller vertical and horizontal margins should be used in the visual
* update.
*/
public static boolean shouldShowSmallestMargins() {
return MODERNIZE_VISUAL_UPDATE_SMALLEST_MARGINS.getValue();
}

/**
* Returns whether the clipboard suggestion should be grouped with other zero suggest items on
* the NTP or start surface in the visual update.
* */
public static boolean shouldMergeClipboardOnNtp() {
return MODERNIZE_VISUAL_UPDATE_MERGE_CLIPBOARD_ON_NTP.getValue();
public static boolean shouldShowSmallestMargins(Context context) {
return shouldShowModernizeVisualUpdate(context)
&& MODERNIZE_VISUAL_UPDATE_SMALLEST_MARGINS.getValue();
}

/** Returns whether the omnibox should directly consume IME (keyboard) insets. */
Expand Down
91 changes: 71 additions & 20 deletions .../omnibox/java/src/org/chromium/chrome/browser/omnibox/styles/OmniboxResourceProvider.java
View file
Expand Up @@ -383,39 +383,53 @@ private static Context maybeWrapContext(
/** Gets the margin, in pixels, on either side of an omnibox suggestion. */
public static @Px int getSideSpacing(@NonNull Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_side_spacing,
selectMarginDimen(context, R.dimen.omnibox_suggestion_side_spacing,
R.dimen.omnibox_suggestion_side_spacing_smaller,
R.dimen.omnibox_suggestion_side_spacing_smallest));
}

/** Gets the start padding for an omnibox suggestion's decoration icon. */
public static @Px int getIconStartPadding(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_24dp_icon_margin_start_modern_bigger,
R.dimen.omnibox_suggestion_24dp_icon_margin_start,
R.dimen.omnibox_suggestion_24dp_icon_margin_start));
if (!OmniboxFeatures.shouldShowModernizeVisualUpdate(context)) {
return context.getResources().getDimensionPixelSize(
R.dimen.omnibox_suggestion_24dp_icon_margin_start);
}
return context.getResources().getDimensionPixelSize(selectMarginDimen(context,
R.dimen.omnibox_suggestion_24dp_icon_margin_start_modern_bigger,
R.dimen.omnibox_suggestion_24dp_icon_margin_start,
R.dimen.omnibox_suggestion_24dp_icon_margin_start));
}

/** Gets the start padding for a large omnibox suggestion decoration icon. */
public static @Px int getLargeIconStartPadding(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_36dp_icon_margin_start_smallest,
R.dimen.omnibox_suggestion_36dp_icon_margin_start,
R.dimen.omnibox_suggestion_36dp_icon_margin_start));
if (!OmniboxFeatures.shouldShowModernizeVisualUpdate(context)) {
return context.getResources().getDimensionPixelSize(
R.dimen.omnibox_suggestion_36dp_icon_margin_start);
}

return context.getResources().getDimensionPixelSize(selectMarginDimen(context,
R.dimen.omnibox_suggestion_36dp_icon_margin_start_smallest,
R.dimen.omnibox_suggestion_36dp_icon_margin_start,
R.dimen.omnibox_suggestion_36dp_icon_margin_start));
}

/** Gets the end padding for a large omnibox suggestion decoration icon. */
public static @Px int getLargeIconEndPadding(Context context) {
if (!OmniboxFeatures.shouldShowModernizeVisualUpdate(context)) {
return context.getResources().getDimensionPixelSize(
R.dimen.omnibox_suggestion_36dp_icon_margin_end);
}

return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_36dp_icon_margin_end_smallest,
selectMarginDimen(context, R.dimen.omnibox_suggestion_36dp_icon_margin_end_smallest,
R.dimen.omnibox_suggestion_36dp_icon_margin_end,
R.dimen.omnibox_suggestion_36dp_icon_margin_end));
}

/** Get the top margin for a suggestion that is the beginning of a group. */
public static int getSuggestionGroupTopMargin(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_group_vertical_margin,
selectMarginDimen(context, R.dimen.omnibox_suggestion_group_vertical_margin,
R.dimen.omnibox_suggestion_group_vertical_smaller_margin,
R.dimen.omnibox_suggestion_group_vertical_smallest_margin));
}
Expand All @@ -428,14 +442,14 @@ public static int getSuggestionGroupTopMargin(Context context) {
}

return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_carousel_suggestion_padding_smaller,
selectMarginDimen(context, R.dimen.omnibox_carousel_suggestion_padding_smaller,
R.dimen.omnibox_carousel_suggestion_padding_smallest,
R.dimen.omnibox_carousel_suggestion_padding_smaller));
}

/** Get the bottom padding for the MV carousel. */
public static @Px int getCarouselBottomPadding(Context context) {
if (OmniboxFeatures.shouldShowSmallerMargins()) {
if (OmniboxFeatures.shouldShowSmallerMargins(context)) {
return 0;
}

Expand All @@ -447,27 +461,64 @@ public static int getSuggestionGroupTopMargin(Context context) {
}

/** Get the top margin for first suggestion in the omnibox with "active color" enabled. */
public static int getActiveOmniboxTopSmallMargin(Context context) {
public static @Px int getActiveOmniboxTopSmallMargin(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_list_active_top_small_margin,
selectMarginDimen(context, R.dimen.omnibox_suggestion_list_active_top_small_margin,
R.dimen.omnibox_suggestion_list_active_top_smaller_margin,
R.dimen.omnibox_suggestion_list_active_top_small_margin));
}

/** Gets the start padding for a header suggestion. */
public static int getHeaderStartPadding(Context context) {
public static @Px int getHeaderStartPadding(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(R.dimen.omnibox_suggestion_header_padding_start_modern,
selectMarginDimen(context, R.dimen.omnibox_suggestion_header_padding_start_modern,
R.dimen.omnibox_suggestion_header_padding_start_modern_smaller,
R.dimen.omnibox_suggestion_header_padding_start_modern_smallest));
}

/**
* Returns the size of the spacer on the left side of the status view when the omnibox is
* focused.
*/
public static @Px int getFocusedStatusViewLeftSpacing(Context context) {
return context.getResources().getDimensionPixelSize(
selectMarginDimen(context, R.dimen.location_bar_status_view_left_space_width,
R.dimen.location_bar_status_view_left_space_width_bigger,
R.dimen.location_bar_status_view_left_space_width_bigger));
}

/**
* Returns the amount of pixels the toolbar should increased its height by when the omnibox is
* focused.
*/
public static @Px int getToolbarOnFocusHeightIncrease(Context context) {
if (!OmniboxFeatures.shouldShowModernizeVisualUpdate(context)) {
return 0;
}

return context.getResources().getDimensionPixelSize(
OmniboxFeatures.shouldShowActiveColorOnOmnibox()
? R.dimen.toolbar_url_focus_height_increase_active_color
: R.dimen.toolbar_url_focus_height_increase_no_active_color);
}

/** Returns the amount of pixels for the toolbar's side padding when the omnibox is focused. */
public static @Px int getToolbarSidePadding(Context context) {
return context.getResources().getDimensionPixelSize(
OmniboxFeatures.shouldShowModernizeVisualUpdate(context)
? OmniboxResourceProvider.selectMarginDimen(context,
R.dimen.toolbar_edge_padding_modern,
R.dimen.toolbar_edge_padding_modern_smaller,
R.dimen.toolbar_edge_padding)
: R.dimen.toolbar_edge_padding);
}

/** */
public static @DimenRes int selectMarginDimen(
@DimenRes int regular, @DimenRes int smaller, @DimenRes int smallest) {
if (OmniboxFeatures.shouldShowSmallestMargins()) {
Context context, @DimenRes int regular, @DimenRes int smaller, @DimenRes int smallest) {
if (OmniboxFeatures.shouldShowSmallestMargins(context)) {
return smallest;
} else if (OmniboxFeatures.shouldShowSmallerMargins()) {
} else if (OmniboxFeatures.shouldShowSmallerMargins(context)) {
return smaller;
}
return regular;
Expand Down
13 changes: 7 additions & 6 deletions .../src/org/chromium/chrome/browser/omnibox/suggestions/DropdownItemViewInfoListManager.java
View file
Expand Up @@ -111,7 +111,7 @@ void setSourceViewInfoList(
? SuggestionCommonProperties.FormFactor.TABLET
: SuggestionCommonProperties.FormFactor.PHONE;
DropdownItemViewInfo previousItem = null;
boolean useSmallestMargins = OmniboxFeatures.shouldShowSmallestMargins();
boolean useSmallestMargins = OmniboxFeatures.shouldShowSmallestMargins(mContext);
int groupTopMargin = OmniboxResourceProvider.getSuggestionGroupTopMargin(mContext);
int groupBottomMargin = mContext.getResources().getDimensionPixelSize(
R.dimen.omnibox_suggestion_group_vertical_smallest_margin);
Expand Down Expand Up @@ -168,7 +168,7 @@ void setSourceViewInfoList(

previousItemWasHeader = item.processor.getViewTypeId() == OmniboxSuggestionUiType.HEADER
&& shouldShowModernizeVisualUpdate
&& (useSmallestMargins || OmniboxFeatures.shouldShowSmallerMargins());
&& (useSmallestMargins || OmniboxFeatures.shouldShowSmallerMargins(mContext));

suggestionsList.add(item);
}
Expand All @@ -194,15 +194,16 @@ void setSourceViewInfoList(
*/
private @Px int getSuggestionListTopMargin(@OmniboxSuggestionUiType int firstSuggestionUiType) {
if (firstSuggestionUiType == OmniboxSuggestionUiType.EDIT_URL_SUGGESTION) {
return OmniboxFeatures.shouldShowSmallerMargins() ? mListActiveOmniboxTopSmallMargin
: 0;
return OmniboxFeatures.shouldShowSmallerMargins(mContext)
? mListActiveOmniboxTopSmallMargin
: 0;
}

if (OmniboxFeatures.shouldShowActiveColorOnOmnibox()) {
if (OmniboxFeatures.shouldShowSmallestMargins()) {
if (OmniboxFeatures.shouldShowSmallestMargins(mContext)) {
return 0;
} else if (OmniboxFeatures.shouldShowSmallBottomMargin()
|| OmniboxFeatures.shouldShowSmallerMargins()) {
|| OmniboxFeatures.shouldShowSmallerMargins(mContext)) {
return mListActiveOmniboxTopSmallMargin;
} else {
return mListActiveOmniboxTopBigMargin;
Expand Down
2 changes: 1 addition & 1 deletion ...rc/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownAdapter.java
View file
Expand Up @@ -37,7 +37,7 @@ public void onAttachedToRecyclerView(@NonNull RecyclerView view) {
super.onAttachedToRecyclerView(view);
mLayoutManager = view.getLayoutManager();
mSelectedItem = RecyclerView.NO_POSITION;
if (OmniboxFeatures.shouldShowSmallestMargins()) {
if (OmniboxFeatures.shouldShowSmallestMargins(view.getContext())) {
view.addItemDecoration(new SuggestionHorizontalDivider(view.getContext()));
}
}
Expand Down
98 changes: 98 additions & 0 deletions ...rg/chromium/chrome/browser/omnibox/suggestions/base/BaseSuggestionViewBinderUnitTest.java
View file
Expand Up @@ -469,4 +469,102 @@ public void applySelectableBackground_reuseConstantStateAcrossViews() {

Assert.assertEquals(state1, state2);
}

@Test
public void iconPadding() {
BaseSuggestionViewBinder.initializeDimensions(mActivity);
int startPadding =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_24dp_icon_margin_start);
int startPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);
int endPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_end);

SuggestionDrawableState smallState =
SuggestionDrawableState.Builder.forColor(0).setUseRoundedCorners(true).build();
mModel.set(BaseSuggestionViewProperties.ICON, smallState);
verify(mIconView).setPadding(startPadding, 0, startPadding, 0);

SuggestionDrawableState largeState = SuggestionDrawableState.Builder.forColor(0)
.setUseRoundedCorners(true)
.setLarge(true)
.build();
mModel.set(BaseSuggestionViewProperties.ICON, largeState);
verify(mIconView).setPadding(startPaddingLarge, 0, endPaddingLarge, 0);
}

@Test
@EnableFeatures(ChromeFeatureList.OMNIBOX_MODERNIZE_VISUAL_UPDATE)
public void iconStartPadding_smallestMarginsRevamp() {
OmniboxFeatures.MODERNIZE_VISUAL_UPDATE_SMALLEST_MARGINS.setForTesting(true);
BaseSuggestionViewBinder.initializeDimensions(mActivity);
int padding =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_24dp_icon_margin_start);
int startPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);
int endPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);

SuggestionDrawableState smallState =
SuggestionDrawableState.Builder.forColor(0).setUseRoundedCorners(true).build();
mModel.set(BaseSuggestionViewProperties.ICON, smallState);
verify(mIconView).setPadding(padding, 0, padding, 0);

SuggestionDrawableState largeState = SuggestionDrawableState.Builder.forColor(0)
.setUseRoundedCorners(true)
.setLarge(true)
.build();
mModel.set(BaseSuggestionViewProperties.ICON, largeState);
verify(mIconView).setPadding(startPaddingLarge, 0, endPaddingLarge, 0);
}

@Test
@EnableFeatures(ChromeFeatureList.OMNIBOX_MODERNIZE_VISUAL_UPDATE)
public void iconStartPadding_smallerMarginsRevamp() {
OmniboxFeatures.MODERNIZE_VISUAL_UPDATE_SMALLEST_MARGINS.setForTesting(true);
BaseSuggestionViewBinder.initializeDimensions(mActivity);
int startPadding =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_24dp_icon_margin_start);
int startPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);
int endPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);

SuggestionDrawableState smallState =
SuggestionDrawableState.Builder.forColor(0).setUseRoundedCorners(true).build();
mModel.set(BaseSuggestionViewProperties.ICON, smallState);
verify(mIconView).setPadding(startPadding, 0, startPadding, 0);

SuggestionDrawableState largeState = SuggestionDrawableState.Builder.forColor(0)
.setUseRoundedCorners(true)
.setLarge(true)
.build();
mModel.set(BaseSuggestionViewProperties.ICON, largeState);
verify(mIconView).setPadding(startPaddingLarge, 0, endPaddingLarge, 0);
}

@Test
@EnableFeatures(ChromeFeatureList.OMNIBOX_MODERNIZE_VISUAL_UPDATE)
@Config(qualifiers = "w600dp-h820dp")
public void iconStartPadding_tabletRevamp() {
BaseSuggestionViewBinder.initializeDimensions(mActivity);
int startPadding =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_24dp_icon_margin_start);
int startPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_start);
int endPaddingLarge =
mResources.getDimensionPixelSize(R.dimen.omnibox_suggestion_36dp_icon_margin_end);

SuggestionDrawableState smallState =
SuggestionDrawableState.Builder.forColor(0).setUseRoundedCorners(true).build();
mModel.set(BaseSuggestionViewProperties.ICON, smallState);
verify(mIconView).setPadding(startPadding, 0, startPadding, 0);

SuggestionDrawableState largeState = SuggestionDrawableState.Builder.forColor(0)
.setUseRoundedCorners(true)
.setLarge(true)
.build();
mModel.set(BaseSuggestionViewProperties.ICON, largeState);
verify(mIconView).setPadding(startPaddingLarge, 0, endPaddingLarge, 0);
}
}
1 change: 0 additions & 1 deletion ...nibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/base/SuggestionLayout.java
View file
Expand Up @@ -6,7 +6,6 @@

import android.content.Context;
import android.graphics.Rect;
import android.view.View.MeasureSpec;
import android.view.ViewGroup;

import androidx.annotation.IntDef;
Expand Down

0 comments on commit 75f6f25

Please sign in to comment.