Commit 918a3654 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Allow single-line text to be top or bottom aligned.

This is a follow-up to
https://chromium-review.googlesource.com/c/chromium/src/+/1764942
which restricted vertical alignment of labels to multiline only.

It relaxes the restriction, allowing all render text to be top or bottom
aligned, though at the expense of the ability to guarantee baseline
alignment when mixed fonts are required to render a line (still a
feature of the default ALIGN_MIDDLE; a note has been put on the new
Label::SetVerticalAlignment() method to this effect).

Bug: 996905
Change-Id: I46d60902e91ea202f48ae2952b4c0cb118d80650
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1772449Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690877}
parent 89bbc693
...@@ -306,6 +306,7 @@ TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab) ...@@ -306,6 +306,7 @@ TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab)
new views::Label(base::string16(), CONTEXT_TAB_HOVER_CARD_TITLE, new views::Label(base::string16(), CONTEXT_TAB_HOVER_CARD_TITLE,
views::style::STYLE_PRIMARY); views::style::STYLE_PRIMARY);
title_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT); title_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
title_label_->SetVerticalAlignment(gfx::ALIGN_TOP);
title_label_->SetMultiLine(true); title_label_->SetMultiLine(true);
title_label_->SetMaxLines(kTitleMaxLines); title_label_->SetMaxLines(kTitleMaxLines);
title_label_->SetProperty(views::kFlexBehaviorKey, title_label_->SetProperty(views::kFlexBehaviorKey,
...@@ -557,14 +558,12 @@ void TabHoverCardBubbleView::UpdateCardContent(const Tab* tab) { ...@@ -557,14 +558,12 @@ void TabHoverCardBubbleView::UpdateCardContent(const Tab* tab) {
} }
base::string16 domain; base::string16 domain;
if (domain_url.SchemeIsFile()) { if (domain_url.SchemeIsFile()) {
title_label_->SetVerticalAlignment(gfx::ALIGN_MIDDLE);
title_label_->SetMultiLine(false); title_label_->SetMultiLine(false);
title_label_->SetElideBehavior(gfx::ELIDE_MIDDLE); title_label_->SetElideBehavior(gfx::ELIDE_MIDDLE);
domain = l10n_util::GetStringUTF16(IDS_HOVER_CARD_FILE_URL_SOURCE); domain = l10n_util::GetStringUTF16(IDS_HOVER_CARD_FILE_URL_SOURCE);
} else { } else {
title_label_->SetElideBehavior(gfx::ELIDE_TAIL); title_label_->SetElideBehavior(gfx::ELIDE_TAIL);
title_label_->SetMultiLine(true); title_label_->SetMultiLine(true);
title_label_->SetVerticalAlignment(gfx::ALIGN_TOP);
domain = url_formatter::FormatUrl( domain = url_formatter::FormatUrl(
domain_url, domain_url,
url_formatter::kFormatUrlOmitDefaults | url_formatter::kFormatUrlOmitDefaults |
......
...@@ -1020,7 +1020,6 @@ Vector2d RenderText::GetLineOffset(size_t line_number) { ...@@ -1020,7 +1020,6 @@ Vector2d RenderText::GetLineOffset(size_t line_number) {
Vector2d offset = display_rect().OffsetFromOrigin(); Vector2d offset = display_rect().OffsetFromOrigin();
// TODO(ckocagil): Apply the display offset for multiline scrolling. // TODO(ckocagil): Apply the display offset for multiline scrolling.
if (!multiline()) { if (!multiline()) {
DCHECK_EQ(ALIGN_MIDDLE, vertical_alignment_);
offset.Add(GetUpdatedDisplayOffset()); offset.Add(GetUpdatedDisplayOffset());
} else { } else {
DCHECK_LT(line_number, lines().size()); DCHECK_LT(line_number, lines().size());
...@@ -1417,14 +1416,20 @@ Vector2d RenderText::GetAlignmentOffset(size_t line_number) { ...@@ -1417,14 +1416,20 @@ Vector2d RenderText::GetAlignmentOffset(size_t line_number) {
offset.set_x((offset.x() + 1) / 2); offset.set_x((offset.x() + 1) / 2);
} }
if (!multiline_) switch (vertical_alignment_) {
offset.set_y(GetBaseline() - GetDisplayTextBaseline()); case ALIGN_TOP:
else if (vertical_alignment_ == ALIGN_TOP) offset.set_y(0);
offset.set_y(0); break;
else if (vertical_alignment_ == ALIGN_MIDDLE) case ALIGN_MIDDLE:
offset.set_y((display_rect_.height() - GetStringSize().height()) / 2); if (multiline_)
else offset.set_y((display_rect_.height() - GetStringSize().height()) / 2);
offset.set_y(display_rect_.height() - GetStringSize().height()); else
offset.set_y(GetBaseline() - GetDisplayTextBaseline());
break;
case ALIGN_BOTTOM:
offset.set_y(display_rect_.height() - GetStringSize().height());
break;
}
return offset; return offset;
} }
......
...@@ -208,9 +208,6 @@ gfx::VerticalAlignment Label::GetVerticalAlignment() const { ...@@ -208,9 +208,6 @@ gfx::VerticalAlignment Label::GetVerticalAlignment() const {
} }
void Label::SetVerticalAlignment(gfx::VerticalAlignment alignment) { void Label::SetVerticalAlignment(gfx::VerticalAlignment alignment) {
// TODO(crbug.com/996905): remove once single-line vertical alignment is
// supported.
DCHECK(GetMultiLine() || alignment == gfx::ALIGN_MIDDLE);
if (GetVerticalAlignment() == alignment) if (GetVerticalAlignment() == alignment)
return; return;
full_text_->SetVerticalAlignment(alignment); full_text_->SetVerticalAlignment(alignment);
...@@ -237,9 +234,6 @@ bool Label::GetMultiLine() const { ...@@ -237,9 +234,6 @@ bool Label::GetMultiLine() const {
void Label::SetMultiLine(bool multi_line) { void Label::SetMultiLine(bool multi_line) {
DCHECK(!multi_line || (elide_behavior_ == gfx::ELIDE_TAIL || DCHECK(!multi_line || (elide_behavior_ == gfx::ELIDE_TAIL ||
elide_behavior_ == gfx::NO_ELIDE)); elide_behavior_ == gfx::NO_ELIDE));
// TODO(crbug.com/996905): remove once single-line vertical alignment is
// supported.
DCHECK(multi_line || GetVerticalAlignment() == gfx::ALIGN_MIDDLE);
if (this->GetMultiLine() == multi_line) if (this->GetMultiLine() == multi_line)
return; return;
multi_line_ = multi_line; multi_line_ = multi_line;
......
...@@ -131,10 +131,9 @@ class VIEWS_EXPORT Label : public View, ...@@ -131,10 +131,9 @@ class VIEWS_EXPORT Label : public View,
// Gets/Sets the vertical alignment. Affects how whitespace is distributed // Gets/Sets the vertical alignment. Affects how whitespace is distributed
// vertically around the label text, or if the label is not tall enough to // vertically around the label text, or if the label is not tall enough to
// render all of the text, what gets cut off. // render all of the text, what gets cut off. ALIGN_MIDDLE is default and is
// // strongly suggested for single-line labels because it produces a consistent
// Currently, this must be ALIGN_MIDDLE (default) for non-multiline labels. // baseline even when rendering with mixed fonts.
// TODO(crbug.com/996905): support single-line vertical alignment.
gfx::VerticalAlignment GetVerticalAlignment() const; gfx::VerticalAlignment GetVerticalAlignment() const;
void SetVerticalAlignment(gfx::VerticalAlignment alignment); void SetVerticalAlignment(gfx::VerticalAlignment alignment);
......
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