Commit 27ecaa37 authored by Tommy Martino's avatar Tommy Martino Committed by Commit Bot

[Autofill Views] Fixing Label Colors

This change solves two problems related to font colors:
* Changes passed down from UX have requested the use of the subtext/hint
  color for all text in footer rows.
* A bug causes text colors to change (permanently) to incorrect values
  when hover styles are applied. This was observed on Linux under GTK+
  theme only. We don't need to modify text colors during RefreshStyle
  anyway, so the simplest fix is to define these at initialization and
  not touch them again.

Change-Id: I80d2de91bb7067b3dc8fea6f369c83658e15c22a
Bug: 861883, 859887
Reviewed-on: https://chromium-review.googlesource.com/1130015
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarFabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573532}
parent fc810ef6
...@@ -138,12 +138,15 @@ class AutofillPopupItemView : public AutofillPopupRowView { ...@@ -138,12 +138,15 @@ class AutofillPopupItemView : public AutofillPopupRowView {
views::MenuConfig::instance().touchable_menu_height + extra_height_); views::MenuConfig::instance().touchable_menu_height + extra_height_);
// TODO(crbug.com/831603): Remove elision responsibilities from controller. // TODO(crbug.com/831603): Remove elision responsibilities from controller.
text_label_ = new views::Label( views::Label* text_label = new views::Label(
controller_->GetElidedValueAt(line_number_), controller_->GetElidedValueAt(line_number_),
{views::style::GetFont(ChromeTextContext::CONTEXT_BODY_TEXT_LARGE, {views::style::GetFont(ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
views::style::TextStyle::STYLE_PRIMARY)}); GetPrimaryTextStyle())});
text_label->SetEnabledColor(views::style::GetColor(
*this, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
GetPrimaryTextStyle()));
AddChildView(text_label_); AddChildView(text_label);
AddSpacerWithSize(views::MenuConfig::instance().item_horizontal_padding, AddSpacerWithSize(views::MenuConfig::instance().item_horizontal_padding,
/*resize=*/true, layout); /*resize=*/true, layout);
...@@ -151,12 +154,15 @@ class AutofillPopupItemView : public AutofillPopupRowView { ...@@ -151,12 +154,15 @@ class AutofillPopupItemView : public AutofillPopupRowView {
const base::string16& description_text = const base::string16& description_text =
controller_->GetElidedLabelAt(line_number_); controller_->GetElidedLabelAt(line_number_);
if (!description_text.empty()) { if (!description_text.empty()) {
subtext_label_ = new views::Label( views::Label* subtext_label = new views::Label(
description_text, description_text,
{views::style::GetFont(ChromeTextContext::CONTEXT_BODY_TEXT_LARGE, {views::style::GetFont(ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
ChromeTextStyle::STYLE_HINT)}); ChromeTextStyle::STYLE_SECONDARY)});
subtext_label->SetEnabledColor(views::style::GetColor(
*this, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
ChromeTextStyle::STYLE_SECONDARY));
AddChildView(subtext_label_); AddChildView(subtext_label);
} }
const gfx::ImageSkia icon = const gfx::ImageSkia icon =
...@@ -174,25 +180,12 @@ class AutofillPopupItemView : public AutofillPopupRowView { ...@@ -174,25 +180,12 @@ class AutofillPopupItemView : public AutofillPopupRowView {
void RefreshStyle() override { void RefreshStyle() override {
SetBackground(CreateBackground()); SetBackground(CreateBackground());
if (text_label_) {
int text_style = is_warning_ ? ChromeTextStyle::STYLE_RED
: views::style::TextStyle::STYLE_PRIMARY;
text_label_->SetEnabledColor(views::style::GetColor(
*this, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE, text_style));
}
if (subtext_label_) {
SkColor secondary = views::style::GetColor(
*this, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
ChromeTextStyle::STYLE_HINT);
subtext_label_->SetEnabledColor(secondary);
}
SchedulePaint(); SchedulePaint();
} }
protected:
virtual int GetPrimaryTextStyle() = 0;
private: private:
void AddSpacerWithSize(int spacer_width, void AddSpacerWithSize(int spacer_width,
bool resize, bool resize,
...@@ -204,8 +197,6 @@ class AutofillPopupItemView : public AutofillPopupRowView { ...@@ -204,8 +197,6 @@ class AutofillPopupItemView : public AutofillPopupRowView {
} }
const int extra_height_; const int extra_height_;
views::Label* text_label_ = nullptr;
views::Label* subtext_label_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(AutofillPopupItemView); DISALLOW_COPY_AND_ASSIGN(AutofillPopupItemView);
}; };
...@@ -224,13 +215,18 @@ class AutofillPopupSuggestionView : public AutofillPopupItemView { ...@@ -224,13 +215,18 @@ class AutofillPopupSuggestionView : public AutofillPopupItemView {
} }
protected: protected:
// AutofillPopupRowView: // AutofillPopupItemView:
std::unique_ptr<views::Background> CreateBackground() override { std::unique_ptr<views::Background> CreateBackground() override {
return views::CreateSolidBackground( return views::CreateSolidBackground(
is_selected_ ? kAutofillPopupSelectedBackgroundColor is_selected_ ? kAutofillPopupSelectedBackgroundColor
: kAutofillPopupBackgroundColor); : kAutofillPopupBackgroundColor);
} }
int GetPrimaryTextStyle() override {
return is_warning_ ? ChromeTextStyle::STYLE_RED
: views::style::TextStyle::STYLE_PRIMARY;
}
private: private:
AutofillPopupSuggestionView(AutofillPopupController* controller, AutofillPopupSuggestionView(AutofillPopupController* controller,
int line_number) int line_number)
...@@ -254,7 +250,7 @@ class AutofillPopupFooterView : public AutofillPopupItemView { ...@@ -254,7 +250,7 @@ class AutofillPopupFooterView : public AutofillPopupItemView {
} }
protected: protected:
// AutofillPopupRowView: // AutofillPopupItemView:
void CreateContent() override { void CreateContent() override {
SetBorder(views::CreateSolidSidedBorder( SetBorder(views::CreateSolidSidedBorder(
/*top=*/views::MenuConfig::instance().separator_thickness, /*top=*/views::MenuConfig::instance().separator_thickness,
...@@ -271,6 +267,10 @@ class AutofillPopupFooterView : public AutofillPopupItemView { ...@@ -271,6 +267,10 @@ class AutofillPopupFooterView : public AutofillPopupItemView {
: kAutofillPopupFooterBackgroundColor); : kAutofillPopupFooterBackgroundColor);
} }
int GetPrimaryTextStyle() override {
return ChromeTextStyle::STYLE_SECONDARY;
}
private: private:
AutofillPopupFooterView(AutofillPopupController* controller, int line_number) AutofillPopupFooterView(AutofillPopupController* controller, int line_number)
: AutofillPopupItemView(controller, line_number, GetCornerRadius()) { : AutofillPopupItemView(controller, line_number, GetCornerRadius()) {
......
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