Polish for Extension Install Dialog.

chrome/browser/ui/views/extensions/extension_install_dialog_view.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.

[sky, 2013/09/09 15:00:30]

nit: we don't need to update copyrights anymore.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/i18n/rtl.h"
 #include "base/strings/string_util.h"
@@ skipping 144 matching lines @@
 
   // The container view for the scroll view.
   CustomScrollableView* scrollable_;
 
   // The preferred size of the dialog.
   gfx::Size dialog_size_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogView);
 };
 
+// A simple view that pre-pends a view with a bullet with the help of a grid

[sky, 2013/09/09 15:00:30]

prepends

+// layout.
+class BulletedView : public views::View {
+ public:
+  explicit BulletedView(views::View* view);
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BulletedView);
+};
+
+BulletedView::BulletedView(views::View* view) {
+  views::GridLayout* layout = new views::GridLayout(this);
+  SetLayoutManager(layout);
+  views::ColumnSet* column_set = layout->AddColumnSet(0);
+  column_set->AddColumn(views::GridLayout::LEADING,
+                        views::GridLayout::LEADING,
+                        0,
+                        views::GridLayout::USE_PREF,
+                        0, // no fixed width
+                        0);
+   column_set->AddColumn(views::GridLayout::LEADING,
+                         views::GridLayout::LEADING,
+                         0,
+                         views::GridLayout::USE_PREF,
+                         0,  // no fixed width
+                         0);
+  layout->StartRow(0, 0);
+  layout->AddView(new views::Label(PrepareForDisplay(string16(), true)));
+  layout->AddView(view);
+}

[Finnur, 2013/09/09 14:34:03]

So, to answer your question: No, I don't need to get all complicated with the
grid layout. This serves my purpose nicely (having a sub-layout).

+
 // A view to display text with an expandable details section.
 class ExpandableContainerView : public views::View,
                                 public views::ButtonListener,
                                 public views::LinkListener,
                                 public ui::AnimationDelegate {
  public:
   ExpandableContainerView(ExtensionInstallDialogView* owner,
                           const string16& description,
                           const PermissionDetails& details,
                           int horizontal_space,
-                          bool show_bullets);

[Finnur, 2013/09/09 14:34:03]

I discovered that this param was always set to false, so I removed it.

+                          bool parent_bulleted);

[Finnur, 2013/09/09 14:34:03]

And then I needed a new param that unfortunately looks related to the one I
removed, but isn't really.

   virtual ~ExpandableContainerView();
 
   // views::View:
   virtual void ChildPreferredSizeChanged(views::View* child) OVERRIDE;
 
   // views::ButtonListener:
   virtual void ButtonPressed(views::Button* sender,
                              const ui::Event& event) OVERRIDE;
 
   // views::LinkListener:
   virtual void LinkClicked(views::Link* source, int event_flags) OVERRIDE;
 
   // ui::AnimationDelegate:
   virtual void AnimationProgressed(const ui::Animation* animation) OVERRIDE;
   virtual void AnimationEnded(const ui::Animation* animation) OVERRIDE;
 
  private:
   // A view which displays all the details of an IssueAdviceInfoEntry.
   class DetailsView : public views::View {
    public:
-    explicit DetailsView(int horizontal_space, bool show_bullets);
+    explicit DetailsView(int horizontal_space, bool parent_bulleted);
     virtual ~DetailsView() {}
 
     // views::View:
     virtual gfx::Size GetPreferredSize() OVERRIDE;
 
     void AddDetail(const string16& detail);
 
     // Animates this to be a height proportional to |state|.
     void AnimateToState(double state);
 
    private:
     views::GridLayout* layout_;
     double state_;
 
-    // Whether to show bullets in front of each item in the details.
-    bool show_bullets_;
+    // Whether the parent item is showing bullets. This will determine how much
+    // extra indentation is needed.
+    bool parent_bulleted_;
 
     DISALLOW_COPY_AND_ASSIGN(DetailsView);
   };
 
   // Expand/Collapse the detail section for this ExpandableContainerView.
   void ToggleDetailLevel();
 
   // The dialog that owns |this|. It's also an ancestor in the View hierarchy.
   ExtensionInstallDialogView* owner_;
 
