Correct padding and focus ring for frameless constrained window dialog

chrome/browser/ui/views/constrained_window_frame_simple.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/constrained_window_frame_simple.h"
 
 #include "chrome/browser/ui/constrained_window.h"
 #include "chrome/browser/ui/views/constrained_window_views.h"
 #include "grit/ui_resources.h"
 #include "grit/chromium_strings.h"
@@ skipping 33 matching lines @@
  private:
   views::Widget* container_;
 
   DISALLOW_COPY_AND_ASSIGN(HeaderLayout);
 };
 
 void HeaderLayout::Layout(views::View* host) {
   if (!host->has_children())
     return;
 
-  int horizontal_padding = ConstrainedWindow::kHorizontalPadding;
-  int vertical_padding = ConstrainedWindow::kVerticalPadding;
-  int row_padding = ConstrainedWindow::kRowPadding;
+  int top_padding = ConstrainedWindow::kCloseButtonPadding;
+  int left_padding = ConstrainedWindow::kHorizontalPadding;
+  int bottom_padding = ConstrainedWindow::kRowPadding;
+  int right_padding = ConstrainedWindow::kCloseButtonPadding;
 
   views::View* header = host->child_at(0);
   gfx::Size preferred_size = GetPreferredSize(host);
-  int width = preferred_size.width() - 2 * horizontal_padding;
-  int height = preferred_size.height() - vertical_padding - row_padding;
+  int width = preferred_size.width() - left_padding - right_padding;
+  int height = preferred_size.height() - top_padding - bottom_padding;
 
-  header->SetBounds(horizontal_padding, vertical_padding, width, height);
+  header->SetBounds(left_padding, top_padding, width, height);
 }
 
 gfx::Size HeaderLayout::GetPreferredSize(views::View* host) {
-  int horizontal_padding = ConstrainedWindow::kHorizontalPadding;
-  int vertical_padding = ConstrainedWindow::kVerticalPadding;
-  int row_padding = ConstrainedWindow::kRowPadding;
+  int left_padding = ConstrainedWindow::kHorizontalPadding;
+  int top_padding = ConstrainedWindow::kCloseButtonPadding;
+  int right_padding = ConstrainedWindow::kCloseButtonPadding;
+  int bottom_padding = ConstrainedWindow::kRowPadding;
 
   views::View* header = host->child_at(0);
   views::View* client_view = container_->client_view();
 
   gfx::Size header_size = header ? header->GetPreferredSize() : gfx::Size();
   gfx::Size client_size =
       client_view ? client_view->GetPreferredSize() : gfx::Size();
   int width = std::max(client_size.width(), header_size.width()) +
-      2 * horizontal_padding;
-  int height = vertical_padding + header_size.height() + row_padding;
+      left_padding + right_padding;
+  int height = header_size.height() + top_padding + bottom_padding;
   return gfx::Size(width, height);
 }
 
 }  // namespace
 
 ConstrainedWindowFrameSimple::HeaderViews::HeaderViews(
     views::View* header,
     views::Label* title_label,
     views::Button* close_button)
     : header(header),
@@ skipping 12 matching lines @@
 
   SetHeaderView(CreateDefaultHeaderView());
 
   set_background(views::Background::CreateSolidBackground(
       ConstrainedWindow::GetBackgroundColor()));
 }
 
 ConstrainedWindowFrameSimple::~ConstrainedWindowFrameSimple() {
 }
 
 void ConstrainedWindowFrameSimple::SetHeaderView(HeaderViews* header_views)

[Mike Wittman, 2012/10/08 21:34:19]

The custom header padding needs to be applied here also.

[please use gerrit instead, 2012/10/09 18:38:09]

One of my upcoming CLs gives the ability to set custom margins through the
dialog delegate. I think that will address your concern.

 {
   RemoveAllChildViews(true);
 
   header_views_.reset(header_views);
 
   AddChildView(header_views_->header);
 }
 
+views::View* ConstrainedWindowFrameSimple::AddPadding(
+    views::View* viewToPad,
+    int top,
+    int left,
+    int bottom,
+    int right) {
+  views::View* result = new views::View();
+  views::GridLayout* layout = new views::GridLayout(result);
+  result->SetLayoutManager(layout);
+  layout->SetInsets(top, left, bottom, right);
+  views::ColumnSet* cs = layout->AddColumnSet(0);
+  cs->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
+                views::GridLayout::USE_PREF, 0, 0);
+  layout->StartRow(0, 0);
+  layout->AddView(viewToPad);
+  return result;
+}
+
 HeaderViews* ConstrainedWindowFrameSimple::CreateDefaultHeaderView() {
+  // kVerticalPadding - kCloseButtonPadding - title ascent height
+  // = 20 - 4 - 5 = 11.
+  const int kTitleTopPadding = 11;
+  const int kTitleLeftPadding = 0;
+  const int kTitleBottomPadding = 0;
+  const int kTitleRightPadding = 0;
+
   views::View* header_view = new views::View;
 
   views::GridLayout* grid_layout = new views::GridLayout(header_view);
   header_view->SetLayoutManager(grid_layout);
 
   views::ColumnSet* header_cs = grid_layout->AddColumnSet(0);
   header_cs->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
                        views::GridLayout::USE_PREF, 0, 0);  // Title.
   header_cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing);
