Left align bubble title and text, adjusted spacing between.

chrome/browser/ui/views/global_error_bubble_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/global_error_bubble_view.h"
 
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/global_error/global_error.h"
@@ skipping 17 matching lines @@
 enum {
   TAG_ACCEPT_BUTTON = 1,
   TAG_CANCEL_BUTTON,
 };
 
 const int kMaxBubbleViewWidth = 262;
 
 // The vertical inset of the wrench bubble anchor from the wrench menu button.
 const int kAnchorVerticalInset = 5;
 
-const int kBubblePadding = 6;
+// Align bubble text with the title.
+const int kBubbleLeftPadding = 19;
+
+const int kBubblePadding = 11;

[msw, 2015/02/27 03:25:50]

nit: this should just be 19px or 20px and combined with the left side constant.
This will make the text spaced evenly on both sides, and the insets will match
those of a standard dialog. (in a followup, we can try to use DialogClientView)

[xiaoling, 2015/02/27 18:00:38]

Done.

 
 }  // namespace
 
 // GlobalErrorBubbleViewBase ---------------------------------------------------
 
 // static
 GlobalErrorBubbleViewBase* GlobalErrorBubbleViewBase::ShowStandardBubbleView(
     Browser* browser,
     const base::WeakPtr<GlobalErrorWithStandardBubble>& error) {
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
@@ skipping 50 matching lines @@
   if (!cancel_string.empty()) {
     cancel_button.reset(new views::LabelButton(this, cancel_string));
     cancel_button->SetStyle(views::Button::STYLE_BUTTON);
     cancel_button->set_tag(TAG_CANCEL_BUTTON);
   }
 
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
 
   // BubbleFrameView adds enough padding below title, no top padding needed.
-  layout->SetInsets(0, kBubblePadding, kBubblePadding, kBubblePadding);
+  set_margins(gfx::Insets(0, kBubbleLeftPadding,
+                          kBubblePadding, kBubblePadding));

[xiaoling, 2015/02/27 02:59:34]

I thought I have moved set_margins to the top of the function, but apparently
not or not uploaded. I'll do that tomorrow.

[msw, 2015/02/27 03:25:50]

Acknowledged.

 
   // First row, message labels.
   views::ColumnSet* cs = layout->AddColumnSet(0);
   cs->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                 1, views::GridLayout::USE_PREF, 0, 0);
 
   // Second row, accept and cancel button.
   cs = layout->AddColumnSet(1);
   cs->AddPaddingColumn(1, views::kRelatedControlHorizontalSpacing);
   cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
                 0, views::GridLayout::USE_PREF, 0, 0);
   if (cancel_button.get()) {
     cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
     cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
                   0, views::GridLayout::USE_PREF, 0, 0);
   }
 
   for (size_t i = 0; i < message_labels.size(); ++i) {
     layout->StartRow(1, 0);
     layout->AddView(message_labels[i]);
     if (i < message_labels.size() - 1)
       layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   }
   layout->AddPaddingRow(0, views::kLabelToControlVerticalSpacing);

[msw, 2015/02/27 03:25:50]

I think this 8px padding between the message and the buttons (visually ~13px)
should be increased to match the padding between the title and the message
(visually ~19px). So, I guess replace this with a local constant of ~14px.

[xiaoling, 2015/02/27 18:00:38]

Done.

 
   layout->StartRow(0, 1);
   layout->AddView(accept_button.release());
   if (cancel_button.get())
     layout->AddView(cancel_button.release());
 
   // Adjust the message label size in case buttons are too long.
   for (size_t i = 0; i < message_labels.size(); ++i)
     message_labels[i]->SizeToFit(layout->GetPreferredSize(this).width());
 
@@ skipping 41 matching lines @@
     error_->BubbleViewDidClose(browser_);
 }
 
 bool GlobalErrorBubbleView::ShouldShowCloseButton() const {
   return error_ && error_->ShouldShowCloseButton();
 }
 
 void GlobalErrorBubbleView::CloseBubbleView() {
   GetWidget()->Close();
 }