[Password Generation] Update Aura UI

chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc

 // Copyright 2014 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/autofill/password_generation_popup_view_views.h"
 
 #include "base/strings/string16.h"
 #include "chrome/browser/ui/autofill/password_generation_popup_controller.h"
+#include "chrome/browser/ui/autofill/popup_constants.h"
+#include "grit/theme_resources.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
 
 namespace autofill {
 
 namespace {
 
 // The amount of whitespace that is present when there is no padding. Used
 // to get the proper spacing in the help section.
-const int kHelpVerticalOffset = 3;
+const int kHelpVerticalOffset = 5;
+const int kPasswordSectionHeight = 62;
 
-// Class that shows the password and the suggestion side-by-side.
-class PasswordRow : public views::View {
+// Wrapper around just the text portions of the generation UI (password and
+// prompting text).
+class PasswordTextBox : public views::View {
  public:
-  PasswordRow(const base::string16& password,
-              const base::string16& suggestion,
-              const gfx::FontList& font_list,
-              int horizontal_border) {
-    set_clip_insets(gfx::Insets(
-        PasswordGenerationPopupView::kPasswordVerticalInset, 0,
-        PasswordGenerationPopupView::kPasswordVerticalInset, 0));
+  PasswordTextBox() {}
+  virtual ~PasswordTextBox() {}
+
+  // |suggestion_text| prompts the user to select the password,
+  // |generated_password| is the generated password, and |font_list| is the font
+  // used for all text in this class.
+  void Init(const base::string16& suggestion_text,
+            const base::string16& generated_password,
+            const gfx::FontList& font_list,
+            const SkColor& text_color) {
     views::BoxLayout* box_layout = new views::BoxLayout(
-        views::BoxLayout::kHorizontal, horizontal_border, 0, 0);
+        views::BoxLayout::kVertical, 0, 13, 5);
     box_layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_FILL);
+        views::BoxLayout::MAIN_AXIS_ALIGNMENT_START);
     SetLayoutManager(box_layout);
 
-    password_label_ = new views::Label(password, font_list);
-    password_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    AddChildView(password_label_);
+    views::Label* suggestion_label = new views::Label(
+        suggestion_text, font_list.DeriveWithStyle(gfx::Font::BOLD));
+    suggestion_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    suggestion_label->SetEnabledColor(text_color);
+    AddChildView(suggestion_label);
 
-    suggestion_label_ = new views::Label(suggestion, font_list);
-    suggestion_label_->SetHorizontalAlignment(gfx::ALIGN_RIGHT);
-    suggestion_label_->SetEnabledColor(
-        PasswordGenerationPopupView::kExplanatoryTextColor);
-    AddChildView(suggestion_label_);
+    views::Label* password_label =
+        new views::Label(generated_password, font_list);
+    password_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    password_label->SetEnabledColor(text_color);
+    AddChildView(password_label);
   }
-  virtual ~PasswordRow() {}
 
   // views::View:
   virtual bool CanProcessEventsWithinSubtree() const OVERRIDE {
     // Send events to the parent view for handling.
     return false;
   }
 
  private:
-  // Child views. Not owned.
-  views::Label* suggestion_label_;
-  views::Label* password_label_;
+  DISALLOW_COPY_AND_ASSIGN(PasswordTextBox);
+};
 
-  DISALLOW_COPY_AND_ASSIGN(PasswordRow);
+// Class that shows the generated password and associated UI (currently a key
+// image and some explanatory text).
+class PasswordBox : public views::View {
+ public:
+  PasswordBox() {}
+  virtual ~PasswordBox() {}
+
+  // |password| is the generated password, |suggestion| is the text prompting
+  // the user to select the password, |font_list| is the font used for all the
+  // text, and |horizontal_border| is the horizontal whitespace between the
+  // parent element and this object.
+  void Init(const base::string16& password,
+            const base::string16& suggestion,
+            const gfx::FontList& font_list,
+            const SkColor& text_color,
+            int horizontal_border) {
+    views::BoxLayout* box_layout = new views::BoxLayout(
+        views::BoxLayout::kHorizontal, horizontal_border, 0, 15);
+    box_layout->set_main_axis_alignment(
+        views::BoxLayout::MAIN_AXIS_ALIGNMENT_START);
+    SetLayoutManager(box_layout);
+
+    views::ImageView* key_image = new views::ImageView();
+    key_image->SetImage(
+        ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+            IDR_GENERATE_PASSWORD_KEY).ToImageSkia());
+    AddChildView(key_image);
+
+    PasswordTextBox* password_text_box = new PasswordTextBox();
+    password_text_box->Init(suggestion, password, font_list, text_color);
+    AddChildView(password_text_box);
+  }
+
+  // views::View:
+  virtual bool CanProcessEventsWithinSubtree() const OVERRIDE {
+    // Send events to the parent view for handling.
+    return false;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PasswordBox);
 };
 
 }  // namespace
 
 PasswordGenerationPopupViewViews::PasswordGenerationPopupViewViews(
     PasswordGenerationPopupController* controller,
     views::Widget* observing_widget)
     : AutofillPopupBaseView(controller, observing_widget),
       password_view_(NULL),
