Commit bf87fc9a authored by Wei Li's avatar Wei Li Committed by Commit Bot

Set default focus behavior for views button subclasses

Other than BaseScrollBarButton, other subclasses of Button class in
views/ should follow Button's default focus behavior.
BaseScrollBarButton should not be focusable, nor accessible for
accessibility.
Set these default values explicitly so changing Button class's default
won't affect them.

Bug: 1001103
Change-Id: If32d8dadf69a5ba5f8d23738678e31d285d3e5ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429435
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813451}
parent d7a13f38
...@@ -34,6 +34,8 @@ ImageButton::ImageButton(PressedCallback callback) ...@@ -34,6 +34,8 @@ ImageButton::ImageButton(PressedCallback callback)
// implementation is flipped horizontally so that the button's images are // implementation is flipped horizontally so that the button's images are
// mirrored when the UI directionality is right-to-left. // mirrored when the UI directionality is right-to-left.
EnableCanvasFlippingForRTLUI(true); EnableCanvasFlippingForRTLUI(true);
// Not focusable by default, only for accessibility.
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
} }
ImageButton::ImageButton(ButtonListener* listener) ImageButton::ImageButton(ButtonListener* listener)
......
...@@ -39,6 +39,12 @@ namespace views { ...@@ -39,6 +39,12 @@ namespace views {
using ImageButtonTest = ViewsTestBase; using ImageButtonTest = ViewsTestBase;
TEST_F(ImageButtonTest, FocusBehavior) {
ImageButton button;
EXPECT_EQ(View::FocusBehavior::ACCESSIBLE_ONLY, button.GetFocusBehavior());
}
TEST_F(ImageButtonTest, Basics) { TEST_F(ImageButtonTest, Basics) {
ImageButton button; ImageButton button;
......
...@@ -40,6 +40,8 @@ LabelButton::LabelButton(PressedCallback callback, ...@@ -40,6 +40,8 @@ LabelButton::LabelButton(PressedCallback callback,
style::GetFont(button_context, style::STYLE_PRIMARY)), style::GetFont(button_context, style::STYLE_PRIMARY)),
cached_default_button_font_list_( cached_default_button_font_list_(
style::GetFont(button_context, style::STYLE_DIALOG_BUTTON_DEFAULT)) { style::GetFont(button_context, style::STYLE_DIALOG_BUTTON_DEFAULT)) {
// Not focusable by default, only for accessibility.
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
ink_drop_container_ = AddChildView(std::make_unique<InkDropContainerView>()); ink_drop_container_ = AddChildView(std::make_unique<InkDropContainerView>());
ink_drop_container_->SetVisible(false); ink_drop_container_->SetVisible(false);
......
...@@ -141,6 +141,12 @@ class LabelButtonTest : public test::WidgetTest { ...@@ -141,6 +141,12 @@ class LabelButtonTest : public test::WidgetTest {
DISALLOW_COPY_AND_ASSIGN(LabelButtonTest); DISALLOW_COPY_AND_ASSIGN(LabelButtonTest);
}; };
TEST_F(LabelButtonTest, FocusBehavior) {
LabelButton button;
EXPECT_EQ(View::FocusBehavior::ACCESSIBLE_ONLY, button.GetFocusBehavior());
}
TEST_F(LabelButtonTest, Init) { TEST_F(LabelButtonTest, Init) {
const base::string16 text(ASCIIToUTF16("abc")); const base::string16 text(ASCIIToUTF16("abc"));
TestLabelButton button(text); TestLabelButton button(text);
......
...@@ -16,7 +16,10 @@ BaseScrollBarButton::BaseScrollBarButton(ButtonListener* listener, ...@@ -16,7 +16,10 @@ BaseScrollBarButton::BaseScrollBarButton(ButtonListener* listener,
: Button(listener), : Button(listener),
repeater_(base::BindRepeating(&BaseScrollBarButton::RepeaterNotifyClick, repeater_(base::BindRepeating(&BaseScrollBarButton::RepeaterNotifyClick,
base::Unretained(this)), base::Unretained(this)),
tick_clock) {} tick_clock) {
// Not focusable by default.
SetFocusBehavior(FocusBehavior::NEVER);
}
BaseScrollBarButton::~BaseScrollBarButton() = default; BaseScrollBarButton::~BaseScrollBarButton() = default;
......
...@@ -71,6 +71,10 @@ TEST_F(BaseScrollBarButtonTest, Metadata) { ...@@ -71,6 +71,10 @@ TEST_F(BaseScrollBarButtonTest, Metadata) {
test::TestViewMetadata(button()); test::TestViewMetadata(button());
} }
TEST_F(BaseScrollBarButtonTest, FocusBehavior) {
EXPECT_EQ(View::FocusBehavior::NEVER, button()->GetFocusBehavior());
}
TEST_F(BaseScrollBarButtonTest, CallbackFiresOnMouseDown) { TEST_F(BaseScrollBarButtonTest, CallbackFiresOnMouseDown) {
EXPECT_CALL(listener(), ButtonPressed(_, _)); EXPECT_CALL(listener(), ButtonPressed(_, _));
......
...@@ -75,7 +75,7 @@ class ScrollBarThumb : public BaseScrollBarThumb { ...@@ -75,7 +75,7 @@ class ScrollBarThumb : public BaseScrollBarThumb {
ScrollBarButton::ScrollBarButton(ButtonListener* listener, Type type) ScrollBarButton::ScrollBarButton(ButtonListener* listener, Type type)
: BaseScrollBarButton(listener), type_(type) { : BaseScrollBarButton(listener), type_(type) {
EnableCanvasFlippingForRTLUI(true); EnableCanvasFlippingForRTLUI(true);
SetFocusBehavior(FocusBehavior::NEVER); DCHECK_EQ(FocusBehavior::NEVER, GetFocusBehavior());
} }
ScrollBarButton::~ScrollBarButton() = default; ScrollBarButton::~ScrollBarButton() = default;
......
...@@ -77,6 +77,8 @@ FrameCaptionButton::FrameCaptionButton(views::ButtonListener* listener, ...@@ -77,6 +77,8 @@ FrameCaptionButton::FrameCaptionButton(views::ButtonListener* listener,
ink_drop_corner_radius_(kCaptionButtonInkDropDefaultCornerRadius), ink_drop_corner_radius_(kCaptionButtonInkDropDefaultCornerRadius),
swap_images_animation_(new gfx::SlideAnimation(this)) { swap_images_animation_(new gfx::SlideAnimation(this)) {
views::SetHitTestComponent(this, hit_test_type); views::SetHitTestComponent(this, hit_test_type);
// Not focusable by default, only for accessibility.
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
SetAnimateOnStateChange(true); SetAnimateOnStateChange(true);
swap_images_animation_->Reset(1); swap_images_animation_->Reset(1);
......
...@@ -6,7 +6,10 @@ ...@@ -6,7 +6,10 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
#include "ui/base/hit_test.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
#include "ui/views/view.h"
#include "ui/views/window/caption_button_types.h"
namespace { namespace {
...@@ -20,10 +23,18 @@ constexpr SkColor kBackgroundColors[] = { ...@@ -20,10 +23,18 @@ constexpr SkColor kBackgroundColors[] = {
} // namespace } // namespace
namespace views {
TEST(FrameCaptionButtonTest, ThemedColorContrast) { TEST(FrameCaptionButtonTest, ThemedColorContrast) {
for (SkColor background_color : kBackgroundColors) { for (SkColor background_color : kBackgroundColors) {
SkColor button_color = SkColor button_color = FrameCaptionButton::GetButtonColor(background_color);
views::FrameCaptionButton::GetButtonColor(background_color);
EXPECT_GE(color_utils::GetContrastRatio(button_color, background_color), 3); EXPECT_GE(color_utils::GetContrastRatio(button_color, background_color), 3);
} }
} }
TEST(FrameCaptionButtonTest, DefaultAccessibilityFocus) {
FrameCaptionButton button(nullptr, CAPTION_BUTTON_ICON_MINIMIZE, HTMINBUTTON);
EXPECT_EQ(View::FocusBehavior::ACCESSIBLE_ONLY, button.GetFocusBehavior());
}
} // namespace views
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment