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 16 matching lines @@
 #include "ui/views/widget/widget_delegate.h"
 
 namespace {
 
 typedef ConstrainedWindowFrameSimple::HeaderViews HeaderViews;
 
 // A layout manager that lays out the header view with proper padding,
 // and sized to the widget's client view.
 class HeaderLayout : public views::LayoutManager {
  public:
-  explicit HeaderLayout(views::Widget* container) : container_(container) {}
+  explicit HeaderLayout(
+      views::Widget* container,
+      int client_left_inset,
+      int client_right_inset);
   virtual ~HeaderLayout() {}
 
   // Overridden from LayoutManager
   virtual void Layout(views::View* host);
   virtual gfx::Size GetPreferredSize(views::View* host);
 
  private:
   views::Widget* container_;
+  int client_left_inset_;
+  int client_right_inset_;
 
   DISALLOW_COPY_AND_ASSIGN(HeaderLayout);
 };
 
+HeaderLayout::HeaderLayout(
+    views::Widget* container,
+    int client_left_inset,
+    int client_right_inset)
+    : container_(container),
+      client_left_inset_(client_left_inset),
+      client_right_inset_(client_right_inset) {}
+
 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 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;
 
-  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;
 
   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;
+  int width = std::max(
+      client_left_inset_ + client_size.width() + client_right_inset_,
+      left_padding + header_size.width() + right_padding);
+  int height = header_size.height() + top_padding;
   return gfx::Size(width, height);
 }
 
 }  // namespace
 
 ConstrainedWindowFrameSimple::HeaderViews::HeaderViews(
     views::View* header,
     views::Label* title_label,
     views::Button* close_button)
     : header(header),
       title_label(title_label),
       close_button(close_button) {
   DCHECK(header);
 }
 
 ConstrainedWindowFrameSimple::ConstrainedWindowFrameSimple(
     ConstrainedWindowViews* container)
-    : container_(container) {
+    : container_(container),
+      client_insets_(ConstrainedWindow::kClientTopPadding,

[Peter Kasting, 2012/10/10 20:22:26]

I didn't totally grasp why you wanted this member.  You said "ConstrainedWindow
will need to override the insets".  How is that going to happen, since there
doesn't currently seem to be any access to this member?  It seems like the right
way would be for the ConstrainedWindow to delete the border, or specify that it
shouldn't be added in the first place, maybe?

[please use gerrit instead, 2012/10/11 00:34:57]

I have not decided how this is going to be done. For now, let's just get rid of
the member.

+                     ConstrainedWindow::kHorizontalPadding,
+                     ConstrainedWindow::kClientBottomPadding,
+                     ConstrainedWindow::kHorizontalPadding) {
   container_->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM);
 
-  layout_ = new HeaderLayout(container_);
+  layout_ = new HeaderLayout(container_, client_insets_.left(),
+                                         client_insets_.right());

[Peter Kasting, 2012/10/10 20:22:26]

It seems like the header shouldn't have to worry about these insets since the
border should already be taking care of them.

[please use gerrit instead, 2012/10/11 00:34:57]

Done.

   SetLayoutManager(layout_);
 
   SetHeaderView(CreateDefaultHeaderView());
 
   set_background(views::Background::CreateSolidBackground(
       ConstrainedWindow::GetBackgroundColor()));
+
+  set_border(views::Border::CreateEmptyBorder(client_insets_.top(),
+                                              client_insets_.left(),
+                                              client_insets_.bottom(),
+                                              client_insets_.right()));
 }
 
 ConstrainedWindowFrameSimple::~ConstrainedWindowFrameSimple() {
 }
 
 void ConstrainedWindowFrameSimple::SetHeaderView(HeaderViews* header_views)
 {
   RemoveAllChildViews(true);
 
   header_views_.reset(header_views);
 
   AddChildView(header_views_->header);
 }
 
 HeaderViews* ConstrainedWindowFrameSimple::CreateDefaultHeaderView() {
+  const int kTitleTopPadding = ConstrainedWindow::kTitleTopPadding -
+      ConstrainedWindow::kCloseButtonPadding;
+  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());
+  title_label->set_border(views::Border::CreateEmptyBorder(kTitleTopPadding,
+      kTitleLeftPadding, kTitleBottomPadding, kTitleRightPadding));
   grid_layout->AddView(title_label);
 
   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)));
+  gfx::Rect bounds(GetContentsBounds());
+  if (header_views_->header)
+    bounds.Inset(0, header_views_->header->GetPreferredSize().height(), 0, 0);
+  return bounds;
 }
 
 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;
-
-  return gfx::Rect(x, y, width, height);
+  gfx::Rect bounds(client_bounds);
+  bounds.Inset(-GetInsets());
+  if (header_views_->header)
+    bounds.Inset(0, -header_views_->header->GetPreferredSize().height(), 0, 0);
+  return bounds;
 }
 
 int ConstrainedWindowFrameSimple::NonClientHitTest(const gfx::Point& point) {
   if (!bounds().Contains(point))
     return HTNOWHERE;
   return HTCLIENT;
 }
 
 void ConstrainedWindowFrameSimple::GetWindowMask(const gfx::Size& size,
                                                  gfx::Path* window_mask) {
@@ skipping 39 matching lines @@
 }
 
 views::ImageButton* ConstrainedWindowFrameSimple::CreateCloseButton() {
   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));
+                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_PRESSED));
   return close_button;
 }