 Chromium Code Reviews
 Chromium Code Reviews Issue 2001843002:
  Use ink drop hover for focus states on toolbar buttons and location  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 2001843002:
  Use ink drop hover for focus states on toolbar buttons and location  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: ui/views/controls/button/md_text_button.cc | 
| diff --git a/ui/views/controls/button/md_text_button.cc b/ui/views/controls/button/md_text_button.cc | 
| index ceaf7c6d739fca96bb38b30f0418ec12252c3680..51d9971c0dc1fea51b1455010cbbac8ed2f8a0d4 100644 | 
| --- a/ui/views/controls/button/md_text_button.cc | 
| +++ b/ui/views/controls/button/md_text_button.cc | 
| @@ -6,6 +6,7 @@ | 
| #include "base/i18n/case_conversion.h" | 
| #include "ui/base/material_design/material_design_controller.h" | 
| +#include "ui/gfx/canvas.h" | 
| #include "ui/gfx/color_utils.h" | 
| #include "ui/native_theme/native_theme.h" | 
| #include "ui/views/background.h" | 
| @@ -17,6 +18,31 @@ namespace views { | 
| namespace { | 
| +// The amount to enlarge the focus border in all directions relative to the | 
| +// button. | 
| +const int kFocusBorderOutset = -2; | 
| + | 
| +// The corner radius of the focus border roundrect. | 
| +const int kFocusBorderCornerRadius = 3; | 
| + | 
| +class MdFocusRing : public views::View { | 
| + public: | 
| + MdFocusRing() { | 
| + SetPaintToLayer(true); | 
| + layer()->SetFillsBoundsOpaquely(false); | 
| + } | 
| + ~MdFocusRing() override {} | 
| + | 
| + bool CanProcessEventsWithinSubtree() const override { return false; } | 
| + | 
| + void OnPaint(gfx::Canvas* canvas) override { | 
| + MdTextButton::PaintMdFocusRing(canvas, this); | 
| + } | 
| + | 
| + private: | 
| + DISALLOW_COPY_AND_ASSIGN(MdFocusRing); | 
| +}; | 
| + | 
| // Inset between clickable region border and button contents (text). | 
| const int kHorizontalPadding = 12; | 
| const int kVerticalPadding = 6; | 
| @@ -78,6 +104,19 @@ MdTextButton* MdTextButton::CreateMdButton(ButtonListener* listener, | 
| return button; | 
| } | 
| +// static | 
| +void MdTextButton::PaintMdFocusRing(gfx::Canvas* canvas, views::View* view) { | 
| + SkPaint paint; | 
| + paint.setAntiAlias(true); | 
| + paint.setColor(view->GetNativeTheme()->GetSystemColor( | 
| + ui::NativeTheme::kColorId_CallToActionColor)); | 
| + paint.setStyle(SkPaint::kStroke_Style); | 
| + paint.setStrokeWidth(1); | 
| + gfx::RectF rect(view->GetLocalBounds()); | 
| + rect.Inset(gfx::InsetsF(0.5)); | 
| + canvas->DrawRoundRect(rect, kFocusBorderCornerRadius, paint); | 
| +} | 
| + | 
| void MdTextButton::SetCallToAction(CallToAction cta) { | 
| if (cta_ == cta) | 
| return; | 
| @@ -86,6 +125,22 @@ void MdTextButton::SetCallToAction(CallToAction cta) { | 
| UpdateColorsFromNativeTheme(); | 
| } | 
| +void MdTextButton::Layout() { | 
| 
sky
2016/05/26 21:13:01
Don't you need to call LabelButton::Layout ?
 
Evan Stade
2016/05/26 22:26:30
oops, yes.
 | 
| + gfx::Rect focus_bounds = GetLocalBounds(); | 
| + focus_bounds.Inset(gfx::Insets(kFocusBorderOutset)); | 
| + focus_ring_->SetBoundsRect(focus_bounds); | 
| +} | 
| + | 
| +void MdTextButton::OnFocus() { | 
| + LabelButton::OnFocus(); | 
| + focus_ring_->SetVisible(true); | 
| +} | 
| + | 
| +void MdTextButton::OnBlur() { | 
| + LabelButton::OnBlur(); | 
| + focus_ring_->SetVisible(false); | 
| +} | 
| + | 
| void MdTextButton::OnNativeThemeChanged(const ui::NativeTheme* theme) { | 
| LabelButton::OnNativeThemeChanged(theme); | 
| UpdateColorsFromNativeTheme(); | 
| @@ -95,6 +150,11 @@ SkColor MdTextButton::GetInkDropBaseColor() const { | 
| return color_utils::DeriveDefaultIconColor(label()->enabled_color()); | 
| } | 
| +bool MdTextButton::ShouldShowInkDropForFocus() const { | 
| + // These types of button use |focus_ring_|. | 
| + return false; | 
| +} | 
| + | 
| void MdTextButton::SetText(const base::string16& text) { | 
| LabelButton::SetText(base::i18n::ToUpper(text)); | 
| } | 
| @@ -110,16 +170,20 @@ void MdTextButton::UpdateStyleToIndicateDefaultStatus() { | 
| MdTextButton::MdTextButton(ButtonListener* listener) | 
| : LabelButton(listener, base::string16()), | 
| - ink_drop_delegate_(this, this), | 
| + focus_ring_(new MdFocusRing()), | 
| cta_(NO_CALL_TO_ACTION) { | 
| - set_ink_drop_delegate(&ink_drop_delegate_); | 
| + set_ink_drop_delegate( | 
| + base::WrapUnique(new ButtonInkDropDelegate(this, this))); | 
| set_has_ink_drop_action_on_click(true); | 
| SetHorizontalAlignment(gfx::ALIGN_CENTER); | 
| SetFocusForPlatform(); | 
| SetMinSize(gfx::Size(kMinWidth, 0)); | 
| SetFocusPainter(nullptr); | 
| - UseMdFocusRing(); | 
| label()->SetAutoColorReadabilityEnabled(false); | 
| + | 
| + AddChildView(focus_ring_); | 
| + focus_ring_->SetVisible(false); | 
| + set_request_focus_on_press(false); | 
| 
sky
2016/05/26 21:13:01
Do any places using MdTextButton and expecting tru
 
Evan Stade
2016/05/26 22:26:30
I don't think so. This was already being done by v
 | 
| } | 
| MdTextButton::~MdTextButton() {} |