Commit 88494df3 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Multiline label minimum size should respect min line height.

Currently, a multiline label will in many cases report a minimum size
that can be smaller than the appropriate line-height for the text style,
causing vertical layouts to short the size of the text by a few pixels.

This results in, for example, a visual up-and-down wobble in the
domain label in a hover card as it animates between tabs, due to the
changing hover card size. This CL fixes this issue.

Note that except in vertical flex (and possibly box) layouts minimum
size tends to not be factored in, so this should only affect a handlful
of situations (and likely produce a better visual result).

Change-Id: Icdc6006b8f51588009b0709773f1854ecf5a6a2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910807
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714548}
parent 5de176f3
...@@ -460,7 +460,8 @@ gfx::Size Label::GetMinimumSize() const { ...@@ -460,7 +460,8 @@ gfx::Size Label::GetMinimumSize() const {
if (!GetVisible() && collapse_when_hidden_) if (!GetVisible() && collapse_when_hidden_)
return gfx::Size(); return gfx::Size();
gfx::Size size(0, font_list().GetHeight()); // Always reserve vertical space for at least one line.
gfx::Size size(0, std::max(font_list().GetHeight(), GetLineHeight()));
if (elide_behavior_ == gfx::ELIDE_HEAD || if (elide_behavior_ == gfx::ELIDE_HEAD ||
elide_behavior_ == gfx::ELIDE_MIDDLE || elide_behavior_ == gfx::ELIDE_MIDDLE ||
elide_behavior_ == gfx::ELIDE_TAIL || elide_behavior_ == gfx::ELIDE_TAIL ||
...@@ -479,6 +480,7 @@ gfx::Size Label::GetMinimumSize() const { ...@@ -479,6 +480,7 @@ gfx::Size Label::GetMinimumSize() const {
size.SetToMin(GetTextSize()); size.SetToMin(GetTextSize());
} }
} }
size.Enlarge(GetInsets().width(), GetInsets().height()); size.Enlarge(GetInsets().width(), GetInsets().height());
return size; return size;
} }
......
...@@ -326,6 +326,54 @@ TEST_F(LabelTest, AlignmentProperty) { ...@@ -326,6 +326,54 @@ TEST_F(LabelTest, AlignmentProperty) {
EXPECT_EQ(was_rtl, base::i18n::IsRTL()); EXPECT_EQ(was_rtl, base::i18n::IsRTL());
} }
TEST_F(LabelTest, MinimumSizeRespectsLineHeight) {
base::string16 text(ASCIIToUTF16("This is example text."));
label()->SetText(text);
const gfx::Size minimum_size = label()->GetMinimumSize();
const int expected_height = minimum_size.height() + 10;
label()->SetLineHeight(expected_height);
EXPECT_EQ(expected_height, label()->GetMinimumSize().height());
}
TEST_F(LabelTest, MinimumSizeRespectsLineHeightMultiline) {
base::string16 text(ASCIIToUTF16("This is example text."));
label()->SetText(text);
label()->SetMultiLine(true);
const gfx::Size minimum_size = label()->GetMinimumSize();
const int expected_height = minimum_size.height() + 10;
label()->SetLineHeight(expected_height);
EXPECT_EQ(expected_height, label()->GetMinimumSize().height());
}
TEST_F(LabelTest, MinimumSizeRespectsLineHeightWithInsets) {
base::string16 text(ASCIIToUTF16("This is example text."));
label()->SetText(text);
const gfx::Size minimum_size = label()->GetMinimumSize();
int expected_height = minimum_size.height() + 10;
label()->SetLineHeight(expected_height);
constexpr gfx::Insets kInsets{2, 3, 4, 5};
expected_height += kInsets.height();
label()->SetBorder(CreateEmptyBorder(kInsets));
EXPECT_EQ(expected_height, label()->GetMinimumSize().height());
}
TEST_F(LabelTest, MinimumSizeRespectsLineHeightMultilineWithInsets) {
base::string16 text(ASCIIToUTF16("This is example text."));
label()->SetText(text);
label()->SetMultiLine(true);
const gfx::Size minimum_size = label()->GetMinimumSize();
int expected_height = minimum_size.height() + 10;
label()->SetLineHeight(expected_height);
constexpr gfx::Insets kInsets{2, 3, 4, 5};
expected_height += kInsets.height();
label()->SetBorder(CreateEmptyBorder(kInsets));
EXPECT_EQ(expected_height, label()->GetMinimumSize().height());
}
TEST_F(LabelTest, ElideBehavior) { TEST_F(LabelTest, ElideBehavior) {
base::string16 text(ASCIIToUTF16("This is example text.")); base::string16 text(ASCIIToUTF16("This is example text."));
label()->SetText(text); label()->SetText(text);
......
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