+      font_list_(ResourceBundle::GetSharedInstance().GetFontList(
+          ResourceBundle::SmallFont)),
       controller_(controller) {
   if (controller_->display_password())
     CreatePasswordView();
 
   help_label_ = new views::StyledLabel(controller_->HelpText(), this);
-  help_label_->SetBaseFontList(controller_->font_list());
+  help_label_->SetBaseFontList(font_list_);
+  help_label_->SetLineHeightMultiplier(1.15);
   views::StyledLabel::RangeStyleInfo default_style;
   default_style.color = kExplanatoryTextColor;
   help_label_->SetDefaultStyle(default_style);
 
-  views::StyledLabel::RangeStyleInfo link_style =
-      views::StyledLabel::RangeStyleInfo::CreateForLink();
-  link_style.color = kLinkColor;
-  help_label_->AddStyleRange(controller_->HelpTextLinkRange(), link_style);
+  help_label_->AddStyleRange(
+      controller_->HelpTextLinkRange(),
+      views::StyledLabel::RangeStyleInfo::CreateForLink());
 
-  help_label_->SetBoundsRect(controller_->help_bounds());
   help_label_->set_background(
       views::Background::CreateSolidBackground(
           kExplanatoryTextBackgroundColor));
   help_label_->SetBorder(views::Border::CreateEmptyBorder(
       controller_->kHelpVerticalPadding - kHelpVerticalOffset,
       controller_->kHorizontalPadding,
       0,
       controller_->kHorizontalPadding));
   AddChildView(help_label_);
 
   set_background(views::Background::CreateSolidBackground(kPopupBackground));
 }
 
 PasswordGenerationPopupViewViews::~PasswordGenerationPopupViewViews() {}
 
 void PasswordGenerationPopupViewViews::CreatePasswordView() {
   if (password_view_)
     return;
 
-  password_view_ = new PasswordRow(controller_->password(),
-                                   controller_->SuggestedText(),
-                                   controller_->font_list(),
-                                   controller_->kHorizontalPadding);
+  password_view_ = new PasswordBox();
+  static_cast<PasswordBox*>(password_view_)->Init(

[Evan Stade, 2014/06/26 00:06:04]

nit: prefer avoiding this cast. Perhaps Init() can return |this| for
convenience. Not sure if that's unconventional.

[Garrett Casto, 2014/06/26 01:48:53]

The problem is that password_view_ is currently just a view::Views, so you need
the cast to call Init(). Changed to make PasswordBox an inner class instead of
just being in an anonymous namespace, which gets rid of the cast.

[Evan Stade, 2014/06/26 03:28:08]

I understand that, but you could do:

PasswordBox* box = new PasswordBox();
box->Init();
password_view_ = box;

or, if you make the change I suggested to Init(),

password_view_ = (new PasswordBox())->Init();

Your solution is OK too.

[Garrett Casto, 2014/07/14 06:18:39]

Ah, misunderstood your second suggestion. Thanks for clarifying.

+      controller_->password(),
+      controller_->SuggestedText(),
+      font_list_,
+      kItemTextColor,

[Evan Stade, 2014/06/26 00:06:04]

Since this is a constant and PasswordBox is only used in one place, might as
well cook this into PasswordBox. Same goes for kHorizontalPadding

[Garrett Casto, 2014/06/26 01:48:53]

Done, though I had to make kItemTextColor (and friends) public as it was
protected in AutofillPopupBaseView.

[Evan Stade, 2014/06/26 03:28:08]

ah, I see

+      controller_->kHorizontalPadding);

[Evan Stade, 2014/06/26 00:06:04]

nit: kind of weird to refer to a static via an instance

[Garrett Casto, 2014/06/26 01:48:53]

Done.

+  password_view_->SetPosition(gfx::Point(kPopupBorderThickness,
+                                         kPopupBorderThickness));
+  password_view_->SizeToPreferredSize();
   AddChildView(password_view_);
 }
 