@@ skipping 250 matching lines @@
         permissions_header = new views::Label(prompt.GetPermissionsHeading());
       }
       permissions_header->SetMultiLine(true);
       permissions_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
       permissions_header->SizeToFit(left_column_width);
       layout->AddView(permissions_header);
 
       for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) {
         layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
         layout->StartRow(0, column_set_id);
-        views::Label* permission_label = new views::Label(PrepareForDisplay(
-            prompt.GetPermission(i), true));
+        views::Label* permission_label =
+            new views::Label(prompt.GetPermission(i));
         permission_label->SetMultiLine(true);
         permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
         permission_label->SizeToFit(left_column_width);
-        layout->AddView(permission_label);
+        layout->AddView(new BulletedView(permission_label));
 
         // If we have more details to provide, show them in collapsed form.
         if (!prompt.GetPermissionsDetails(i).empty()) {
           layout->StartRow(0, column_set_id);
           PermissionDetails details;
           details.push_back(
               PrepareForDisplay(prompt.GetPermissionsDetails(i), false));
           ExpandableContainerView* details_container =
               new ExpandableContainerView(
-                  this, string16(), details, left_column_width, false);
+                  this, string16(), details, left_column_width, true);
           layout->AddView(details_container);
         }
       }
     } else {
       layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
       layout->StartRow(0, column_set_id);
       views::Label* permission_label = new views::Label(
           l10n_util::GetStringUTF16(IDS_EXTENSION_NO_SPECIAL_PERMISSIONS));
       permission_label->SetMultiLine(true);
       permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
@@ skipping 31 matching lines @@
       layout->StartRowWithPadding(
           0, column_set_id,
           0, views::kRelatedControlVerticalSpacing);
 
       PermissionDetails details;
       const IssueAdviceInfoEntry& entry = prompt.GetOAuthIssue(i);
       for (size_t x = 0; x < entry.details.size(); ++x)
         details.push_back(entry.details[x]);
       ExpandableContainerView* issue_advice_view =
           new ExpandableContainerView(
-              this, entry.description, details, space_for_oauth, false);
+              this, entry.description, details, space_for_oauth, true);
       layout->AddView(issue_advice_view);
     }
   }
   if (prompt.GetRetainedFileCount()) {
     // Slide in under the permissions or OAuth, if there are any. If there are
     // either, the retained files prompt stretches all the way to the right of
     // the dialog. If there are no permissions or OAuth, the retained files
     // prompt just takes up the left column.
     int space_for_files = left_column_width;
     if (prompt.GetPermissionCount() || prompt.GetOAuthIssueCount()) {
@@ skipping 108 matching lines @@
 
 // static
 ExtensionInstallPrompt::ShowDialogCallback
 ExtensionInstallPrompt::GetDefaultShowDialogCallback() {
   return base::Bind(&ShowExtensionInstallDialogImpl);
 }
 
 // ExpandableContainerView::DetailsView ----------------------------------------
 
 ExpandableContainerView::DetailsView::DetailsView(int horizontal_space,
-                                                  bool show_bullets)
+                                                  bool parent_bulleted)
     : layout_(new views::GridLayout(this)),
       state_(0),
-      show_bullets_(show_bullets) {
+      parent_bulleted_(parent_bulleted) {
   SetLayoutManager(layout_);
   views::ColumnSet* column_set = layout_->AddColumnSet(0);
-  column_set->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  // If the parent is using bullets for its items, then a padding of one unit
+  // will make the child item (which has no bullet) look like a sibling of its
+  // parent. Therefore increase the indentation by one more unit to show that it
+  // is in fact a child item (with no missing bullet) and not a sibling.
+  column_set->AddPaddingColumn(
+      0, views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1));
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::LEADING,
                         0,
                         views::GridLayout::FIXED,
                         horizontal_space,
                         0);
 }
 
 void ExpandableContainerView::DetailsView::AddDetail(const string16& detail) {
   layout_->StartRowWithPadding(0, 0,
                                0, views::kRelatedControlSmallVerticalSpacing);
   views::Label* detail_label =
-      new views::Label(PrepareForDisplay(detail, show_bullets_));
+      new views::Label(PrepareForDisplay(detail, false));
   detail_label->SetMultiLine(true);
   detail_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   layout_->AddView(detail_label);
 }
 
 gfx::Size ExpandableContainerView::DetailsView::GetPreferredSize() {
   gfx::Size size = views::View::GetPreferredSize();
   return gfx::Size(size.width(), size.height() * state_);
 }
 
 void ExpandableContainerView::DetailsView::AnimateToState(double state) {
   state_ = state;
   PreferredSizeChanged();
   SchedulePaint();
 }
 
 // ExpandableContainerView -----------------------------------------------------
 
 ExpandableContainerView::ExpandableContainerView(
     ExtensionInstallDialogView* owner,
     const string16& description,
     const PermissionDetails& details,
     int horizontal_space,
-    bool show_bullets)
+    bool parent_bulleted)
     : owner_(owner),
       details_view_(NULL),
       arrow_view_(NULL),
       slide_animation_(this),
       expanded_(false) {
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
   int column_set_id = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(column_set_id);
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::LEADING,
                         0,
                         views::GridLayout::USE_PREF,
                         0,
                         0);
   if (!description.empty()) {
     layout->StartRow(0, column_set_id);
 
-    views::Label* description_label =
-        new views::Label(PrepareForDisplay(description, true));
+    views::Label* description_label = new views::Label(description);
     description_label->SetMultiLine(true);
     description_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     description_label->SizeToFit(horizontal_space);
-    layout->AddView(description_label);
+    layout->AddView(new BulletedView(description_label));
   }
 
   if (details.empty())
     return;
 
