Add message and Help Center link to the chooser UI

chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc

Index: chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
diff --git a/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc b/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
index 4a412c6a53ccd7af07c70572614ddc340035e1bc..4da6a763421721b061e03f55ed9d791248be85dd 100644
--- a/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
+++ b/chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc
@@ -30,10 +30,14 @@
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/controls/separator.h"
+#include "ui/views/controls/styled_label.h"
+#include "ui/views/controls/styled_label_listener.h"
 #include "ui/views/controls/table/table_view.h"
 #include "ui/views/controls/table/table_view_observer.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
+#include "ui/views/layout/layout_constants.h"
 
 namespace {
 
@@ -43,16 +47,6 @@ const int kChooserPermissionBubbleWidth = 300;
 // Chooser permission bubble height
 const int kChooserPermissionBubbleHeight = 200;
 
-// Spacing constant for outer margin. This is added to the
-// bubble margin itself to equalize the margins at 13px.
-const int kBubbleOuterMargin = 5;
-
-// Spacing between major items should be 9px.
-const int kItemMajorSpacing = 9;
-
-// Button border size, draws inside the spacing distance.
-const int kButtonBorderSize = 2;
-
 }  // namespace
 
 scoped_ptr<BubbleUi> ChooserBubbleDelegate::BuildBubbleUi() {
@@ -65,6 +59,7 @@ class ChooserTableModel;
 // View implementation for the chooser bubble.
 class ChooserBubbleUiViewDelegate : public views::BubbleDelegateView,
                                     public views::ButtonListener,
+                                    public views::StyledLabelListener,
                                     public views::TableViewObserver {
  public:
   ChooserBubbleUiViewDelegate(views::View* anchor_view,
@@ -75,14 +70,19 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDelegateView,
 
   void Close();
 
-  // BubbleDelegateView:
+  // views::BubbleDelegateView:
   bool ShouldShowWindowTitle() const override;
   base::string16 GetWindowTitle() const override;
   void OnWidgetDestroying(views::Widget* widget) override;
 
-  // ButtonListener:
+  // views::ButtonListener:
   void ButtonPressed(views::Button* button, const ui::Event& event) override;
 
+  // views::StyledLabelListener:
+  void StyledLabelLinkClicked(views::StyledLabel* label,
+                              const gfx::Range& range,
+                              int event_flags) override;
+
   // views::TableViewObserver:
   void OnSelectionChanged() override;
 
@@ -99,6 +99,7 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDelegateView,
 
   views::LabelButton* connect_button_;
   views::LabelButton* cancel_button_;
+  views::StyledLabel* label_;
   views::TableView* table_view_;
   ChooserTableModel* chooser_table_model_;
   bool button_pressed_;
@@ -146,6 +147,24 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
       owner_(owner),
       chooser_bubble_delegate_(chooser_bubble_delegate),
       button_pressed_(false) {
+  // TODO(juncai): try using DialogClientView to build the chooser UI view since
+  // they look similar.
+  // https://crbug.com/587545
+  // ------------------------------------
+  // | Chooser bubble title             |
+  // | -------------------------------- |
+  // | | option 0                     | |
+  // | | option 1                     | |
+  // | | option 2                     | |
+  // | |                              | |
+  // | |                              | |
+  // | |                              | |
+  // | -------------------------------- |
+  // |           [ Connect ] [ Cancel ] |
+  // |----------------------------------|
+  // | Not seeing your device? Get help |
+  // ------------------------------------
+
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
 
@@ -153,7 +172,7 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
   column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
                         views::GridLayout::USE_PREF, 0, 0);

[Evan Stade, 2016/02/17 23:09:01]

so |layout| only has one column set with one column in it... this should be a
box layout

[juncai, 2016/02/19 01:51:00]

Tried using the following code to add a TableView to BoxLayout:
views::View* table_parent = table_view_->CreateParentIfNecessary();
AddChildView(table_parent);
layout->SetFlexForView(table_parent, 1);

But somehow the table view is not shown. Any suggestions? Thanks!

 
-  layout->StartRow(1, 0);
+  layout->StartRow(0, 0);

[msw, 2016/02/17 22:58:16]

Should the table's row be resizable? (if the bubble is shorter than expected?)

[Jeffrey Yasskin, 2016/02/17 23:54:25]

The UX folks are still figuring out how to size the bubble appropriately:
https://groups.google.com/a/google.com/d/msg/chrome-ui-review/AHU4T6B-8sU/kBU...
Bluetooth, especially, causes a problem because it discovers devices over time,
which could cause the buttons to jump around.

 
   // Create a table view
   std::vector<ui::TableColumn> table_columns;
@@ -169,25 +188,23 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
                   views::GridLayout::FILL, views::GridLayout::FILL,
                   kChooserPermissionBubbleWidth,
                   kChooserPermissionBubbleHeight);
-  if (chooser_bubble_delegate_->NumOptions() == 0) {
+  if (chooser_bubble_delegate_->NumOptions() == 0)
     table_view_->SetEnabled(false);
-  }
 
-  layout->AddPaddingRow(0, kItemMajorSpacing);
+  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   views::View* button_row = new views::View();
   views::GridLayout* button_layout = new views::GridLayout(button_row);
   views::ColumnSet* button_columns = button_layout->AddColumnSet(0);
   button_row->SetLayoutManager(button_layout);
-  layout->StartRow(1, 0);
+  layout->StartRow(0, 0);
   layout->AddView(button_row);
 
   // Lay out the Connect/Cancel buttons.
   button_columns->AddColumn(views::GridLayout::TRAILING,

[Evan Stade, 2016/02/17 23:09:01]

this also seems like it should be a (horizontal) box layout

[juncai, 2016/02/19 01:51:00]

Done.

                             views::GridLayout::FILL, 100,
                             views::GridLayout::USE_PREF, 0, 0);
-  button_columns->AddPaddingColumn(0,
-                                   kItemMajorSpacing - (2 * kButtonBorderSize));
+  button_columns->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
   button_columns->AddColumn(views::GridLayout::TRAILING,
                             views::GridLayout::FILL, 0,
                             views::GridLayout::USE_PREF, 0, 0);
@@ -208,7 +225,30 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
   cancel_button_->SetStyle(views::Button::STYLE_BUTTON);
   button_layout->AddView(cancel_button_);
 
-  button_layout->AddPaddingRow(0, kBubbleOuterMargin);
+  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+
+  // Lay out the separator.
+  layout->StartRow(0, 0);
+  views::Separator* separator =
+      new views::Separator(views::Separator::HORIZONTAL);

[msw, 2016/02/17 22:58:16]

nit: inline in AddView() call below; remove |separator|.

[juncai, 2016/02/19 01:51:00]

Done.

+  layout->AddView(separator);
+
+  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+
+  // Lay out the styled label.
+  layout->StartRow(0, 0);
+  base::string16 link =
+      l10n_util::GetStringUTF16(IDS_CHOOSER_BUBBLE_GET_HELP_LINK_TEXT);
+  size_t offset;
+  base::string16 text = l10n_util::GetStringFUTF16(
+      IDS_CHOOSER_BUBBLE_NO_DEVICE_TEXT, link, &offset);
+  label_ = new views::StyledLabel(text, this);
+  views::StyledLabel::RangeStyleInfo link_style =
+      views::StyledLabel::RangeStyleInfo::CreateForLink();
+  link_style.color =
+      GetNativeTheme()->GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);

[msw, 2016/02/17 22:58:16]

aside: odd that CreateForLink doesn't do this.

[Evan Stade, 2016/02/17 23:09:01]

I don't think you can call GetNativeTheme() yet (and get a good return value)
because you haven't been added to a parent view or widget hierarchy. This
probably breaks on Linux either in gtk theme mode or classic theme mode.

[Jeffrey Yasskin, 2016/02/17 23:54:25]

It does, by causing the created label at
https://code.google.com/p/chromium/codesearch/#chromium/src/ui/views/controls...
to be a Link, which has a default color at
https://code.google.com/p/chromium/codesearch/#chromium/src/ui/views/controls....

Jun, if you remove this line, does the link look like a link?

[juncai, 2016/02/19 01:51:00]

override
views::BubbleDelegateView::OnNativeThemeChanged to set the link text color.

Done.

[juncai, 2016/02/19 01:51:00]

If I remove this line, the link still looks like a link, but the color is red
instead of blue.

[juncai, 2016/02/20 02:25:05]

ok, this line of code is not needed, estade@ pointed out that I was testing the
bubble UI using the the gtk theme and the link text is red. In classic theme the
link text is blue. I changed it to classic theme and it works.

+  label_->AddStyleRange(gfx::Range(offset, offset + link.length()), link_style);
+  layout->AddView(label_);
 }
 
 ChooserBubbleUiViewDelegate::~ChooserBubbleUiViewDelegate() {
@@ -256,6 +296,13 @@ void ChooserBubbleUiViewDelegate::ButtonPressed(views::Button* button,
   }
 }
 
+void ChooserBubbleUiViewDelegate::StyledLabelLinkClicked(
+    views::StyledLabel* label,
+    const gfx::Range& range,
+    int event_flags) {
+  chooser_bubble_delegate_->OpenHelpCenterURL();
+}
+
 void ChooserBubbleUiViewDelegate::OnSelectionChanged() {
   connect_button_->SetEnabled(!table_view_->selection_model().empty());
 }