Commit c7cb3e90 authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

Fix LabelButtonTest.ButtonStyleIsDefaultStyle,HighlightedButtonStyle

Since r478352 SecondaryUI buttons on Mac have all been MdTextButtons,
so we don't really care how regular LabelButtons look on Mac.

Remove some mac-specific logic from PlatformStyle and the
label_button_unittest.cc to make these tests pass. Some tests
currently fail just because the text color changes from solid
black/white to something different under MD due to logic in
ui::NativeTheme

Bug: 713030
Change-Id: I3b925918024d6c260da007ceb96900a2afb557a4
Reviewed-on: https://chromium-review.googlesource.com/741401Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512704}
parent 4417837d
...@@ -51,17 +51,6 @@ TEST_F(BlueButtonTest, Border) { ...@@ -51,17 +51,6 @@ TEST_F(BlueButtonTest, Border) {
widget->GetContentsView()->AddChildView(blue_button); widget->GetContentsView()->AddChildView(blue_button);
blue_button->SizeToPreferredSize(); blue_button->SizeToPreferredSize();
#if defined(OS_MACOSX)
// On Mac, themed STYLE_BUTTON buttons provide blue theming for dialog-default
// buttons. This makes it unlikely that they will appear together with a
// BlueButton on the same dialog. So the sizes don't really need to be
// consistent. However, for the purposes of this test (e.g. to ensure we don't
// accidentally make BlueButtons look like themed buttons on Mac), force the
// sizes to match (ignoring the minimum size) so that the bitmaps can be
// compared.
EXPECT_NE(button->size(), blue_button->size()); // Verify this is needed.
blue_button->SetSize(button->size());
#endif
SkBitmap blue_button_bitmap; SkBitmap blue_button_bitmap;
blue_button_bitmap.allocN32Pixels(blue_button->size().width(), blue_button_bitmap.allocN32Pixels(blue_button->size().width(),
......
...@@ -189,7 +189,7 @@ gfx::Size LabelButton::CalculatePreferredSize() const { ...@@ -189,7 +189,7 @@ gfx::Size LabelButton::CalculatePreferredSize() const {
label.SetLineHeight(label_->line_height()); label.SetLineHeight(label_->line_height());
label.SetShadows(label_->shadows()); label.SetShadows(label_->shadows());
if (style_ == STYLE_BUTTON && PlatformStyle::kDefaultLabelButtonHasBoldFont) { if (style_ == STYLE_BUTTON) {
// Some text appears wider when rendered normally than when rendered bold. // Some text appears wider when rendered normally than when rendered bold.
// Accommodate the widest, as buttons may show bold and shouldn't resize. // Accommodate the widest, as buttons may show bold and shouldn't resize.
const int current_width = label.GetPreferredSize().width(); const int current_width = label.GetPreferredSize().width();
...@@ -468,13 +468,11 @@ void LabelButton::UpdateStyleToIndicateDefaultStatus() { ...@@ -468,13 +468,11 @@ void LabelButton::UpdateStyleToIndicateDefaultStatus() {
// never be given default status. // never be given default status.
DCHECK_EQ(cached_normal_font_list_.GetFontSize(), DCHECK_EQ(cached_normal_font_list_.GetFontSize(),
label()->font_list().GetFontSize()); label()->font_list().GetFontSize());
const bool bold =
PlatformStyle::kDefaultLabelButtonHasBoldFont && is_default_;
// TODO(tapted): This should use style::GetFont(), but this part can just be // TODO(tapted): This should use style::GetFont(), but this part can just be
// deleted when default buttons no longer go bold. Colors will need updating // deleted when default buttons no longer go bold. Colors will need updating
// still. // still.
label_->SetFontList(bold ? cached_default_button_font_list_ label_->SetFontList(is_default_ ? cached_default_button_font_list_
: cached_normal_font_list_); : cached_normal_font_list_);
InvalidateLayout(); InvalidateLayout();
ResetLabelEnabledColor(); ResetLabelEnabledColor();
} }
...@@ -547,10 +545,7 @@ void LabelButton::ResetCachedPreferredSize() { ...@@ -547,10 +545,7 @@ void LabelButton::ResetCachedPreferredSize() {
} }
void LabelButton::ResetLabelEnabledColor() { void LabelButton::ResetLabelEnabledColor() {
const SkColor color = const SkColor color = button_state_colors_[state()];
explicitly_set_colors_[state()]
? button_state_colors_[state()]
: PlatformStyle::TextColorForButton(button_state_colors_, *this);
if (state() != STATE_DISABLED && label_->enabled_color() != color) if (state() != STATE_DISABLED && label_->enabled_color() != color)
label_->SetEnabledColor(color); label_->SetEnabledColor(color);
} }
......
...@@ -97,8 +97,6 @@ class LabelButtonTest : public test::WidgetTest { ...@@ -97,8 +97,6 @@ class LabelButtonTest : public test::WidgetTest {
styled_highlight_text_color_ = styled_normal_text_color_ = styled_highlight_text_color_ = styled_normal_text_color_ =
button_->GetNativeTheme()->GetSystemColor( button_->GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_ButtonEnabledColor); ui::NativeTheme::kColorId_ButtonEnabledColor);
#elif defined(OS_MACOSX)
styled_highlight_text_color_ = SK_ColorWHITE;
#else #else
styled_highlight_text_color_ = styled_normal_text_color_; styled_highlight_text_color_ = styled_normal_text_color_;
#endif #endif
...@@ -398,26 +396,13 @@ TEST_F(LabelButtonTest, ButtonStyleIsDefaultStyle) { ...@@ -398,26 +396,13 @@ TEST_F(LabelButtonTest, ButtonStyleIsDefaultStyle) {
button->SizeToPreferredSize(); button->SizeToPreferredSize();
button->Layout(); button->Layout();
EXPECT_EQ(styled_highlight_text_color_, button->label()->enabled_color()); EXPECT_EQ(styled_highlight_text_color_, button->label()->enabled_color());
if (PlatformStyle::kDefaultLabelButtonHasBoldFont) { EXPECT_NE(non_default_size, button->label()->size());
EXPECT_NE(non_default_size, button->label()->size()); EXPECT_EQ(button->label()->font_list().GetFontWeight(),
EXPECT_EQ(button->label()->font_list().GetFontWeight(), gfx::Font::Weight::BOLD);
gfx::Font::Weight::BOLD);
} else {
EXPECT_EQ(non_default_size, button->label()->size());
EXPECT_EQ(button->label()->font_list().GetFontWeight(),
gfx::Font::Weight::NORMAL);
}
} }
// Ensure the label gets the correct style when pressed or becoming default. // Ensure the label gets the correct style when pressed or becoming default.
TEST_F(LabelButtonTest, HighlightedButtonStyle) { TEST_F(LabelButtonTest, HighlightedButtonStyle) {
#if defined(OS_MACOSX)
// On Mac, ensure the normal and highlight colors are different, to ensure the
// tests are actually testing something. This might be the case on other
// platforms.
EXPECT_NE(styled_normal_text_color_, styled_highlight_text_color_);
#endif
// For STYLE_TEXTBUTTON, the NativeTheme might not provide SK_ColorBLACK, but // For STYLE_TEXTBUTTON, the NativeTheme might not provide SK_ColorBLACK, but
// it should be the same for normal and pressed states. // it should be the same for normal and pressed states.
EXPECT_EQ(themed_normal_text_color_, button_->label()->enabled_color()); EXPECT_EQ(themed_normal_text_color_, button_->label()->enabled_color());
......
...@@ -25,7 +25,7 @@ ...@@ -25,7 +25,7 @@
namespace views { namespace views {
namespace { namespace {
#if !defined(DESKTOP_LINUX) && !defined(OS_MACOSX) #if !defined(DESKTOP_LINUX)
// Default text and shadow colors for STYLE_BUTTON. // Default text and shadow colors for STYLE_BUTTON.
const SkColor kStyleButtonTextColor = SK_ColorBLACK; const SkColor kStyleButtonTextColor = SK_ColorBLACK;
const SkColor kStyleButtonShadowColor = SK_ColorWHITE; const SkColor kStyleButtonShadowColor = SK_ColorWHITE;
...@@ -37,7 +37,6 @@ const SkColor kStyleButtonShadowColor = SK_ColorWHITE; ...@@ -37,7 +37,6 @@ const SkColor kStyleButtonShadowColor = SK_ColorWHITE;
const int PlatformStyle::kMinLabelButtonWidth = 70; const int PlatformStyle::kMinLabelButtonWidth = 70;
const int PlatformStyle::kMinLabelButtonHeight = 33; const int PlatformStyle::kMinLabelButtonHeight = 33;
const bool PlatformStyle::kDefaultLabelButtonHasBoldFont = true;
const bool PlatformStyle::kDialogDefaultButtonCanBeCancel = true; const bool PlatformStyle::kDialogDefaultButtonCanBeCancel = true;
const bool PlatformStyle::kSelectWordOnRightClick = false; const bool PlatformStyle::kSelectWordOnRightClick = false;
const bool PlatformStyle::kSelectAllOnRightClickWhenUnfocused = false; const bool PlatformStyle::kSelectAllOnRightClickWhenUnfocused = false;
...@@ -60,19 +59,12 @@ std::unique_ptr<ScrollBar> PlatformStyle::CreateScrollBar(bool is_horizontal) { ...@@ -60,19 +59,12 @@ std::unique_ptr<ScrollBar> PlatformStyle::CreateScrollBar(bool is_horizontal) {
#endif #endif
} }
// static
SkColor PlatformStyle::TextColorForButton(
const ButtonColorByState& color_by_state,
const LabelButton& button) {
return color_by_state[button.state()];
}
// static // static
void PlatformStyle::OnTextfieldEditFailed() {} void PlatformStyle::OnTextfieldEditFailed() {}
#endif // OS_MACOSX #endif // OS_MACOSX
#if !defined(DESKTOP_LINUX) && !defined(OS_MACOSX) #if !defined(DESKTOP_LINUX)
// static // static
void PlatformStyle::ApplyLabelButtonTextStyle( void PlatformStyle::ApplyLabelButtonTextStyle(
Label* label, Label* label,
......
...@@ -28,9 +28,6 @@ class VIEWS_EXPORT PlatformStyle { ...@@ -28,9 +28,6 @@ class VIEWS_EXPORT PlatformStyle {
static const int kMinLabelButtonWidth; static const int kMinLabelButtonWidth;
static const int kMinLabelButtonHeight; static const int kMinLabelButtonHeight;
// Whether dialog-default buttons are given a bold font style.
static const bool kDefaultLabelButtonHasBoldFont;
// Whether the default button for a dialog can be the Cancel button. // Whether the default button for a dialog can be the Cancel button.
static const bool kDialogDefaultButtonCanBeCancel; static const bool kDialogDefaultButtonCanBeCancel;
...@@ -68,10 +65,6 @@ class VIEWS_EXPORT PlatformStyle { ...@@ -68,10 +65,6 @@ class VIEWS_EXPORT PlatformStyle {
// Creates the default scrollbar for the given orientation. // Creates the default scrollbar for the given orientation.
static std::unique_ptr<ScrollBar> CreateScrollBar(bool is_horizontal); static std::unique_ptr<ScrollBar> CreateScrollBar(bool is_horizontal);
// Returns the current text color for the current button state.
static SkColor TextColorForButton(const ButtonColorByState& color_by_state,
const LabelButton& button);
// Applies platform styles to |label| and fills |color_by_state| with the text // Applies platform styles to |label| and fills |color_by_state| with the text
// colors for normal, pressed, hovered, and disabled states, if the colors for // colors for normal, pressed, hovered, and disabled states, if the colors for
// Button::STYLE_BUTTON buttons differ from those provided by ui::NativeTheme. // Button::STYLE_BUTTON buttons differ from those provided by ui::NativeTheme.
......
...@@ -16,7 +16,6 @@ namespace views { ...@@ -16,7 +16,6 @@ namespace views {
const int PlatformStyle::kMinLabelButtonWidth = 32; const int PlatformStyle::kMinLabelButtonWidth = 32;
const int PlatformStyle::kMinLabelButtonHeight = 30; const int PlatformStyle::kMinLabelButtonHeight = 30;
const bool PlatformStyle::kDefaultLabelButtonHasBoldFont = false;
const bool PlatformStyle::kDialogDefaultButtonCanBeCancel = false; const bool PlatformStyle::kDialogDefaultButtonCanBeCancel = false;
const bool PlatformStyle::kSelectWordOnRightClick = true; const bool PlatformStyle::kSelectWordOnRightClick = true;
const bool PlatformStyle::kSelectAllOnRightClickWhenUnfocused = true; const bool PlatformStyle::kSelectAllOnRightClickWhenUnfocused = true;
...@@ -45,27 +44,6 @@ std::unique_ptr<ScrollBar> PlatformStyle::CreateScrollBar(bool is_horizontal) { ...@@ -45,27 +44,6 @@ std::unique_ptr<ScrollBar> PlatformStyle::CreateScrollBar(bool is_horizontal) {
return std::make_unique<CocoaScrollBar>(is_horizontal); return std::make_unique<CocoaScrollBar>(is_horizontal);
} }
// static
SkColor PlatformStyle::TextColorForButton(
const ButtonColorByState& color_by_state,
const LabelButton& button) {
Button::ButtonState state = button.state();
if (button.style() == Button::STYLE_BUTTON && button.is_default()) {
// For convenience, we currently assume Mac wants the color corresponding to
// the pressed state for default buttons.
state = Button::STATE_PRESSED;
}
return color_by_state[state];
}
// static
void PlatformStyle::ApplyLabelButtonTextStyle(
views::Label* label,
ButtonColorByState* color_by_state) {
ButtonColorByState& colors = *color_by_state;
colors[Button::STATE_PRESSED] = SK_ColorWHITE;
}
// static // static
void PlatformStyle::OnTextfieldEditFailed() { void PlatformStyle::OnTextfieldEditFailed() {
NSBeep(); NSBeep();
......
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