Vertically align text on empty bookmarks bar

chrome/browser/ui/views/bookmarks/bookmark_bar_instructions_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_instructions_view.h"
 
 #include <algorithm>
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/defaults.h"
@@ skipping 22 matching lines @@
   return 6;
 }
 
 }  // namespace
 
 BookmarkBarInstructionsView::BookmarkBarInstructionsView(
     BookmarkBarInstructionsDelegate* delegate)
     : delegate_(delegate),
       instructions_(NULL),
       import_link_(NULL),
-      baseline_(-1),
       updated_colors_(false) {
   instructions_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_BOOKMARKS_NO_ITEMS));
   instructions_->SetAutoColorReadabilityEnabled(false);
   instructions_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   AddChildView(instructions_);
 
   if (browser_defaults::kShowImportOnBookmarkBar) {
     import_link_ = new views::Link(
         l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_IMPORT_LINK));
     // We don't want the link to alter tab navigation.
     import_link_->SetFocusBehavior(FocusBehavior::NEVER);
     import_link_->set_listener(this);
     import_link_->set_context_menu_controller(this);
     import_link_->SetAutoColorReadabilityEnabled(false);
     import_link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     AddChildView(import_link_);
   }
 }
 
 gfx::Size BookmarkBarInstructionsView::GetPreferredSize() const {
   int ascent = 0, descent = 0, height = 0, width = 0;
   for (int i = 0; i < child_count(); ++i) {
     const views::View* view = child_at(i);
     gfx::Size pref = view->GetPreferredSize();
     int baseline = view->GetBaseline();

[Peter Kasting, 2016/06/13 20:49:00]

Does this code need changing too?  Seems like it should stay in sync with
Layout().

[kylix_rd, 2016/06/13 20:59:43]

This code is merely for determining the preferred size of the control, which
will be the smallest bounding box around the text. In this case, the baseline
will be the proper offset from a 0 origin.

In the control's current incarnation, there are, at most, two child controls,
both of which will use the same font size. Even though this code is trying to
align all the controls on their baseline, in practice, this code seems to not
actually do anything.

I was reticent to remove it without getting more feedback.

[Peter Kasting, 2016/06/13 21:06:06]

I'd simplify it as much as possible.  Seems like you can skip the ascent +
descent stuff and just ask the font for its height directly since there's no
alignment to take into account.

[kylix_rd, 2016/06/13 21:17:20]

Done.

     if (baseline != -1) {
       ascent = std::max(ascent, baseline);
       descent = std::max(descent, pref.height() - baseline);
     } else {
       height = std::max(pref.height(), height);
     }
     width += pref.width();
   }
   width += (child_count() - 1) * GetViewPadding();
   if (ascent != 0)
     height = std::max(ascent + descent, height);
   return gfx::Size(width, height);
 }
 
 void BookmarkBarInstructionsView::Layout() {
   int remaining_width = width();
   int x = 0;
   for (int i = 0; i < child_count(); ++i) {
     views::View* view = child_at(i);
     gfx::Size pref = view->GetPreferredSize();
-    int baseline = view->GetBaseline();
-    int y;
-    if (baseline != -1 && baseline_ != -1)
-      y = baseline_ - baseline;
-    else
-      y = (height() - pref.height()) / 2;
     int view_width = std::min(remaining_width, pref.width());
-    view->SetBounds(x, y, view_width, pref.height());
+    view->SetBounds(x, 0, view_width, height());

[Peter Kasting, 2016/06/13 20:49:00]

How does this look in the attached bookmark bar?

[kylix_rd, 2016/06/13 20:59:43]

I'll admit that I'm not sure. How is attached vs. detached bookmarks bar
enabled/disabled?

I would imagine it shouldn't look any different because all this code does is
mimic what the LabelButton does with its embedded Label and LinkLabel controls.
The LabelButton::Layout() code sets the label_ bounds to be the full height of
the control itself.

[Peter Kasting, 2016/06/13 21:06:06]

Toggle with ctrl-shift-b.

[kylix_rd, 2016/06/13 21:17:20]

Ok, the alignment remains correct.

     x += view_width + GetViewPadding();
     remaining_width = std::max(0, width() - x);
   }
 }
 
 const char* BookmarkBarInstructionsView::GetClassName() const {
   return "BookmarkBarInstructionsView";
 }
 
 void BookmarkBarInstructionsView::OnThemeChanged() {
@@ skipping 45 matching lines @@
     // code (which only adjusts luminance) doesn't work well in this case.
     SkColor bg = theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR);
     SkColor link_color =
         GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
     bool link_has_contrast = color_utils::GetContrastRatio(link_color, bg) >=
                              color_utils::kMinimumReadableContrastRatio;
     import_link_->SetUnderline(!link_has_contrast);
     import_link_->SetEnabledColor(link_has_contrast ? link_color : text_color);
   }
 }