Commit e9263549 authored by Livvie Lin's avatar Livvie Lin Committed by Commit Bot

Update Link to underline on hover

For accessibility, color should not be the only indicator of a link.
Thus, indicate the existence of a link by underlining it on hover.

Bug: 1060186
Change-Id: If8a994fa1226e1e30f6238719a6d8f94f25ffbd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441938Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Commit-Queue: Livvie Lin <livvielin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813336}
parent 991f4851
...@@ -71,6 +71,14 @@ bool Link::GetCanProcessEventsWithinSubtree() const { ...@@ -71,6 +71,14 @@ bool Link::GetCanProcessEventsWithinSubtree() const {
return View::GetCanProcessEventsWithinSubtree(); return View::GetCanProcessEventsWithinSubtree();
} }
void Link::OnMouseEntered(const ui::MouseEvent& event) {
RecalculateFont();
}
void Link::OnMouseExited(const ui::MouseEvent& event) {
RecalculateFont();
}
bool Link::OnMousePressed(const ui::MouseEvent& event) { bool Link::OnMousePressed(const ui::MouseEvent& event) {
if (!GetEnabled() || if (!GetEnabled() ||
(!event.IsLeftMouseButton() && !event.IsMiddleMouseButton())) (!event.IsLeftMouseButton() && !event.IsMiddleMouseButton()))
...@@ -198,9 +206,10 @@ void Link::OnClick(const ui::Event& event) { ...@@ -198,9 +206,10 @@ void Link::OnClick(const ui::Event& event) {
void Link::RecalculateFont() { void Link::RecalculateFont() {
const int style = font_list().GetFontStyle(); const int style = font_list().GetFontStyle();
const int intended_style = ((GetEnabled() && HasFocus()) || force_underline_) const int intended_style =
? (style | gfx::Font::UNDERLINE) ((GetEnabled() && (HasFocus() || IsMouseHovered())) || force_underline_)
: (style & ~gfx::Font::UNDERLINE); ? (style | gfx::Font::UNDERLINE)
: (style & ~gfx::Font::UNDERLINE);
if (style != intended_style) if (style != intended_style)
Label::SetFontList(font_list().DeriveWithStyle(intended_style)); Label::SetFontList(font_list().DeriveWithStyle(intended_style));
......
...@@ -60,6 +60,8 @@ class VIEWS_EXPORT Link : public Label { ...@@ -60,6 +60,8 @@ class VIEWS_EXPORT Link : public Label {
// Label: // Label:
gfx::NativeCursor GetCursor(const ui::MouseEvent& event) override; gfx::NativeCursor GetCursor(const ui::MouseEvent& event) override;
bool GetCanProcessEventsWithinSubtree() const override; bool GetCanProcessEventsWithinSubtree() const override;
void OnMouseEntered(const ui::MouseEvent& event) override;
void OnMouseExited(const ui::MouseEvent& event) override;
bool OnMousePressed(const ui::MouseEvent& event) override; bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override; bool OnMouseDragged(const ui::MouseEvent& event) override;
void OnMouseReleased(const ui::MouseEvent& event) override; void OnMouseReleased(const ui::MouseEvent& event) override;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <vector> #include <vector>
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/ui_base_switches.h" #include "ui/base/ui_base_switches.h"
#include "ui/events/base_event_utils.h" #include "ui/events/base_event_utils.h"
...@@ -31,16 +32,29 @@ class LinkTest : public test::BaseControlTestWidget { ...@@ -31,16 +32,29 @@ class LinkTest : public test::BaseControlTestWidget {
LinkTest& operator=(const LinkTest&) = delete; LinkTest& operator=(const LinkTest&) = delete;
~LinkTest() override = default; ~LinkTest() override = default;
void SetUp() override {
test::BaseControlTestWidget::SetUp();
event_generator_ = std::make_unique<ui::test::EventGenerator>(
GetContext(), widget()->GetNativeWindow());
event_generator_->set_assume_window_at_origin(false);
}
protected: protected:
void CreateWidgetContent(View* container) override { void CreateWidgetContent(View* container) override {
// Create a widget containing a link which does not take the full size.
link_ = container->AddChildView( link_ = container->AddChildView(
std::make_unique<Link>(base::ASCIIToUTF16("TestLink"))); std::make_unique<Link>(base::ASCIIToUTF16("TestLink")));
link_->SetBoundsRect(
gfx::ScaleToEnclosedRect(container->GetLocalBounds(), 0.5f));
} }
Link* link() { return link_; } Link* link() { return link_; }
ui::test::EventGenerator* event_generator() { return event_generator_.get(); }
public: public:
Link* link_ = nullptr; Link* link_ = nullptr;
std::unique_ptr<ui::test::EventGenerator> event_generator_;
}; };
} // namespace } // namespace
...@@ -75,4 +89,30 @@ TEST_F(LinkTest, TestLinkTap) { ...@@ -75,4 +89,30 @@ TEST_F(LinkTest, TestLinkTap) {
EXPECT_TRUE(link_clicked); EXPECT_TRUE(link_clicked);
} }
// This test doesn't work on Mac due to crbug.com/1071633.
#if !defined(OS_MAC)
// Tests that hovering and unhovering a link adds and removes an underline.
TEST_F(LinkTest, TestUnderlineOnHover) {
// A non-hovered link should not be underlined.
const gfx::Rect link_bounds = link()->GetBoundsInScreen();
const gfx::Point off_link = link_bounds.bottom_right() + gfx::Vector2d(1, 1);
event_generator()->MoveMouseTo(off_link);
EXPECT_FALSE(link()->IsMouseHovered());
const auto link_underlined = [link = link()]() {
return !!(link->font_list().GetFontStyle() & gfx::Font::UNDERLINE);
};
EXPECT_FALSE(link_underlined());
// Hovering the link should underline it.
event_generator()->MoveMouseTo(link_bounds.CenterPoint());
EXPECT_TRUE(link()->IsMouseHovered());
EXPECT_TRUE(link_underlined());
// Un-hovering the link should remove the underline again.
event_generator()->MoveMouseTo(off_link);
EXPECT_FALSE(link()->IsMouseHovered());
EXPECT_FALSE(link_underlined());
}
#endif // !defined(OS_MAC)
} // namespace views } // 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