-  details_view_ = new DetailsView(horizontal_space, show_bullets);
+  details_view_ = new DetailsView(horizontal_space, parent_bulleted);
 
   layout->StartRow(0, column_set_id);
   layout->AddView(details_view_);
 
   for (size_t i = 0; i < details.size(); ++i)
     details_view_->AddDetail(details[i]);
 
-  // Prepare the columns for the More Details row.
+  views::Link* link = new views::Link(
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS));
+
+  // Make sure the link width column is as wide as needed for both Show and
+  // Hide details, so that the arrow doesn't shift horizontally when we toggle.
+  int link_col_width =
+      views::kRelatedControlHorizontalSpacing +
+      std::max(link->font_list().GetStringWidth(
+                   l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_DETAILS)),
+               link->font_list().GetStringWidth(
+                   l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS)));
+
   column_set = layout->AddColumnSet(++column_set_id);
-  column_set->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  // Padding to the left of the More Details column. If the parent is using
+  // bullets for its items, then a padding of one unit will make the child item
+  // (which has no bullet) look like a sibling of its parent. Therefore increase
+  // the indentation by one more unit to show that it is in fact a child item
+  // (with no missing bullet) and not a sibling.
+  column_set->AddPaddingColumn(
+      0, views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1));
+  // The More Details column.
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::LEADING,
                         0,
-                        views::GridLayout::USE_PREF,
-                        0,
-                        0);
-  column_set->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+                        views::GridLayout::FIXED,
+                        link_col_width,
+                        link_col_width);
+  // The Up/Down arrow column.
   column_set->AddColumn(views::GridLayout::LEADING,
                        views::GridLayout::LEADING,
                        0,
                        views::GridLayout::USE_PREF,
                        0,
                        0);
-  column_set->AddColumn(views::GridLayout::LEADING,
-                        views::GridLayout::LEADING,
-                        0,
-                        views::GridLayout::USE_PREF,
-                        0,
-                        0);

[Finnur, 2013/09/09 14:34:03]

This column wasn't used, so I eated it.

 
   // Add the More Details link.
   layout->StartRow(0, column_set_id);
-  more_details_ = new views::Link(
-      l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS));
+  more_details_ = link;
   more_details_->set_listener(this);
   more_details_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   layout->AddView(more_details_);
 
   // Add the arrow after the More Details link.
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   arrow_toggle_ = new views::ImageButton(this);
   arrow_toggle_->SetImage(views::Button::STATE_NORMAL,
                           rb.GetImageSkiaNamed(IDR_DOWN_ARROW));
   layout->AddView(arrow_toggle_);
@@ skipping 42 matching lines @@
 }
 
 void ExpandableContainerView::ToggleDetailLevel() {
   expanded_ = !expanded_;
 
   if (slide_animation_.IsShowing())
     slide_animation_.Hide();
   else
     slide_animation_.Show();
 }