cros: Make folder rename consistent with other renaming patterns.

In app list, the rename textfield is inconsistent with other renaming
patterns. This CL aims to make it consistent and improve it as well.
It makes the following changes:

- Change background color on hover.
- Change size of name textfield.
- Add border when name textfield is focused.
- When text field is pressed, highlight all text rather than position
  cursor.
- Changes underlying view width from max width to variable length,
  depending on size of current folder name.

Test: Manual
Bug: 1127498
Change-Id: I78727116737acdc261419255e3b8b132f79c3313
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406281Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807246}

ash/app_list/views/app_list_view_unittest.cc

@@ -1571,9 +1571,9 @@ TEST_P(AppListViewFocusTest, HittingLeftRightWhenFocusOnTextfield) {
   TestLeftAndRightKeyTraversalOnTextfield(search_box_view()->search_box());
 }
 
-// Tests that the focus is reset onto the search box and the folder exits after
-// hitting enter on folder name.
-TEST_P(AppListViewFocusTest, FocusResetAfterHittingEnterOnFolderName) {
+// Tests that the focus is reset and the folder does not exit after hitting
+// enter/escape on folder name.
+TEST_P(AppListViewFocusTest, FocusResetAfterHittingEnterOrEscapeOnFolderName) {
   Show();
 
   // Transition to FULLSCREEN_ALL_APPS state and open the folder.
@@ -1589,8 +1589,14 @@ TEST_P(AppListViewFocusTest, FocusResetAfterHittingEnterOnFolderName) {
 
   // Hit enter key.
   SimulateKeyPress(ui::VKEY_RETURN, false);
-  search_box_view()->search_box()->RequestFocus();
-  EXPECT_FALSE(contents_view()->apps_container_view()->IsInFolderView());
+  EXPECT_TRUE(contents_view()->apps_container_view()->IsInFolderView());
+  EXPECT_FALSE(folder_name_view->HasFocus());
+
+  // Refocus and hit escape key.
+  folder_name_view->RequestFocus();
+  SimulateKeyPress(ui::VKEY_ESCAPE, false);
+  EXPECT_TRUE(contents_view()->apps_container_view()->IsInFolderView());
+  EXPECT_FALSE(folder_name_view->HasFocus());
 }
 
 // Tests that the selection highlight follows the page change.

ash/app_list/views/folder_header_view.cc

@@ -20,6 +20,7 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/text_elider.h"
 #include "ui/strings/grit/ui_strings.h"
+#include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/textfield/textfield.h"
@@ -30,8 +31,7 @@ namespace ash {
 
 namespace {
 
-constexpr int kMaxFolderNameWidth = 204;
-constexpr SkColor kFolderNameColor = gfx::kGoogleGrey700;
+constexpr SkColor kFolderNameTextColor = gfx::kGoogleGrey700;
 constexpr SkColor kFolderTitleHintTextColor = gfx::kGoogleGrey600;
 
 }  // namespace
@@ -42,26 +42,51 @@ class FolderHeaderView::FolderNameView : public views::Textfield,
   explicit FolderNameView(FolderHeaderView* folder_header_view)
       : folder_header_view_(folder_header_view) {
     DCHECK(folder_header_view_);
-    SetBorder(views::CreateEmptyBorder(1, 1, 1, 1));
+    // Make folder name font size 14px.
+    SetFontList(
+        ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(2));
+    SetTextColor(kFolderNameTextColor);
+    set_placeholder_text_color(kFolderTitleHintTextColor);
+    SetNameViewBorderAndBackground(/*is_active=*/false);
     SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
   }
 
   ~FolderNameView() override = default;
 
   gfx::Size CalculatePreferredSize() const override {
-    return gfx::Size(kMaxFolderNameWidth,
+    return gfx::Size(AppListConfig::instance().folder_header_max_width(),
                      AppListConfig::instance().folder_header_height());
   }
 
+  void SetNameViewBorderAndBackground(bool is_active) {
+    int horizontal_padding = AppListConfig::instance().folder_name_padding();
+    SetBorder(views::CreatePaddedBorder(
+        views::CreateRoundedRectBorder(
+            AppListConfig::instance().folder_name_border_thickness(),
+            AppListConfig::instance().folder_name_border_radius(),
+            is_active ? AppListConfig::instance().folder_name_border_color()
+                      : SK_ColorTRANSPARENT),
+        gfx::Insets(0, horizontal_padding)));
+
+    SetBackgroundColor(
+        AppListConfig::instance().GetFolderNameBackgroundColor(is_active));
+  }
+
   void OnFocus() override {
+    SetNameViewBorderAndBackground(/*is_active=*/true);
     SetText(base::UTF8ToUTF16(folder_header_view_->folder_item_->name()));
     starting_name_ = GetText();
     folder_header_view_->previous_folder_name_ = starting_name_;
-    SelectAll(false);
+
+    if (!defer_select_all_)
+      SelectAll(false);
+
     Textfield::OnFocus();
   }
 
   void OnBlur() override {
+    SetNameViewBorderAndBackground(/*is_active=*/false);
+
     // Collapse whitespace when FolderNameView loses focus.
     folder_header_view_->ContentsChanged(
         this, base::CollapseWhitespace(GetText(), false));
@@ -81,15 +106,76 @@ class FolderHeaderView::FolderNameView : public views::Textfield,
       }
     }
 
+    defer_select_all_ = false;
+
     Textfield::OnBlur();
   }
 
+  bool DoesMouseEventActuallyIntersect(const ui::MouseEvent& event) {
+    // Since hitbox for this view is extended for tap, we need to manually
+    // calculate this when checking for mouse events.
+    return GetLocalBounds().Contains(event.location());
+  }
+
+  bool OnMousePressed(const ui::MouseEvent& event) override {
+    // Since hovering changes the background color, only taps should be
+    // triggered using the extended event target.
+    if (!DoesMouseEventActuallyIntersect(event))
+      return false;
+
+    return Textfield::OnMousePressed(event);
+  }
+
+  void OnMouseExited(const ui::MouseEvent& event) override {
+    if (!HasFocus()) {
+      SetBackgroundColor(AppListConfig::instance().GetFolderNameBackgroundColor(
+          /*is_active=*/false));
+    }
+
+    has_mouse_already_entered_ = false;
+  }
+
+  void OnMouseMoved(const ui::MouseEvent& event) override {
+    if (DoesMouseEventActuallyIntersect(event) && !has_mouse_already_entered_) {
+      // If this is reached, the mouse is entering the view.
+      // Recreate border to have custom corner radius.
+      SetBackground(views::CreateRoundedRectBackground(
+          AppListConfig::instance().GetFolderNameBackgroundColor(
+              /*is_active=*/true),
+          AppListConfig::instance().folder_name_border_radius()));
+      has_mouse_already_entered_ = true;
+    } else if (!DoesMouseEventActuallyIntersect(event) &&
+               has_mouse_already_entered_ && !HasFocus()) {
+      // If this is reached, the mouse is exiting the view on its horizontal
+      // edges.
+      SetBackgroundColor(AppListConfig::instance().GetFolderNameBackgroundColor(
+          /*is_active=*/false));
+      has_mouse_already_entered_ = false;
+    }
+  }
+
+  void OnMouseReleased(const ui::MouseEvent& event) override {
+    if (defer_select_all_) {
+      defer_select_all_ = false;
+
+      if (!HasSelection())
+        SelectAll(false);
+    }
+  }
+
   bool DoesIntersectRect(const views::View* target,
                          const gfx::Rect& rect) const override {
     DCHECK_EQ(target, this);
     gfx::Rect textfield_bounds = target->GetLocalBounds();
-    int horizontal_padding = -(textfield_bounds.height() * 1.5);
+
+    // Ensure that the tap target for this view is always at least the view's
+    // minimum width.
+    int min_width =
+        std::max(AppListConfig::instance().folder_header_min_tap_width(),
+                 textfield_bounds.width());
+    int horizontal_padding = -((min_width - textfield_bounds.width()) / 2);
     textfield_bounds.Inset(gfx::Insets(0, horizontal_padding));
+
     return textfield_bounds.Intersects(rect);
   }
 
@@ -101,6 +187,18 @@ class FolderHeaderView::FolderNameView : public views::Textfield,
   // rename metric.
   base::string16 starting_name_;
 
+  // If the view is focused via a mouse press event, then selection will be
+  // cleared by its mouse release. To address this, defer selecting all
+  // until we receive mouse release.
+  bool defer_select_all_ = false;
+
+  // Because of this view's custom event target, this view receives mouse enter
+  // events in areas where the view isn't actually occupying. To check whether a
+  // user has entered/exited this, we must check every mouse move event. This
+  // bool tracks whether the mouse has entered the view, avoiding repainting the
+  // background on each mouse move event.
+  bool has_mouse_already_entered_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(FolderNameView);
 };
 
@@ -113,15 +211,7 @@ FolderHeaderView::FolderHeaderView(FolderHeaderViewDelegate* delegate)
       delegate_(delegate),
       folder_name_visible_(true),
       is_tablet_mode_(false) {
-  folder_name_view_->set_placeholder_text_color(kFolderTitleHintTextColor);
   folder_name_view_->SetPlaceholderText(folder_name_placeholder_text_);
-  folder_name_view_->SetBorder(views::NullBorder());
-
-  // Make folder name font size 14px.
-  folder_name_view_->SetFontList(
-      ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(2));
-  folder_name_view_->SetBackgroundColor(SK_ColorTRANSPARENT);
-  folder_name_view_->SetTextColor(kFolderNameColor);
   folder_name_view_->set_controller(this);
   AddChildView(folder_name_view_);
 
@@ -206,7 +296,7 @@ bool FolderHeaderView::IsFolderNameEnabledForTest() const {
 }
 
 gfx::Size FolderHeaderView::CalculatePreferredSize() const {
-  return gfx::Size(kMaxFolderNameWidth,
+  return gfx::Size(AppListConfig::instance().folder_header_max_width(),
                    folder_name_view_->GetPreferredSize().height());
 }
 
@@ -223,7 +313,7 @@ views::View* FolderHeaderView::GetFolderNameViewForTest() const {
 }
 
 int FolderHeaderView::GetMaxFolderNameWidth() const {
-  return kMaxFolderNameWidth;
+  return AppListConfig::instance().folder_header_max_width();
 }
 
 base::string16 FolderHeaderView::GetElidedFolderName(
@@ -247,20 +337,20 @@ void FolderHeaderView::Layout() {
     return;
 
   gfx::Rect text_bounds(rect);
-  base::string16 text = folder_item_ && !folder_item_->name().empty()
-                            ? base::UTF8ToUTF16(folder_item_->name())
-                            : folder_name_placeholder_text_;
+
+  base::string16 text = folder_name_view_->GetText().empty()
+                            ? folder_name_placeholder_text_
+                            : folder_name_view_->GetText();
   int text_width =
       gfx::Canvas::GetStringWidth(text, folder_name_view_->GetFontList()) +
       folder_name_view_->GetCaretBounds().width() +
       folder_name_view_->GetInsets().width();
-  text_width = std::min(text_width, GetMaxFolderNameWidth());
+  text_width =
+      std::min(text_width, AppListConfig::instance().folder_header_max_width());
+  text_width =
+      std::max(text_width, AppListConfig::instance().folder_header_min_width());
   text_bounds.set_x(std::max(0, rect.x() + (rect.width() - text_width) / 2));
-
-  // The width of the text field should always be the maximum length possible,
-  // to prevent the touch target from resizing with the text. The width should
-  // also stay within the FolderHeaderView bounds.
-  text_bounds.set_width(std::min(rect.width(), GetMaxFolderNameWidth()));
+  text_bounds.set_width(std::min(rect.width(), text_width));
 
   text_bounds.ClampToCenteredSize(gfx::Size(
       text_bounds.width(), folder_name_view_->GetPreferredSize().height()));
@@ -292,12 +382,16 @@ void FolderHeaderView::ContentsChanged(views::Textfield* sender,
   Layout();
 }
 
+bool FolderHeaderView::ShouldNameViewClearFocus(const ui::KeyEvent& key_event) {
+  return key_event.type() == ui::ET_KEY_PRESSED &&
+         (key_event.key_code() == ui::VKEY_RETURN ||
+          key_event.key_code() == ui::VKEY_ESCAPE);
+}
+
 bool FolderHeaderView::HandleKeyEvent(views::Textfield* sender,
                                       const ui::KeyEvent& key_event) {
-  if (key_event.key_code() == ui::VKEY_RETURN &&
-      key_event.type() == ui::ET_KEY_PRESSED) {
-    delegate_->GiveBackFocusToSearchBox();
-    delegate_->NavigateBack(folder_item_, key_event);
+  if (ShouldNameViewClearFocus(key_event)) {
+    folder_name_view_->GetFocusManager()->ClearFocus();
     return true;
   }
   if (!IsUnhandledLeftRightKeyEvent(key_event))

ash/app_list/views/folder_header_view.h

@@ -68,6 +68,10 @@ class APP_LIST_EXPORT FolderHeaderView : public views::View,
   // Returns elided folder name from |folder_name|.
   base::string16 GetElidedFolderName(const base::string16& folder_name) const;
 
+  // Returns whether |folder_name_view_| should clear focus based on
+  // |key_event_|.
+  bool ShouldNameViewClearFocus(const ui::KeyEvent& key_event);
+
   // views::View:
   void Layout() override;
 

ash/app_list/views/folder_header_view_unittest.cc

@@ -147,7 +147,7 @@ TEST_F(FolderHeaderViewTest, WhitespaceCollapsedWhenFolderNameViewLosesFocus) {
   EXPECT_EQ("N A", delegate_->folder_name());
 }
 
-TEST_F(FolderHeaderViewTest, MaxFoldernNameLength) {
+TEST_F(FolderHeaderViewTest, MaxFolderNameLength) {
   // Creating a folder with empty folder name.
   AppListFolderItem* folder_item = model_->CreateAndPopulateFolderWithApps(2);
   folder_header_view_->SetFolderItem(folder_item);
@@ -205,34 +205,57 @@ void SendTap(GestureHandler* handler, const gfx::Point& location) {
   handler->OnGestureEvent(&tap_up);
 }
 
+template <typename EventHandler>
+void SendPress(EventHandler* handler, const gfx::Point& location) {
+  ui::MouseEvent press_down(ui::ET_MOUSE_PRESSED,
+                            gfx::PointF(location.x(), location.y()),
+                            gfx::PointF(0, 0), base::TimeTicks::Now(), 0, 0);
+  handler->OnMouseEvent(&press_down);
+  ui::MouseEvent press_up(ui::ET_MOUSE_RELEASED,
+                          gfx::PointF(location.x(), location.y()),
+                          gfx::PointF(0, 0), base::TimeTicks::Now(), 0, 0);
+  handler->OnMouseEvent(&press_up);
+}
+
 }  // namespace
 
-// Tests that folder name textfield is triggered when user touches on or near
-// the folder name. (see https://crbug.com/997364)
+// Tests that when folder name is small, the folder name textfield is triggered
+// by only tap when on the textfieldd or near it to the left/right.
 TEST_F(FolderHeaderViewTest, TriggerFolderRenameAfterTappingNearFolderName) {
-  // Creating a folder with empty folder name.
+  // Create a folder with a small name.
   AppListFolderItem* folder_item = model_->CreateAndPopulateFolderWithApps(2);
   folder_header_view_->SetFolderItem(folder_item);
+  UpdateFolderName("ab");
 
   // Get in screen bounds of folder name
-  const gfx::Rect name_view_bounds =
-      folder_header_view_->GetFolderNameViewForTest()->GetBoundsInScreen();
+  views::View* name_view = folder_header_view_->GetFolderNameViewForTest();
+  const gfx::Rect name_view_bounds = name_view->GetBoundsInScreen();
 
   // Tap folder name and check that folder renaming is triggered.
-  SendTap(folder_header_view_->GetFolderNameViewForTest(),
-          name_view_bounds.CenterPoint());
+  SendTap(name_view, name_view_bounds.CenterPoint());
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(folder_header_view_->GetFolderNameViewForTest()->HasFocus());
+  EXPECT_TRUE(name_view->HasFocus());
 
   // Clear focus from the folder name.
   widget_->GetFocusManager()->ClearFocus();
-  ASSERT_FALSE(folder_header_view_->GetFolderNameViewForTest()->HasFocus());
+  ASSERT_FALSE(name_view->HasFocus());
 
   // Test that tapping near (but not directly on) the folder name still
   // triggers folder rename.
   // Tap folder name and check that folder renaming is triggered.
-  SendTap(widget_.get(), name_view_bounds.top_right());
-  EXPECT_TRUE(folder_header_view_->GetFolderNameViewForTest()->HasFocus());
+  gfx::Point right_of_name_view = name_view_bounds.right_center();
+  right_of_name_view.Offset(2, 0);
+  SendTap(widget_.get(), right_of_name_view);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(name_view->HasFocus());
+
+  // Clear focus from the folder name.
+  widget_->GetFocusManager()->ClearFocus();
+  ASSERT_FALSE(name_view->HasFocus());
+
+  // Test that clicking in the same spot won't trigger folder rename.
+  SendPress(widget_.get(), right_of_name_view);
+  EXPECT_FALSE(name_view->HasFocus());
 }
 
 }  // namespace test

ash/public/cpp/app_list/app_list_config.cc

@@ -293,7 +293,16 @@ AppListConfig::AppListConfig(AppListConfigType type)
       expand_arrow_tile_height_(72),
       folder_bubble_radius_(FolderUnclippedIconDimensionForType(type) / 2),
       folder_bubble_y_offset_(0),
-      folder_header_height_(20),
+      folder_header_height_(32),
+      folder_header_min_width_(24),
+      folder_header_max_width_(200),
+      folder_header_min_tap_width_(32),
+      folder_name_border_radius_(4),
+      folder_name_border_thickness_(2),
+      folder_name_padding_(8),
+      folder_name_border_color_(gfx::kGoogleBlue600),
+      folder_name_background_color_(SK_ColorTRANSPARENT),
+      folder_name_background_color_active_(gfx::kGoogleGrey100),
       folder_icon_dimension_(FolderClippedIconDimensionForType(type)),
       folder_unclipped_icon_dimension_(
           FolderUnclippedIconDimensionForType(type)),
@@ -305,7 +314,7 @@ AppListConfig::AppListConfig(AppListConfigType type)
       item_icon_in_folder_icon_margin_(ItemIconInFolderIconMarginForType(type)),
       folder_dropping_circle_radius_(folder_bubble_radius_),
       folder_dropping_delay_(0),
-      folder_background_color_(gfx::kGoogleGrey100),
+      folder_background_color_(SK_ColorWHITE),
       page_flip_zone_size_(20),
       grid_tile_spacing_in_folder_(8),
       blur_radius_(30),
@@ -420,6 +429,16 @@ AppListConfig::AppListConfig(const AppListConfig& base_config,
                                      inner_tile_scale_y)),
       folder_bubble_y_offset_(base_config.folder_bubble_y_offset_),
       folder_header_height_(base_config.folder_header_height_),
+      folder_header_min_width_(base_config.folder_header_min_width_),
+      folder_header_max_width_(base_config.folder_header_max_width_),
+      folder_header_min_tap_width_(base_config.folder_header_min_tap_width_),
+      folder_name_border_radius_(base_config.folder_name_border_radius_),
+      folder_name_border_thickness_(base_config.folder_name_border_thickness_),
+      folder_name_padding_(base_config.folder_name_padding_),
+      folder_name_border_color_(base_config.folder_name_border_color_),
+      folder_name_background_color_(base_config.folder_name_background_color_),
+      folder_name_background_color_active_(
+          base_config.folder_name_background_color_active_),
       folder_icon_dimension_(MinScale(base_config.folder_icon_dimension_,
                                       scale_x,
                                       inner_tile_scale_y)),
@@ -534,4 +553,9 @@ SkColor AppListConfig::GetCardifiedBackgroundColor(bool is_active) const {
                    : cardified_background_color_;
 }
 
+SkColor AppListConfig::GetFolderNameBackgroundColor(bool is_active) const {
+  return is_active ? folder_name_background_color_active_
+                   : folder_name_background_color_;
+}
+
 }  // namespace ash

ash/public/cpp/app_list/app_list_config.h

@@ -116,6 +116,17 @@ class ASH_PUBLIC_EXPORT AppListConfig {
   int folder_bubble_radius() const { return folder_bubble_radius_; }
   int folder_bubble_y_offset() const { return folder_bubble_y_offset_; }
   int folder_header_height() const { return folder_header_height_; }
+  int folder_header_min_width() const { return folder_header_min_width_; }
+  int folder_header_max_width() const { return folder_header_max_width_; }
+  int folder_header_min_tap_width() const {
+    return folder_header_min_tap_width_;
+  }
+  int folder_name_border_radius() const { return folder_name_border_radius_; }
+  int folder_name_border_thickness() const {
+    return folder_name_border_thickness_;
+  }
+  int folder_name_padding() const { return folder_name_padding_; }
+  SkColor folder_name_border_color() const { return folder_name_border_color_; }
   int folder_icon_dimension() const { return folder_icon_dimension_; }
   int folder_unclipped_icon_dimension() const {
     return folder_unclipped_icon_dimension_;
@@ -253,6 +264,9 @@ class ASH_PUBLIC_EXPORT AppListConfig {
   // Returns the color and opacity for the page background.
   SkColor GetCardifiedBackgroundColor(bool is_active) const;
 
+  // Returns the folder name background color.
+  SkColor GetFolderNameBackgroundColor(bool is_active) const;
+
  private:
   const AppListConfigType type_;
 
@@ -374,6 +388,29 @@ class ASH_PUBLIC_EXPORT AppListConfig {
   // The height of the in folder name and pagination buttons.
   const int folder_header_height_;
 
+  // The min and max widths of the folder name.
+  const int folder_header_min_width_;
+  const int folder_header_max_width_;
+
+  // The min width of folder name for tap events.
+  const int folder_header_min_tap_width_;
+
+  // The border radius for folder name.
+  const int folder_name_border_radius_;
+
+  // The border thickness for folder name.
+  const int folder_name_border_thickness_;
+
+  // The inner padding for folder name.
+  const int folder_name_padding_;
+
+  // The color of the folder name border.
+  const SkColor folder_name_border_color_;
+
+  // Background colors for folder name.
+  const SkColor folder_name_background_color_;
+  const SkColor folder_name_background_color_active_;
+
   // The icon dimension of folder.
   const int folder_icon_dimension_;