app_list: Fix image button placement and color

This patch fixes the placement and color of image buttons in
search_box_view. Previously, the right padding was not calculated and
close button was using search box color.

Bug: 941524
Test: manual
Change-Id: I201b21257a11e188490136b05ac68dc5e15cebcb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626735Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662909}

ash/app_list/views/search_box_view.cc

@@ -66,8 +66,6 @@ constexpr int kSearchBoxFocusRingPadding = 4;
 
 constexpr SkColor kSearchBoxFocusRingColor = gfx::kGoogleBlue300;
 
-constexpr int kAssistantIconSize = 24;
-constexpr int kCloseIconSize = 24;
 constexpr int kSearchBoxFocusRingCornerRadius = 28;
 
 // Range of the fraction of app list from collapsed to peeking that search box
@@ -181,8 +179,8 @@ void SearchBoxView::UpdateSearchIcon() {
   const gfx::VectorIcon& icon = search_model_->search_engine_is_google()
                                     ? google_icon
                                     : kSearchEngineNotGoogleIcon;
-  SetSearchIconImage(gfx::CreateVectorIcon(icon, search_box::kSearchIconSize,
-                                           search_box_color()));
+  SetSearchIconImage(
+      gfx::CreateVectorIcon(icon, search_box::kIconSize, search_box_color()));
 }
 
 void SearchBoxView::UpdateSearchBoxBorder() {
@@ -216,9 +214,10 @@ int SearchBoxView::GetFocusRingSpacing() {
 
 void SearchBoxView::SetupCloseButton() {
   views::ImageButton* close = close_button();
-  close->SetImage(views::ImageButton::STATE_NORMAL,
-                  gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
-                                        search_box_color()));
+  close->SetImage(
+      views::ImageButton::STATE_NORMAL,
+      gfx::CreateVectorIcon(views::kIcCloseIcon, search_box::kIconSize,
+                            gfx::kGoogleGrey700));
   close->SetVisible(false);
   base::string16 close_button_label(
       l10n_util::GetStringUTF16(IDS_APP_LIST_CLEAR_SEARCHBOX));
@@ -327,8 +326,11 @@ void SearchBoxView::UpdateLayout(double progress,
   const int horizontal_spacing = gfx::Tween::LinearIntValueBetween(
       progress, GetBoxLayoutPaddingForState(current_state),
       GetBoxLayoutPaddingForState(target_state));
+  const int horizontal_right_padding =
+      horizontal_spacing -
+      (search_box::kButtonSizeDip - search_box::kIconSize) / 2;
   box_layout()->set_inside_border_insets(
-      gfx::Insets(0, horizontal_spacing, 0, 0));
+      gfx::Insets(0, horizontal_spacing, 0, horizontal_right_padding));
   box_layout()->set_between_child_spacing(horizontal_spacing);
   if (show_assistant_button()) {
     assistant_button()->layer()->SetOpacity(gfx::Tween::LinearIntValueBetween(
@@ -452,10 +454,6 @@ void SearchBoxView::OnWallpaperProminentColorsReceived(
   SetSearchBoxColor(
       prominent_colors[static_cast<int>(ColorProfileType::DARK_MUTED)]);
   UpdateSearchIcon();
-  close_button()->SetImage(
-      views::Button::STATE_NORMAL,
-      gfx::CreateVectorIcon(views::kIcCloseIcon, kCloseIconSize,
-                            search_box_color()));
   search_box()->set_placeholder_text_color(search_box_color());
   UpdateBackgroundColor(search_box::kSearchBoxBackgroundDefault);
   SchedulePaint();
@@ -747,7 +745,7 @@ void SearchBoxView::SetupAssistantButton() {
       views::ImageButton::STATE_NORMAL,
       gfx::CreateVectorIcon(
           embedded_assistant ? ash::kAssistantMicIcon : ash::kAssistantIcon,
-          kAssistantIconSize, search_box_color()));
+          search_box::kIconSize, gfx::kGoogleGrey700));
   base::string16 assistant_button_label(l10n_util::GetStringUTF16(
       embedded_assistant ? IDS_APP_LIST_START_ASSISTANT_VOICE_QUERY
                          : IDS_APP_LIST_START_ASSISTANT));

ash/app_list/views/search_box_view_unittest.cc

@@ -224,7 +224,7 @@ TEST_F(SearchBoxViewTest, SearchBoxInactiveSearchBoxGoogle) {
   SetSearchEngineIsGoogle(true);
   SetSearchBoxActive(false, ui::ET_UNKNOWN);
   const gfx::ImageSkia expected_icon =
-      gfx::CreateVectorIcon(kGoogleBlackIcon, search_box::kSearchIconSize,
+      gfx::CreateVectorIcon(kGoogleBlackIcon, search_box::kIconSize,
                             search_box::kDefaultSearchboxColor);
   view()->ModelChanged();
 
@@ -240,7 +240,7 @@ TEST_F(SearchBoxViewTest, SearchBoxActiveSearchEngineGoogle) {
   SetSearchEngineIsGoogle(true);
   SetSearchBoxActive(true, ui::ET_MOUSE_PRESSED);
   const gfx::ImageSkia expected_icon =
-      gfx::CreateVectorIcon(kGoogleColorIcon, search_box::kSearchIconSize,
+      gfx::CreateVectorIcon(kGoogleColorIcon, search_box::kIconSize,
                             search_box::kDefaultSearchboxColor);
   view()->ModelChanged();
 
@@ -255,9 +255,9 @@ TEST_F(SearchBoxViewTest, SearchBoxActiveSearchEngineGoogle) {
 TEST_F(SearchBoxViewTest, SearchBoxInactiveSearchEngineNotGoogle) {
   SetSearchEngineIsGoogle(false);
   SetSearchBoxActive(false, ui::ET_UNKNOWN);
-  const gfx::ImageSkia expected_icon = gfx::CreateVectorIcon(
-      kSearchEngineNotGoogleIcon, search_box::kSearchIconSize,
-      search_box::kDefaultSearchboxColor);
+  const gfx::ImageSkia expected_icon =
+      gfx::CreateVectorIcon(kSearchEngineNotGoogleIcon, search_box::kIconSize,
+                            search_box::kDefaultSearchboxColor);
   view()->ModelChanged();
 
   const gfx::ImageSkia actual_icon =
@@ -271,9 +271,9 @@ TEST_F(SearchBoxViewTest, SearchBoxInactiveSearchEngineNotGoogle) {
 TEST_F(SearchBoxViewTest, SearchBoxActiveSearchEngineNotGoogle) {
   SetSearchEngineIsGoogle(false);
   SetSearchBoxActive(true, ui::ET_UNKNOWN);
-  const gfx::ImageSkia expected_icon = gfx::CreateVectorIcon(
-      kSearchEngineNotGoogleIcon, search_box::kSearchIconSize,
-      search_box::kDefaultSearchboxColor);
+  const gfx::ImageSkia expected_icon =
+      gfx::CreateVectorIcon(kSearchEngineNotGoogleIcon, search_box::kIconSize,
+                            search_box::kDefaultSearchboxColor);
   view()->ModelChanged();
 
   const gfx::ImageSkia actual_icon =

ui/chromeos/search_box/search_box_constants.h

@@ -30,8 +30,11 @@ SEARCH_BOX_EXPORT constexpr int kSearchBoxBorderCornerRadiusSearchResult = 20;
 // Preferred height of search box.
 SEARCH_BOX_EXPORT constexpr int kSearchBoxPreferredHeight = 48;
 
-// The size of the search icon in the search box.
-SEARCH_BOX_EXPORT constexpr int kSearchIconSize = 24;
+// The size of the icon in the search box.
+SEARCH_BOX_EXPORT constexpr int kIconSize = 24;
+
+// The size of the image button in the search box.
+SEARCH_BOX_EXPORT constexpr int kButtonSizeDip = 40;
 
 }  // namespace search_box
 

ui/chromeos/search_box/search_box_view_base.cc

@@ -39,8 +39,6 @@ namespace {
 
 constexpr int kInnerPadding = 16;
 
-constexpr int kButtonSizeDip = 48;
-
 // Preferred width of search box.
 constexpr int kSearchBoxPreferredWidth = 544;
 
@@ -282,7 +280,7 @@ SearchBoxViewBase::SearchBoxViewBase(SearchBoxViewDelegate* delegate)
 
   // An invisible space view to align |search_box_| to center.
   search_box_right_space_ = new views::View();
-  search_box_right_space_->SetPreferredSize(gfx::Size(kSearchIconSize, 0));
+  search_box_right_space_->SetPreferredSize(gfx::Size(kIconSize, 0));
   content_container_->AddChildView(search_box_right_space_);
 
   assistant_button_ = new SearchBoxImageButton(this);