Update holding space styles for focused and selected states.

Focused state:
- Blue ring drawn outside of view.

Selected state:
- Blue ring drawn at border of view.
- Semi-transparent blue overlay drawn over view.

This CL also updates size of the pin icon.

Bug: 1131267
Change-Id: Ia3e45e0c42cbe99e40893ab5efc2e085eb91f2af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434766
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811486}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -25,11 +25,13 @@ constexpr int kHoldingSpaceChipsPerRow = 2;
 constexpr int kHoldingSpaceColumnSpacing = 8;
 constexpr int kHoldingSpaceColumnWidth = 160;
 constexpr int kHoldingSpaceCornerRadius = 8;
+constexpr int kHoldingSpacePinIconSize = 20;
 constexpr int kHoldingSpaceRowSpacing = 8;
 constexpr gfx::Insets kHoldingSpaceScreenshotPadding(8);
 constexpr int kHoldingSpaceScreenshotSpacing = 8;
 constexpr gfx::Size kHoldingSpaceScreenshotSize(104, 80);
 constexpr gfx::Insets kHoldingSpaceScreenshotsContainerPadding(8, 0);
+constexpr float kHoldingSpaceSelectedOverlayOpacity = 0.24f;
 constexpr int kHoldingSpaceTrayIconMainAxisMargin = 6;
 
 // Context menu commands.

ash/system/holding_space/holding_space_item_chip_view.cc

@@ -6,16 +6,11 @@
 
 #include "ash/public/cpp/holding_space/holding_space_constants.h"
 #include "ash/public/cpp/holding_space/holding_space_item.h"
-#include "ash/public/cpp/shelf_config.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/system/holding_space/holding_space_item_view.h"
 #include "ash/system/tray/tray_popup_item_style.h"
 #include "ash/system/tray/tray_popup_utils.h"
 #include "ash/system/user/rounded_image_view.h"
