Commit 59135b40 authored by Wei Li's avatar Wei Li Committed by Commit Bot

[color] Use blue focus ring for app info label

App info label used gray dashed rectangle as its focus ring. That is not
consistent with other places which use regular blue focus ring. This CL
changes app info label to use FocusRing as well to make the appearance
consistent.

Since this was the only caller for canvas' DrawFocusRect() and
DrawDashedRect(). We remove all those custom calls.

Before and after screenshots are at https://bit.ly/2VlOdyy

BUG=553726, 1059867

Change-Id: If8a2a32002ccb4ae11a28ef229a3964a662c0233
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141221Reviewed-by: default avatardanakj <danakj@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Commit-Queue: Wei Li <weili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758995}
parent 65723df7
...@@ -4,13 +4,15 @@ ...@@ -4,13 +4,15 @@
#include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h" #include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h"
#include "ui/gfx/canvas.h" #include "ui/views/controls/focus_ring.h"
AppInfoLabel::AppInfoLabel(const base::string16& text) AppInfoLabel::AppInfoLabel(const base::string16& text)
: AppInfoLabel(text, : AppInfoLabel(text,
views::style::CONTEXT_LABEL, views::style::CONTEXT_LABEL,
views::style::STYLE_PRIMARY) {} views::style::STYLE_PRIMARY) {}
AppInfoLabel::~AppInfoLabel() = default;
AppInfoLabel::AppInfoLabel(const base::string16& text, AppInfoLabel::AppInfoLabel(const base::string16& text,
int text_context, int text_context,
int text_style, int text_style,
...@@ -21,20 +23,5 @@ AppInfoLabel::AppInfoLabel(const base::string16& text, ...@@ -21,20 +23,5 @@ AppInfoLabel::AppInfoLabel(const base::string16& text,
// still needs to be able to tab-navigate them and get screen reader feedback. // still needs to be able to tab-navigate them and get screen reader feedback.
SetFocusBehavior(views::View::FocusBehavior::ALWAYS); SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
SetHorizontalAlignment(gfx::ALIGN_LEFT); SetHorizontalAlignment(gfx::ALIGN_LEFT);
} focus_ring_ = views::FocusRing::Install(this);
void AppInfoLabel::PaintFocusRing(gfx::Canvas* canvas) const {
gfx::Rect focus_ring_bounds = GetTextBounds();
focus_ring_bounds.Intersect(GetLocalBounds());
canvas->DrawFocusRect(focus_ring_bounds);
}
void AppInfoLabel::OnFocus() {
Label::OnFocus();
SchedulePaint();
}
void AppInfoLabel::OnBlur() {
Label::OnBlur();
SchedulePaint();
} }
...@@ -5,14 +5,21 @@ ...@@ -5,14 +5,21 @@
#ifndef CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_ #ifndef CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_
#define CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_ #define CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_
#include <memory>
#include "ui/views/controls/label.h" #include "ui/views/controls/label.h"
namespace views {
class FocusRing;
}
// Label styled for use in AppInfo dialog so accessible users can step through // Label styled for use in AppInfo dialog so accessible users can step through
// and have each line read. // and have each line read.
// TODO(dfried): merge functionality into views::Label. // TODO(dfried): merge functionality into views::Label.
class AppInfoLabel : public views::Label { class AppInfoLabel : public views::Label {
public: public:
explicit AppInfoLabel(const base::string16& text); explicit AppInfoLabel(const base::string16& text);
~AppInfoLabel() override;
// See documentation on views::Label::Label(). // See documentation on views::Label::Label().
AppInfoLabel(const base::string16& text, AppInfoLabel(const base::string16& text,
...@@ -22,10 +29,7 @@ class AppInfoLabel : public views::Label { ...@@ -22,10 +29,7 @@ class AppInfoLabel : public views::Label {
gfx::DirectionalityMode::DIRECTIONALITY_FROM_TEXT); gfx::DirectionalityMode::DIRECTIONALITY_FROM_TEXT);
private: private:
// views::Label: std::unique_ptr<views::FocusRing> focus_ring_;
void PaintFocusRing(gfx::Canvas* canvas) const override;
void OnFocus() override;
void OnBlur() override;
}; };
#endif // CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_ #endif // CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_
...@@ -98,34 +98,6 @@ int Canvas::DefaultCanvasTextAlignment() { ...@@ -98,34 +98,6 @@ int Canvas::DefaultCanvasTextAlignment() {
return base::i18n::IsRTL() ? TEXT_ALIGN_RIGHT : TEXT_ALIGN_LEFT; return base::i18n::IsRTL() ? TEXT_ALIGN_RIGHT : TEXT_ALIGN_LEFT;
} }
void Canvas::DrawDashedRect(const RectF& inrect, SkColor color) {
if (inrect.IsEmpty())
return;
RectF rect = inrect;
cc::PaintFlags flags;
flags.setColor(color);
SkScalar intervals[] = {1.f, 1.f};
flags.setStrokeWidth(1.f);
flags.setStyle(cc::PaintFlags::kStroke_Style);
rect.Inset(gfx::InsetsF(0.5f));
flags.setPathEffect(SkDashPathEffect::Make(intervals, 2, 0));
// Top-left to top-right.
canvas_->drawLine(rect.x() - 0.5f, rect.y(), rect.right() + 0.5f, rect.y(),
flags);
// Top-left to bottom-left.
canvas_->drawLine(rect.right() + 0.5f, rect.bottom(), rect.x() - 0.5f,
rect.bottom(), flags);
// Bottom-right to bottom-left.
canvas_->drawLine(rect.x(), rect.y() - 0.5f, rect.x(), rect.bottom() + 0.5f,
flags);
// Bottom-right to top-right.
canvas_->drawLine(rect.right(), rect.bottom() + 0.5f, rect.right(),
rect.y() - 0.5f, flags);
}
float Canvas::UndoDeviceScaleFactor() { float Canvas::UndoDeviceScaleFactor() {
SkScalar scale_factor = 1.0f / image_scale_; SkScalar scale_factor = 1.0f / image_scale_;
canvas_->scale(scale_factor, scale_factor); canvas_->scale(scale_factor, scale_factor);
...@@ -309,14 +281,6 @@ void Canvas::DrawPath(const SkPath& path, const cc::PaintFlags& flags) { ...@@ -309,14 +281,6 @@ void Canvas::DrawPath(const SkPath& path, const cc::PaintFlags& flags) {
canvas_->drawPath(path, flags); canvas_->drawPath(path, flags);
} }
void Canvas::DrawFocusRect(const Rect& rect) {
DrawFocusRect(RectF(rect));
}
void Canvas::DrawFocusRect(const RectF& rect) {
DrawDashedRect(rect, SK_ColorGRAY);
}
void Canvas::DrawSolidFocusRect(RectF rect, SkColor color, int thickness) { void Canvas::DrawSolidFocusRect(RectF rect, SkColor color, int thickness) {
cc::PaintFlags flags; cc::PaintFlags flags;
flags.setColor(color); flags.setColor(color);
......
...@@ -151,9 +151,6 @@ class GFX_EXPORT Canvas { ...@@ -151,9 +151,6 @@ class GFX_EXPORT Canvas {
// Canvas::TEXT_ALIGN_RIGHT. // Canvas::TEXT_ALIGN_RIGHT.
static int DefaultCanvasTextAlignment(); static int DefaultCanvasTextAlignment();
// Draws a dashed rectangle of the specified color.
void DrawDashedRect(const RectF& rect, SkColor color);
// Unscales by the image scale factor (aka device scale factor), and returns // Unscales by the image scale factor (aka device scale factor), and returns
// that factor. This is useful when callers want to draw directly in the // that factor. This is useful when callers want to draw directly in the
// native scale. // native scale.
...@@ -381,14 +378,6 @@ class GFX_EXPORT Canvas { ...@@ -381,14 +378,6 @@ class GFX_EXPORT Canvas {
const Rect& display_rect, const Rect& display_rect,
int flags); int flags);
// Draws a dotted gray rectangle used for focus purposes.
// DEPRECATED in favor of the RectF version below.
// TODO(funkysidd): Remove this (http://crbug.com/553726)
void DrawFocusRect(const Rect& rect);
// Draws a dotted gray rectangle used for focus purposes.
void DrawFocusRect(const RectF& rect);
// Draws a |rect| in the specified region with the specified |color|. The // Draws a |rect| in the specified region with the specified |color|. The
// width of the stroke is |thickness| dip, but the actual pixel width will be // width of the stroke is |thickness| dip, but the actual pixel width will be
// floored to ensure an integral value. // floored to ensure an integral value.
......
...@@ -598,10 +598,6 @@ std::unique_ptr<gfx::RenderText> Label::CreateRenderText() const { ...@@ -598,10 +598,6 @@ std::unique_ptr<gfx::RenderText> Label::CreateRenderText() const {
return render_text; return render_text;
} }
void Label::PaintFocusRing(gfx::Canvas* canvas) const {
// No focus ring by default.
}
gfx::Rect Label::GetTextBounds() const { gfx::Rect Label::GetTextBounds() const {
MaybeBuildDisplayText(); MaybeBuildDisplayText();
...@@ -645,8 +641,6 @@ void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) { ...@@ -645,8 +641,6 @@ void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) {
void Label::OnPaint(gfx::Canvas* canvas) { void Label::OnPaint(gfx::Canvas* canvas) {
View::OnPaint(canvas); View::OnPaint(canvas);
PaintText(canvas); PaintText(canvas);
if (HasFocus())
PaintFocusRing(canvas);
} }
void Label::OnThemeChanged() { void Label::OnThemeChanged() {
......
...@@ -267,9 +267,6 @@ class VIEWS_EXPORT Label : public View, ...@@ -267,9 +267,6 @@ class VIEWS_EXPORT Label : public View,
// Create a single RenderText instance to actually be painted. // Create a single RenderText instance to actually be painted.
virtual std::unique_ptr<gfx::RenderText> CreateRenderText() const; virtual std::unique_ptr<gfx::RenderText> CreateRenderText() const;
// Draw a focus ring. The default implementation does nothing.
virtual void PaintFocusRing(gfx::Canvas* canvas) const;
// Returns the preferred size and position of the text in local coordinates, // Returns the preferred size and position of the text in local coordinates,
// which may exceed the local bounds of the label. // which may exceed the local bounds of the label.
gfx::Rect GetTextBounds() const; gfx::Rect GetTextBounds() const;
......
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