Commit 1e1e5484 authored by Wei Li's avatar Wei Li Committed by Commit Bot

Fix status bubble text low contrast with some themes

Status bubble text was drawn with certain transparency. When it was
changed to be drawn on a label, the color would be automatically
computed based on foreground and background colors, which makes the text
color even lighter with some themes. We fix that by disable auto color
readability on that label.

BUG=952061

Change-Id: I25ec54aa82c4d62e7fa67f146f0e7c9e39511eac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565504
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652879}
parent 4a43698a
...@@ -2451,6 +2451,9 @@ void BrowserView::OnThemeChanged() { ...@@ -2451,6 +2451,9 @@ void BrowserView::OnThemeChanged() {
ui::NativeTheme::GetInstanceForNativeUi()->NotifyObservers(); ui::NativeTheme::GetInstanceForNativeUi()->NotifyObservers();
} }
if (status_bubble_)
status_bubble_->OnThemeChanged();
views::View::OnThemeChanged(); views::View::OnThemeChanged();
} }
......
...@@ -157,6 +157,7 @@ class StatusBubbleViews::StatusView : public views::View { ...@@ -157,6 +157,7 @@ class StatusBubbleViews::StatusView : public views::View {
// views::View: // views::View:
void Layout() override; void Layout() override;
void OnThemeChanged() override;
// Set the bubble text, or hide the bubble if |text| is an empty string. // Set the bubble text, or hide the bubble if |text| is an empty string.
// Triggers an animation sequence to display if |should_animate_open| is true. // Triggers an animation sequence to display if |should_animate_open| is true.
...@@ -203,6 +204,9 @@ class StatusBubbleViews::StatusView : public views::View { ...@@ -203,6 +204,9 @@ class StatusBubbleViews::StatusView : public views::View {
void StartHiding(); void StartHiding();
void StartShowing(); void StartShowing();
// Set the text label's colors according to the theme.
void SetTextLabelColors(views::Label* label);
// views::View: // views::View:
const char* GetClassName() const override; const char* GetClassName() const override;
void OnPaint(gfx::Canvas* canvas) override; void OnPaint(gfx::Canvas* canvas) override;
...@@ -235,16 +239,10 @@ StatusBubbleViews::StatusView::StatusView(StatusBubbleViews* status_bubble, ...@@ -235,16 +239,10 @@ StatusBubbleViews::StatusView::StatusView(StatusBubbleViews* status_bubble,
: status_bubble_(status_bubble), popup_size_(popup_size) { : status_bubble_(status_bubble), popup_size_(popup_size) {
animation_ = std::make_unique<StatusViewAnimation>(this, 0, 0); animation_ = std::make_unique<StatusViewAnimation>(this, 0, 0);
// Text color is the foreground tab text color at 60% alpha.
std::unique_ptr<views::Label> text = std::make_unique<views::Label>(); std::unique_ptr<views::Label> text = std::make_unique<views::Label>();
const auto* theme_provider = status_bubble_->base_view()->GetThemeProvider(); // Don't move this after AddChildView() since this function would trigger
SkColor bubble_color = // repaint which should not happen in the constructor.
theme_provider->GetColor(ThemeProperties::COLOR_STATUS_BUBBLE); SetTextLabelColors(text.get());
SkColor blended_text_color = color_utils::AlphaBlend(
theme_provider->GetColor(ThemeProperties::COLOR_TAB_TEXT), bubble_color,
0.6f);
text->SetEnabledColor(color_utils::GetColorWithMinimumContrast(
blended_text_color, bubble_color));
text->SetHorizontalAlignment(gfx::ALIGN_LEFT); text->SetHorizontalAlignment(gfx::ALIGN_LEFT);
text_ = AddChildView(std::move(text)); text_ = AddChildView(std::move(text));
} }
...@@ -264,6 +262,10 @@ void StatusBubbleViews::StatusView::Layout() { ...@@ -264,6 +262,10 @@ void StatusBubbleViews::StatusView::Layout() {
text_->SetBoundsRect(text_rect); text_->SetBoundsRect(text_rect);
} }
void StatusBubbleViews::StatusView::OnThemeChanged() {
SetTextLabelColors(text_);
}
void StatusBubbleViews::StatusView::SetText(const base::string16& text, void StatusBubbleViews::StatusView::SetText(const base::string16& text,
bool should_animate_open) { bool should_animate_open) {
if (text.empty()) { if (text.empty()) {
...@@ -427,6 +429,17 @@ void StatusBubbleViews::StatusView::SetWidth(int new_width) { ...@@ -427,6 +429,17 @@ void StatusBubbleViews::StatusView::SetWidth(int new_width) {
Layout(); Layout();
} }
void StatusBubbleViews::StatusView::SetTextLabelColors(views::Label* text) {
const auto* theme_provider = status_bubble_->base_view()->GetThemeProvider();
SkColor bubble_color =
theme_provider->GetColor(ThemeProperties::COLOR_STATUS_BUBBLE);
text->SetBackgroundColor(bubble_color);
// Text color is the foreground tab text color at 60% alpha.
text->SetEnabledColor(color_utils::AlphaBlend(
theme_provider->GetColor(ThemeProperties::COLOR_TAB_TEXT), bubble_color,
0.6f));
}
const char* StatusBubbleViews::StatusView::GetClassName() const { const char* StatusBubbleViews::StatusView::GetClassName() const {
return "StatusBubbleViews::StatusView"; return "StatusBubbleViews::StatusView";
} }
...@@ -744,6 +757,11 @@ int StatusBubbleViews::GetWidthForURL(const base::string16& url_string) { ...@@ -744,6 +757,11 @@ int StatusBubbleViews::GetWidthForURL(const base::string16& url_string) {
kTextHorizPadding + 1; kTextHorizPadding + 1;
} }
void StatusBubbleViews::OnThemeChanged() {
if (popup_)
popup_->ThemeChanged();
}
void StatusBubbleViews::SetStatus(const base::string16& status_text) { void StatusBubbleViews::SetStatus(const base::string16& status_text) {
if (size_.IsEmpty()) if (size_.IsEmpty())
return; // We have no bounds, don't attempt to show the popup. return; // We have no bounds, don't attempt to show the popup.
......
...@@ -61,6 +61,9 @@ class StatusBubbleViews : public StatusBubble { ...@@ -61,6 +61,9 @@ class StatusBubbleViews : public StatusBubble {
// Gets the width that a bubble should be for a given string // Gets the width that a bubble should be for a given string
int GetWidthForURL(const base::string16& url_string); int GetWidthForURL(const base::string16& url_string);
// Notifies the bubble's popup that browser's theme is changed.
void OnThemeChanged();
// Overridden from StatusBubble: // Overridden from StatusBubble:
void SetStatus(const base::string16& status) override; void SetStatus(const base::string16& status) override;
void SetURL(const GURL& url) override; void SetURL(const GURL& url) override;
......
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