+gfx::Size PasswordGenerationPopupViewViews::GetBounds() {

[Evan Stade, 2014/06/26 00:06:04]

hmm, from this angle, this method name is a little confusing, because it sounds
related to View::GetBoundsInScreen() or View::bounds().

Really, this is more like GetPreferredSize(), so it should have a name like
GetPreferredSizeOfPasswordView(). (Ideally, you would actually need to implement
GetPreferredSize(), but instead the view is sized through some unorthodox dance
with the controller)

[Garrett Casto, 2014/06/26 01:48:53]

Done.

+  int height = kPopupBorderThickness;
+  if (controller_->display_password()) {
+    // Add divider height as well.
+    height += kPasswordSectionHeight + 1;
+  }
+  int width = controller_->GetMinimumWidth();
+  int popup_width = width - 2 * kPopupBorderThickness;
+  height += help_label_->GetHeightForWidth(popup_width) +
+      help_label_->GetInsets().height();
+  return gfx::Size(width, height + kPopupBorderThickness);
+}
+
 void PasswordGenerationPopupViewViews::Show() {
   DoShow();
 }
 
 void PasswordGenerationPopupViewViews::Hide() {
   // The controller is no longer valid after it hides us.
   controller_ = NULL;
 
   DoHide();
 }
 
 void PasswordGenerationPopupViewViews::UpdateBoundsAndRedrawPopup() {
-  // Currently the UI can change from not offering a password to offering
-  // a password (e.g. the user is editing a generated password and deletes it),
-  // but it can't change the other way around.
-  if (controller_->display_password())
-    CreatePasswordView();
 
   DoUpdateBoundsAndRedrawPopup();
 }
 
 void PasswordGenerationPopupViewViews::PasswordSelectionUpdated() {
   if (!password_view_)
     return;
 
   password_view_->set_background(
       views::Background::CreateSolidBackground(
           controller_->password_selected() ?
           kHoveredBackgroundColor :
           kPopupBackground));
 }
 
 void PasswordGenerationPopupViewViews::Layout() {
-  if (password_view_)
-    password_view_->SetBoundsRect(controller_->password_bounds());
-
-  help_label_->SetBoundsRect(controller_->help_bounds());
+  // Need to leave room for the border.
+  int y = kPopupBorderThickness;
+  int popup_width = bounds().width() - 2 * kPopupBorderThickness;
+  if (controller_->display_password()) {
+    // Currently the UI can change from not offering a password to offering
+    // a password (e.g. the user is editing a generated password and deletes
+    // it), but it can't change the other way around.
+    CreatePasswordView();
+    password_view_->SetBounds(
+        kPopupBorderThickness, y, popup_width, kPasswordSectionHeight);
+    divider_bounds_ =
+        gfx::Rect(kPopupBorderThickness, password_view_->bounds().bottom(),
+                  popup_width, 1);
+    y = divider_bounds_.bottom();
+  }

[Evan Stade, 2014/06/26 00:06:04]

nit: \n

[Garrett Casto, 2014/06/26 01:48:53]

Done.

+  help_label_->SetBounds(kPopupBorderThickness, y, popup_width,
+                         help_label_->GetHeightForWidth(popup_width) +
+                         help_label_->GetInsets().height());
 }
 
 void PasswordGenerationPopupViewViews::OnPaint(gfx::Canvas* canvas) {
   if (!controller_)
     return;
 
   // Draw border and background.
   views::View::OnPaint(canvas);
 
   // Divider line needs to be drawn after OnPaint() otherwise the background
   // will overwrite the divider.
   if (password_view_)
-    canvas->FillRect(controller_->divider_bounds(), kDividerColor);
+    canvas->FillRect(divider_bounds_, kDividerColor);
 }
 
 void PasswordGenerationPopupViewViews::StyledLabelLinkClicked(
     const gfx::Range& range, int event_flags) {
   controller_->OnSavedPasswordsLinkClicked();
 }
 
+bool PasswordGenerationPopupViewViews::IsPointInPasswordBounds(
+    const gfx::Point& point) {
+  return password_view_->bounds().Contains(point);
+}
+
 PasswordGenerationPopupView* PasswordGenerationPopupView::Create(
     PasswordGenerationPopupController* controller) {
   views::Widget* observing_widget =
       views::Widget::GetTopLevelWidgetForNativeView(
           controller->container_view());
 
   // If the top level widget can't be found, cancel the popup since we can't
   // fully set it up.
   if (!observing_widget)
     return NULL;
 
   return new PasswordGenerationPopupViewViews(controller, observing_widget);
 }
 
 }  // namespace autofill