-#include "ui/base/dragdrop/drag_drop_types.h"
-#include "ui/views/animation/flood_fill_ink_drop_ripple.h"
-#include "ui/views/animation/ink_drop_impl.h"
-#include "ui/views/background.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
@@ -42,16 +37,6 @@ HoldingSpaceItemChipView::HoldingSpaceItemChipView(
   TrayPopupItemStyle(TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL)
       .SetupLabel(label_);
 
-  const auto* color_provider = AshColorProvider::Get();
-  SetBackground(views::CreateRoundedRectBackground(
-      color_provider->GetControlsLayerColor(
-          AshColorProvider::ControlsLayerType::kControlBackgroundColorInactive),
-      kHoldingSpaceCornerRadius));
-
-  SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER);
-  SetInkDropVisibleOpacity(
-      color_provider->GetRippleAttributes().inkdrop_opacity);
-
   // Subscribe to be notified of changes to `item_`'s image.
   image_subscription_ =
       item->image().AddImageSkiaChangedCallback(base::BindRepeating(
@@ -59,7 +44,7 @@ HoldingSpaceItemChipView::HoldingSpaceItemChipView(
 
   UpdateImage();
 
-  AddPin(this /*parent*/);
+  AddPin(/*parent=*/this);
 }
 
 HoldingSpaceItemChipView::~HoldingSpaceItemChipView() = default;

ash/system/holding_space/holding_space_item_view.cc

@@ -14,13 +14,16 @@
 #include "ash/system/holding_space/holding_space_item_view_delegate.h"
 #include "base/bind.h"
 #include "ui/base/class_property.h"
+#include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/accessibility/view_accessibility.h"
+#include "ui/views/background.h"
 #include "ui/views/controls/button/image_button.h"
-#include "ui/views/controls/focus_ring.h"
 #include "ui/views/controls/highlight_path_generator.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
+#include "ui/views/painter.h"
+#include "ui/views/style/platform_style.h"
 #include "ui/views/vector_icons.h"
 
 namespace ash {
@@ -32,6 +35,37 @@ namespace {
 // overridden by subclasses.
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kIsHoldingSpaceItemViewProperty, false)
 
+// CallbackPainter -------------------------------------------------------------
+
+// A painter which delegates painting to a callback.
+class CallbackPainter : public views::Painter {
+ public:
+  using Callback = base::RepeatingCallback<void(gfx::Canvas*, gfx::Size)>;
+
+  CallbackPainter(const CallbackPainter&) = delete;
+  CallbackPainter& operator=(const CallbackPainter&) = delete;
+  ~CallbackPainter() override = default;
+
+  // Creates a painted layer which delegates painting to `callback`.
+  static std::unique_ptr<ui::LayerOwner> CreatePaintedLayer(Callback callback) {
+    auto owner = views::Painter::CreatePaintedLayer(
+        base::WrapUnique(new CallbackPainter(callback)));
+    owner->layer()->SetFillsBoundsOpaquely(false);
+    return owner;
+  }
+
+ private:
+  explicit CallbackPainter(Callback callback) : callback_(callback) {}
+
+  // views::Painter:
+  gfx::Size GetMinimumSize() const override { return gfx::Size(); }
+  void Paint(gfx::Canvas* canvas, const gfx::Size& size) override {
+    callback_.Run(canvas, size);
+  }
+
+  Callback callback_;
+};
+
 }  // namespace
 
 // HoldingSpaceItemView --------------------------------------------------------
@@ -47,24 +81,39 @@ HoldingSpaceItemView::HoldingSpaceItemView(
 
   SetNotifyEnterExitOnChild(true);
 
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
-
+  // Accessibility.
   GetViewAccessibility().OverrideName(item->text());
+  GetViewAccessibility().OverrideRole(ax::mojom::Role::kButton);
 
-  // Install the selection ring before installing the focus ring so that the
-  // selection ring will paint beneath the focus ring.
-  views::FocusRing* selection_ring = views::FocusRing::Install(this);
-  selection_ring->SetColor(gfx::kPlaceholderColor);
-  selection_ring->SetHasFocusPredicate(
-      [this](views::View* selection_ring) { return this->selected(); });
+  // Background.
+  SetBackground(views::CreateRoundedRectBackground(
+      AshColorProvider::Get()->GetControlsLayerColor(
+          AshColorProvider::ControlsLayerType::kControlBackgroundColorInactive),
+      kHoldingSpaceCornerRadius));
 
-  SetFocusBehavior(FocusBehavior::ALWAYS);
-  views::FocusRing* focus_ring = views::FocusRing::Install(this);
-  focus_ring->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
+  // Layer.
+  SetPaintToLayer();
+  layer()->SetFillsBoundsOpaquely(false);
 
-  // The selection ring, focus ring, and ink drop layers should match the corner
-  // radius of this view. Installation of a highlight path generator does this.
+  // Focus.
+  SetFocusBehavior(FocusBehavior::ALWAYS);
+  focused_layer_owner_ =
+      CallbackPainter::CreatePaintedLayer(base::BindRepeating(
+          &HoldingSpaceItemView::OnPaintFocus, base::Unretained(this)));
+  layer()->Add(focused_layer_owner_->layer());
+
+  // Selection.
+  selected_layer_owner_ =
+      CallbackPainter::CreatePaintedLayer(base::BindRepeating(
+          &HoldingSpaceItemView::OnPaintSelect, base::Unretained(this)));
+  layer()->Add(selected_layer_owner_->layer());
+
+  // Ink drop.
+  SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER);
+  SetInkDropVisibleOpacity(
+      AshColorProvider::Get()->GetRippleAttributes().inkdrop_opacity);
+
+  // Ink drop layers should match the corner radius of this view.
   views::InstallRoundRectHighlightPathGenerator(this, gfx::Insets(),
                                                 kHoldingSpaceCornerRadius);
 
@@ -85,6 +134,32 @@ SkColor HoldingSpaceItemView::GetInkDropBaseColor() const {
   return AshColorProvider::Get()->GetRippleAttributes().base_color;
 }
 
+void HoldingSpaceItemView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  gfx::Rect bounds = GetLocalBounds();
+  selected_layer_owner_->layer()->SetBounds(bounds);
+  selected_layer_owner_->layer()->SchedulePaint(
+      selected_layer_owner_->layer()->bounds());
+
+  // The focus ring is painted just outside the bounds for this view.
+  const float kFocusInsets =
+      -2.f - (views::PlatformStyle::kFocusHaloThickness / 2.f);
+
+  bounds.Inset(gfx::Insets(kFocusInsets));
+  focused_layer_owner_->layer()->SetBounds(bounds);
+  focused_layer_owner_->layer()->SchedulePaint(
+      focused_layer_owner_->layer()->bounds());
+}
+
+void HoldingSpaceItemView::OnFocus() {
+  focused_layer_owner_->layer()->SchedulePaint(
+      focused_layer_owner_->layer()->bounds());
+}
+
+void HoldingSpaceItemView::OnBlur() {
+  focused_layer_owner_->layer()->SchedulePaint(
+      focused_layer_owner_->layer()->bounds());
+}
+
 void HoldingSpaceItemView::OnGestureEvent(ui::GestureEvent* event) {
   delegate_->OnHoldingSpaceItemViewGestureEvent(this, *event);
 }
@@ -118,7 +193,9 @@ void HoldingSpaceItemView::SetSelected(bool selected) {
     return;
 
   selected_ = selected;
-  InvalidateLayout();
+
+  selected_layer_owner_->layer()->SchedulePaint(
+      selected_layer_owner_->layer()->bounds());
 }
 
 void HoldingSpaceItemView::AddPin(views::View* parent) {
@@ -130,10 +207,10 @@ void HoldingSpaceItemView::AddPin(views::View* parent) {
   const SkColor icon_color = AshColorProvider::Get()->GetContentLayerColor(
       AshColorProvider::ContentLayerType::kSystemMenuIconColor);
 
-  const gfx::ImageSkia unpinned_icon =
-      gfx::CreateVectorIcon(views::kUnpinIcon, icon_color);
-  const gfx::ImageSkia pinned_icon =
-      gfx::CreateVectorIcon(views::kPinIcon, icon_color);
+  const gfx::ImageSkia unpinned_icon = gfx::CreateVectorIcon(
+      views::kUnpinIcon, kHoldingSpacePinIconSize, icon_color);
+  const gfx::ImageSkia pinned_icon = gfx::CreateVectorIcon(
+      views::kPinIcon, kHoldingSpacePinIconSize, icon_color);
 
   pin_->SetImage(views::Button::STATE_NORMAL, unpinned_icon);
   pin_->SetToggledImage(views::Button::STATE_NORMAL, &pinned_icon);
@@ -141,6 +218,47 @@ void HoldingSpaceItemView::AddPin(views::View* parent) {
                                          base::Unretained(this)));
 }
 
+void HoldingSpaceItemView::OnPaintFocus(gfx::Canvas* canvas, gfx::Size size) {
+  if (!HasFocus())
+    return;
+
+  cc::PaintFlags flags;
+  flags.setAntiAlias(true);
+  flags.setColor(AshColorProvider::Get()->GetControlsLayerColor(
+      AshColorProvider::ControlsLayerType::kFocusRingColor));
+  flags.setStrokeWidth(views::PlatformStyle::kFocusHaloThickness);
+  flags.setStyle(cc::PaintFlags::kStroke_Style);
+
+  gfx::Rect bounds = gfx::Rect(size);
+  bounds.Inset(gfx::Insets(flags.getStrokeWidth() / 2));
+  canvas->DrawRoundRect(bounds, kHoldingSpaceCornerRadius, flags);
+}
+
+void HoldingSpaceItemView::OnPaintSelect(gfx::Canvas* canvas, gfx::Size size) {
+  if (!selected_)
+    return;
+
+  const SkColor color = AshColorProvider::Get()->GetControlsLayerColor(
+      AshColorProvider::ControlsLayerType::kFocusRingColor);
+
+  const SkColor overlay_color =
+      SkColorSetA(color, kHoldingSpaceSelectedOverlayOpacity * 0xFF);
+
+  cc::PaintFlags flags;
+  flags.setAntiAlias(true);
+  flags.setColor(overlay_color);
+
+  gfx::Rect bounds = gfx::Rect(size);
+  canvas->DrawRoundRect(bounds, kHoldingSpaceCornerRadius, flags);
+
+  flags.setColor(color);
+  flags.setStrokeWidth(views::PlatformStyle::kFocusHaloThickness);
+  flags.setStyle(cc::PaintFlags::kStroke_Style);
+
+  bounds.Inset(gfx::Insets(flags.getStrokeWidth() / 2));
+  canvas->DrawRoundRect(bounds, kHoldingSpaceCornerRadius, flags);
+}
+
 void HoldingSpaceItemView::OnPinPressed() {
   const bool is_item_pinned = HoldingSpaceController::Get()->model()->GetItem(
       HoldingSpaceItem::GetFileBackedItemId(HoldingSpaceItem::Type::kPinnedFile,

ash/system/holding_space/holding_space_item_view.h

@@ -5,6 +5,8 @@
 #ifndef ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_ITEM_VIEW_H_
 #define ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_ITEM_VIEW_H_
 
+#include <memory>
+
 #include "ash/ash_export.h"
 #include "ui/views/animation/ink_drop_host_view.h"
 #include "ui/views/metadata/metadata_header_macros.h"
@@ -34,21 +36,26 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
 
   // views::InkDropHostView:
   SkColor GetInkDropBaseColor() const override;
+  void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+  void OnFocus() override;
+  void OnBlur() override;
   void OnGestureEvent(ui::GestureEvent* event) override;
   bool OnKeyPressed(const ui::KeyEvent& event) override;
   void OnMouseEvent(ui::MouseEvent* event) override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
 
+  const HoldingSpaceItem* item() const { return item_; }
+
   void SetSelected(bool selected);
   bool selected() const { return selected_; }
 
-  const HoldingSpaceItem* item() const { return item_; }
-
  protected:
   void AddPin(views::View* parent);
 
  private:
+  void OnPaintFocus(gfx::Canvas* canvas, gfx::Size size);
+  void OnPaintSelect(gfx::Canvas* canvas, gfx::Size size);
   void OnPinPressed();
   void UpdatePin();
 
@@ -56,6 +63,10 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   const HoldingSpaceItem* const item_;
   views::ToggleImageButton* pin_ = nullptr;
 
+  // Owners for the layers used to paint focused and selected states.
+  std::unique_ptr<ui::LayerOwner> selected_layer_owner_;
+  std::unique_ptr<ui::LayerOwner> focused_layer_owner_;
+
   // Whether or not this view is selected.
   bool selected_ = false;
 

ash/system/holding_space/pinned_files_container.cc

@@ -99,6 +99,8 @@ PinnedFilesContainer::PinnedFilesContainer(
   scroll_view->SetBackgroundColor(base::nullopt);
   scroll_view->SetDrawOverflowIndicator(false);
   scroll_view->SetVisible(false);
+  scroll_view->SetPaintToLayer();
+  scroll_view->layer()->SetFillsBoundsOpaquely(false);
   scroll_view->ClipHeightTo(0, INT_MAX);
 
   item_chips_container_ = scroll_view->SetContents(