Make views::ToggleButton use the ink drop ripple (not higlight) for

focus.

This matches Polymer behavior and is preferential to the current focus
effect (blue rectangle).

Also get rid of the the focus rect for views::Slider, which already has
the glow effect when focused and hence needs no further changes.

Bug: 658783
Change-Id: Id9d5e3f1434b0d444154be933f0067f97aaaed33
Reviewed-on: https://chromium-review.googlesource.com/592494
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Valery Arkhangorodsky <varkha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491620}

ash/system/network/network_list.cc

@@ -114,7 +114,7 @@ class NetworkListView::SectionHeaderRowView : public views::View,
   }
 
   virtual void SetIsOn(bool enabled) {
-    toggle_->SetEnabled(true);
+    toggle_->set_accepts_events(true);
     toggle_->SetIsOn(enabled, true);
   }
 
@@ -141,9 +141,10 @@ class NetworkListView::SectionHeaderRowView : public views::View,
     DCHECK_EQ(toggle_, sender);
     // In the event of frequent clicks, helps to prevent a toggle button state
     // from becoming inconsistent with the async operation of enabling /
-    // disabling of mobile radio. The toggle will get re-enabled in the next
-    // call to NetworkListView::Update().
-    toggle_->SetEnabled(false);
+    // disabling of mobile radio. The toggle will get unlocked in the next
+    // call to NetworkListView::Update(). Note that we don't disable/enable
+    // because that would clear focus.
+    toggle_->set_accepts_events(false);
     OnToggleToggled(toggle_->is_on());
   }
 

ash/system/tray/tray_popup_utils.cc

@@ -250,7 +250,6 @@ views::ToggleButton* TrayPopupUtils::CreateToggleButton(
       (kTrayToggleButtonWidth - toggle_size.width()) / 2;
   toggle->SetBorder(views::CreateEmptyBorder(
       gfx::Insets(vertical_padding, horizontal_padding)));
-  toggle->SetFocusPainter(CreateFocusPainter());
   toggle->SetAccessibleName(l10n_util::GetStringUTF16(accessible_name_id));
   return toggle;
 }

ui/views/animation/square_ink_drop_ripple.cc

@@ -238,6 +238,8 @@ void SquareInkDropRipple::AnimateStateChange(
       }
       break;
     case InkDropState::ACTION_PENDING:
+      if (old_ink_drop_state == new_ink_drop_state)
+        return;
       DCHECK_EQ(InkDropState::HIDDEN, old_ink_drop_state)
           << " old_ink_drop_state=" << ToString(old_ink_drop_state);
       AnimateToOpacity(visible_opacity_,

ui/views/controls/button/toggle_button.cc

@@ -111,8 +111,6 @@ const char ToggleButton::kViewClassName[] = "ToggleButton";
 
 ToggleButton::ToggleButton(ButtonListener* listener)
     : CustomButton(listener),
-      is_on_(false),
-      slide_animation_(this),
       thumb_view_(new ThumbView()) {
   slide_animation_.SetSlideDuration(80 /* ms */);
   slide_animation_.SetTweenType(gfx::Tween::LINEAR);
@@ -185,6 +183,10 @@ const char* ToggleButton::GetClassName() const {
   return kViewClassName;
 }
 
+bool ToggleButton::CanAcceptEvent(const ui::Event& event) {
+  return accepts_events_ && CustomButton::CanAcceptEvent(event);
+}
+
 void ToggleButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   UpdateThumb();
 }
@@ -202,9 +204,32 @@ void ToggleButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   node_data->AddIntAttribute(ui::AX_ATTR_CHECKED_STATE, checked_state);
 }
 
