Put alert dialog text in a scroll view.

ui/views/controls/message_box_view.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 "ui/views/controls/message_box_view.h"
 
 #include "base/i18n/rtl.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/image_view.h"

[msw, 2014/07/09 15:56:59]

nit: remove this

[meacer, 2014/07/09 18:06:56]

Done.

 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
+#include "ui/views/controls/scroll_view.h"
+#include "ui/views/controls/scrollbar/overlay_scroll_bar.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/client_view.h"
 #include "ui/views/window/dialog_delegate.h"
 
 namespace {
 
 const int kDefaultMessageWidth = 320;
 
@@ skipping 40 matching lines @@
     : options(NO_OPTIONS),
       message(message),
       message_width(kDefaultMessageWidth),
       inter_row_vertical_spacing(kRelatedControlVerticalSpacing) {}
 
 MessageBoxView::InitParams::~InitParams() {
 }
 
 MessageBoxView::MessageBoxView(const InitParams& params)
     : prompt_field_(NULL),
-      icon_(NULL),
       checkbox_(NULL),
       link_(NULL),
       message_width_(params.message_width) {
   Init(params);
 }
 
 MessageBoxView::~MessageBoxView() {}
 
 base::string16 MessageBoxView::GetInputText() {
   return prompt_field_ ? prompt_field_->text() : base::string16();
 }
 
 bool MessageBoxView::IsCheckBoxSelected() {
   return checkbox_ ? checkbox_->checked() : false;
 }
 
-void MessageBoxView::SetIcon(const gfx::ImageSkia& icon) {
-  if (!icon_)
-    icon_ = new ImageView();
-  icon_->SetImage(icon);
-  icon_->SetBounds(0, 0, icon.width(), icon.height());
-  ResetLayoutManager();
-}
-
 void MessageBoxView::SetCheckBoxLabel(const base::string16& label) {
   if (!checkbox_)
     checkbox_ = new Checkbox(label);
   else
     checkbox_->SetText(label);
   ResetLayoutManager();
 }
 
 void MessageBoxView::SetCheckBoxSelected(bool selected) {
   if (!checkbox_)
@@ skipping 88 matching lines @@
   inter_row_vertical_spacing_ = params.inter_row_vertical_spacing;
 
   ResetLayoutManager();
 }
 
 void MessageBoxView::ResetLayoutManager() {
   // Initialize the Grid Layout Manager used for this dialog box.
   GridLayout* layout = GridLayout::CreatePanel(this);
   SetLayoutManager(layout);
 
-  gfx::Size icon_size;
-  if (icon_)
-    icon_size = icon_->GetPreferredSize();
-
   // Add the column set for the message displayed at the top of the dialog box.
   // And an icon, if one has been set.

[msw, 2014/07/09 15:56:59]

nit: remove this part of the comment

[meacer, 2014/07/09 18:06:56]

Done.

   const int message_column_view_set_id = 0;
   ColumnSet* column_set = layout->AddColumnSet(message_column_view_set_id);
-  if (icon_) {
-    column_set->AddColumn(GridLayout::LEADING, GridLayout::LEADING, 0,
-                          GridLayout::FIXED, icon_size.width(),
-                          icon_size.height());
-    column_set->AddPaddingColumn(0, kUnrelatedControlHorizontalSpacing);
-  }
   column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, 1,
                         GridLayout::FIXED, message_width_, 0);
 
   // Column set for extra elements, if any.
   const int extra_column_view_set_id = 1;
   if (prompt_field_ || checkbox_ || link_) {
     column_set = layout->AddColumnSet(extra_column_view_set_id);
-    if (icon_) {
-      column_set->AddPaddingColumn(
-          0, icon_size.width() + kUnrelatedControlHorizontalSpacing);
-    }
     column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, 1,
                           GridLayout::USE_PREF, 0, 0);
   }
 
-  for (size_t i = 0; i < message_labels_.size(); ++i) {
-    layout->StartRow(i, message_column_view_set_id);
-    if (icon_) {
-      if (i == 0)
-        layout->AddView(icon_);
-      else
-        layout->SkipColumns(1);
-    }
-    layout->AddView(message_labels_[i]);
-  }
+  const int kMaxScrollViewHeight = 600;

[sky, 2014/07/09 16:20:15]

Why do you need to clip this? Wouldn't it be better to use work area?

[meacer, 2014/07/09 18:06:56]

The clipping + overlay scroll bar gave a useful combination where both the
dialog was constrained to a maximum height and the scroll bar was visible when
needed.

It looks like the scroll contents aren't visible at all when the view isn't
clipped. I don't have much expertise with the views, so maybe I'm missing
something?

[sky, 2014/07/09 23:21:08]

'Clip' was a bad choice on my side. I'm suggesting you use the work area of the
monitor you're on and not an arbitrary value.
ScrollView is a bit odd, you may need to explicitly set the bounds of
message_contents.

[meacer, 2014/07/10 00:27:53]

The message view is constrained by the browser window, which means I'll need to
scale according to the browser window and not the whole screen, and will have to
pass the browser window to MessageView. Does that sound reasonable?

[msw, 2014/07/10 00:34:58]

You may be able to override MessageBoxView::GetMinimumSize (and ditto for the
relevant DialogDelegate?) to let the dialog be sized automatically to fit within
the browser window (kind of like CollectedCookiesViews, which albeit isn't
perfect...).

[meacer, 2014/07/10 19:36:10]

I've tried a couple of approaches, and none seem to work other than using
ClipHeightTo with a pixel value:

- Overriding GetMinimumSize doesn't clamp the height to the browser window.
- message_contents_->SetBoundsRect doesn't set a height for the scroll view.
- NativeScrollBar doesn't show even when I set message_contents_->SetBoundsRect.

+  views::View* message_contents = new views::View();
+  message_contents->SetLayoutManager(
+      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
+  for (size_t i = 0; i < message_labels_.size(); ++i)
+    message_contents->AddChildView(message_labels_[i]);
+  ScrollView* scroll_view = new views::ScrollView();
+  scroll_view->SetVerticalScrollBar(new views::OverlayScrollBar(false));

[sky, 2014/07/09 16:20:15]

Why are you going with overlay?

[meacer, 2014/07/09 18:06:56]

Oddly, NativeScrollBar doesn't show for me at all, and Kennedy looked a bit too
thick and I didn't see it used elsewhere so wanted to play safe. Happy to change
it if you feel otherwise though.

[sky, 2014/07/09 23:21:08]

You should go with the default. The NativeScrollBar should only show if needed,
although it may vary depending upon platform (NativeTheme).

+  scroll_view->ClipHeightTo(0, kMaxScrollViewHeight);
+  scroll_view->SetContents(message_contents);
+  layout->StartRow(0, message_column_view_set_id);
+  layout->AddView(scroll_view);
 
   if (prompt_field_) {
     layout->AddPaddingRow(0, inter_row_vertical_spacing_);
     layout->StartRow(0, extra_column_view_set_id);
     layout->AddView(prompt_field_);
   }
 
   if (checkbox_) {
     layout->AddPaddingRow(0, inter_row_vertical_spacing_);
     layout->StartRow(0, extra_column_view_set_id);
     layout->AddView(checkbox_);
   }
 
   if (link_) {
     layout->AddPaddingRow(0, inter_row_vertical_spacing_);
     layout->StartRow(0, extra_column_view_set_id);
     layout->AddView(link_);
   }
 }
 
 }  // namespace views