-  header_cs->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
-                       views::GridLayout::USE_PREF, 0, 0);  // Close Button.
+  header_cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
+                       0, views::GridLayout::USE_PREF, 0, 0);  // Close Button.
 
   // Header row.
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   grid_layout->StartRow(0, 0);
+
   views::Label* title_label = new views::Label();
   title_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   title_label->SetFont(rb.GetFont(ConstrainedWindow::kTitleFontStyle));
   title_label->SetEnabledColor(ConstrainedWindow::GetTextColor());
   title_label->SetText(container_->widget_delegate()->GetWindowTitle());
-  grid_layout->AddView(title_label);
+  grid_layout->AddView(AddPadding(title_label, kTitleTopPadding,
+      kTitleLeftPadding, kTitleBottomPadding, kTitleRightPadding));
 
   views::Button* close_button = CreateCloseButton();
   grid_layout->AddView(close_button);
 
   return new HeaderViews(header_view, title_label, close_button);
 }
 
 gfx::Rect ConstrainedWindowFrameSimple::GetBoundsForClientView() const {
-  int horizontal_padding = ConstrainedWindow::kHorizontalPadding;
-  int vertical_padding = ConstrainedWindow::kVerticalPadding;
-  int row_padding = ConstrainedWindow::kRowPadding;
   gfx::Size header_size =
       header_views_->header ?
       header_views_->header->GetPreferredSize() : gfx::Size();
 
-  return gfx::Rect(horizontal_padding,
-                   vertical_padding + header_size.height() + row_padding,
-                   std::max(0, width() - 2 * horizontal_padding),
-                   std::max(0, (height() - 2 * vertical_padding -
-                                header_size.height() - row_padding)));
+  int client_x = ConstrainedWindow::kHorizontalPadding;
+  int client_y = ConstrainedWindow::kCloseButtonPadding + header_size.height() +
+      ConstrainedWindow::kRowPadding;
+  int client_width = std::max(0, width() - client_x -
+                                 ConstrainedWindow::kHorizontalPadding);
+  int client_height = std::max(0, height() - client_y -
+                                  ConstrainedWindow::kVerticalPadding);
+
+  return gfx::Rect(client_x, client_y, client_width, client_height);
 }
 
 gfx::Rect ConstrainedWindowFrameSimple::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
-  int horizontal_padding = ConstrainedWindow::kHorizontalPadding;
-  int vertical_padding = ConstrainedWindow::kVerticalPadding;
-  int row_padding = ConstrainedWindow::kRowPadding;
   gfx::Size header_size =
       header_views_->header ?
       header_views_->header->GetPreferredSize() : gfx::Size();
 
-  int x = client_bounds.x() - horizontal_padding;
-  int y = client_bounds.y() - vertical_padding - header_size.height() -
-      row_padding;
-  int width = client_bounds.width() + 2 * horizontal_padding;
-  int height = client_bounds.height() + 2 * vertical_padding +
-      header_size.height() + row_padding;
+  int x = client_bounds.x() - ConstrainedWindow::kHorizontalPadding;
+  int y = client_bounds.y() - ConstrainedWindow::kCloseButtonPadding -
+      header_size.height() - ConstrainedWindow::kRowPadding;
+  int width = client_bounds.width() + ConstrainedWindow::kHorizontalPadding +
+      ConstrainedWindow::kCloseButtonPadding;
+  int height = client_bounds.height() + ConstrainedWindow::kCloseButtonPadding +
+      header_size.height() + ConstrainedWindow::kRowPadding;

[Mike Wittman, 2012/10/08 21:34:19]

This should also include ConstrainedWindow::kVerticalPadding to account for the
bottom part of the frame.

[please use gerrit instead, 2012/10/09 18:38:09]

Done.

 
   return gfx::Rect(x, y, width, height);
 }
 
 int ConstrainedWindowFrameSimple::NonClientHitTest(const gfx::Point& point) {
   if (!bounds().Contains(point))
     return HTNOWHERE;
   return HTCLIENT;
 }
 
@@ skipping 38 matching lines @@
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   views::ImageButton* close_button = new views::ImageButton(this);
   close_button->SetImage(views::CustomButton::BS_NORMAL,
                           rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X));
   close_button->SetImage(views::CustomButton::BS_HOT,
                           rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_HOVER));
   close_button->SetImage(views::CustomButton::BS_PUSHED,
                           rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_HOVER));
   return close_button;
 }