+void ToggleButton::OnFocus() {
+  CustomButton::OnFocus();
+  AnimateInkDrop(views::InkDropState::ACTION_PENDING, nullptr);
+}
+
+void ToggleButton::OnBlur() {
+  CustomButton::OnBlur();
+
+  // The ink drop may have already gone away if the user clicked after focusing.
+  if (GetInkDrop()->GetTargetInkDropState() ==
+      views::InkDropState::ACTION_PENDING) {
+    AnimateInkDrop(views::InkDropState::ACTION_TRIGGERED, nullptr);
+  }
+}
+
 void ToggleButton::NotifyClick(const ui::Event& event) {
   SetIsOn(!is_on(), true);
-  CustomButton::NotifyClick(event);
+
+  // Skip over CustomButton::NotifyClick, to customize the ink drop animation.
+  // Leave the ripple in place when the button is activated via the keyboard.
+  if (!event.IsKeyEvent()) {
+    AnimateInkDrop(InkDropState::ACTION_TRIGGERED,
+                   ui::LocatedEvent::FromIfValid(&event));
+  }
+
+  Button::NotifyClick(event);
 }
 
 void ToggleButton::PaintButtonContents(gfx::Canvas* canvas) {
@@ -247,7 +272,7 @@ std::unique_ptr<InkDropRipple> ToggleButton::CreateInkDropRipple() const {
 }
 
 SkColor ToggleButton::GetInkDropBaseColor() const {
-  return GetTrackColor(is_on());
+  return GetTrackColor(is_on() || HasFocus());
 }
 
 void ToggleButton::AnimationProgressed(const gfx::Animation* animation) {

ui/views/controls/button/toggle_button.h

@@ -23,6 +23,10 @@ class VIEWS_EXPORT ToggleButton : public CustomButton {
   void SetIsOn(bool is_on, bool animate);
   bool is_on() const { return is_on_; }
 
+  void set_accepts_events(bool accepts_events) {
+    accepts_events_ = accepts_events;
+  }
+
   // views::View:
   gfx::Size CalculatePreferredSize() const override;
 
@@ -43,9 +47,12 @@ class VIEWS_EXPORT ToggleButton : public CustomButton {
 
   // views::View:
   const char* GetClassName() const override;
+  bool CanAcceptEvent(const ui::Event& event) override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
   void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+  void OnFocus() override;
+  void OnBlur() override;
 
   // CustomButton:
   void NotifyClick(const ui::Event& event) override;
@@ -59,10 +66,14 @@ class VIEWS_EXPORT ToggleButton : public CustomButton {
   // gfx::AnimationDelegate:
   void AnimationProgressed(const gfx::Animation* animation) override;
 
-  bool is_on_;
-  gfx::SlideAnimation slide_animation_;
+  bool is_on_ = false;
+  gfx::SlideAnimation slide_animation_{this};
   ThumbView* thumb_view_;
 
+  // When false, this button won't accept input. Different from View::SetEnabled
+  // in that the view retains focus when this is false but not when disabled.
+  bool accepts_events_ = true;
+
   DISALLOW_COPY_AND_ASSIGN(ToggleButton);
 };
 

ui/views/controls/button/toggle_button_unittest.cc

@@ -9,6 +9,7 @@
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/views/style/platform_style.h"
 #include "ui/views/test/views_test_base.h"
 
@@ -25,6 +26,8 @@ class TestToggleButton : public ToggleButton {
     SetInkDropMode(InkDropMode::OFF);
   }
 
+  using View::Focus;
+
  protected:
   // ToggleButton:
   void AddInkDropLayer(ui::Layer* ink_drop_layer) override {
@@ -54,7 +57,8 @@ class ToggleButtonTest : public ViewsTestBase {
     // Create a widget so that the ToggleButton can query the hover state
     // correctly.
     widget_.reset(new Widget);
-    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
+    Widget::InitParams params =
+        CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
     params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
     params.bounds = gfx::Rect(0, 0, 650, 650);
     widget_->Init(params);
@@ -100,4 +104,53 @@ TEST_F(ToggleButtonTest, ToggleButtonDestroyed) {
   EXPECT_EQ(0, counter());
 }
 
+// Make sure nothing bad happens when the widget is destroyed while the
+// ToggleButton has focus (and is showing a ripple).
+TEST_F(ToggleButtonTest, ShutdownWithFocus) {
+  button()->RequestFocus();
+  if (PlatformStyle::kUseRipples)
+    EXPECT_EQ(1, counter());
+  else
+    EXPECT_EQ(0, counter());
+}
+
+// Verify that ToggleButton::accepts_events_ works as expected.
+TEST_F(ToggleButtonTest, AcceptEvents) {
+  EXPECT_FALSE(button()->is_on());
+  ui::test::EventGenerator generator(widget()->GetNativeWindow());
+
+  // Clicking toggles.
+  generator.ClickLeftButton();
+  EXPECT_TRUE(button()->is_on());
+  generator.ClickLeftButton();
+  EXPECT_FALSE(button()->is_on());
+
+  // Spacebar toggles.
+  button()->RequestFocus();
+  generator.PressKey(ui::VKEY_SPACE, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_SPACE, ui::EF_NONE);
+  EXPECT_TRUE(button()->is_on());
+  generator.PressKey(ui::VKEY_SPACE, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_SPACE, ui::EF_NONE);
+  EXPECT_FALSE(button()->is_on());
+
+  // Spacebar and clicking do nothing when not accepting events, but focus is
+  // not affected.
+  button()->set_accepts_events(false);
+  EXPECT_TRUE(button()->HasFocus());
+  generator.PressKey(ui::VKEY_SPACE, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_SPACE, ui::EF_NONE);
+  EXPECT_FALSE(button()->is_on());
+  generator.ClickLeftButton();
+  EXPECT_FALSE(button()->is_on());
+
+  // Re-enable events and clicking and spacebar resume working.
+  button()->set_accepts_events(true);
+  generator.PressKey(ui::VKEY_SPACE, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_SPACE, ui::EF_NONE);
+  EXPECT_TRUE(button()->is_on());
+  generator.ClickLeftButton();
+  EXPECT_FALSE(button()->is_on());
+}
+
 }  // namespace views

ui/views/controls/slider.cc

@@ -20,7 +20,6 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
-#include "ui/native_theme/native_theme.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
@@ -184,21 +183,6 @@ void Slider::MoveButtonTo(const gfx::Point& point) {
       VALUE_CHANGED_BY_USER);
 }
 
-void Slider::OnPaintFocus(gfx::Canvas* canvas) {
-  if (!HasFocus())
-    return;
-
-  // TODO(estade): make this a glow effect instead: crbug.com/658783
-  gfx::Rect focus_bounds = GetLocalBounds();
-  focus_bounds.Inset(gfx::Insets(1));
-  canvas->DrawSolidFocusRect(
-      gfx::RectF(focus_bounds),
-      SkColorSetA(GetNativeTheme()->GetSystemColor(
-                      ui::NativeTheme::kColorId_FocusedBorderColor),
-                  0x99),
-      2);
-}
-
 void Slider::OnSliderDragStarted() {
   SetHighlighted(true);
   if (listener_)
@@ -321,8 +305,6 @@ void Slider::OnPaint(gfx::Canvas* canvas) {
   canvas->DrawCircle(
       thumb_center,
       is_active_ ? kThumbRadius : (kThumbRadius - kThumbStroke / 2), flags);
-
-  OnPaintFocus(canvas);
 }
 
 void Slider::OnFocus() {

ui/views/controls/slider.h

@@ -87,8 +87,6 @@ class VIEWS_EXPORT Slider : public View, public gfx::AnimationDelegate {
   // Moves the button to the specified point and updates the value accordingly.
   void MoveButtonTo(const gfx::Point& point);
 
-  void OnPaintFocus(gfx::Canvas* canvas);
-
   // Notify the listener_, if not NULL, that dragging started.
   void OnSliderDragStarted();