Harmonize EchoDialogView

As per crbug.com/788015 Comments 1,5:
- Fix padding sizes
- Change button text "Dismiss" -> "Got it"
- Make a button default (blue)
- Replace "Learn more" link with (?) button
  - Button has a11y text and is Tab stop
- Remove X (Close) button from upper right
- Clean up code, use standard LayoutProvider

Bug: 788015
Test: Set up a new Chromebook with Goodies offer available, witness a
dialog living in Harmony

Change-Id: I95058420c54a1b2b48881b95b458971ec163a8ae
Reviewed-on: https://chromium-review.googlesource.com/1040868
Commit-Queue: Greg Levin <glevin@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561384}

chrome/app/chromeos_strings.grdp

@@ -1431,16 +1431,13 @@
 
   <!-- Chrome OS Strings -->
   <message name="IDS_ECHO_CONSENT_DIALOG_TEXT" desc="Dialog text shown when user is asked to give consent to proceed with redeeming an ECHO offer.">
-    <ph name="SERVICE_NAME">$1<ex>Google Drive</ex></ph> wants to check if you are using an eligible Chrome OS device. <ph name="MORE_INFO_LINK">$2<ex>More info</ex></ph>
+    <ph name="SERVICE_NAME">$1<ex>Google Drive</ex></ph> wants to check if you are using an eligible Chrome OS device.
   </message>
   <message name="IDS_ECHO_DISABLED_CONSENT_DIALOG_TEXT" desc="Dialog text shown when user is informed that redeeming offers is disabled for the device.">
-    Your IT administrator has disabled Chrome Goodies for your device. <ph name="MORE_INFO_LINK">$1<ex>More info</ex></ph>
+    Your IT administrator has disabled Chrome Goodies for your device.
   </message>
   <message name="IDS_ECHO_CONSENT_DISMISS_BUTTON" desc="Dismiss dialog button label for disabled echo dialog.">
-    Dismiss
-  </message>
-  <message name="IDS_OFFERS_CONSENT_INFOBAR_LABEL_LEARN_MORE" desc="Text of the Learn More link in the echo dialog.">
-    Learn More
+    Got it
   </message>
   <message name="IDS_OFFERS_CONSENT_INFOBAR_ENABLE_BUTTON" desc="Enable button label.">
     Allow
@@ -2908,6 +2905,9 @@
   <message name="IDS_CHROMEOS_ACC_LOGIN_SIGNIN_PUBLIC_ACCOUNT" desc="Status when signing in into a public account.">
     Entering public session.
   </message>
+  <message name="IDS_CHROMEOS_ACC_LEARN_MORE" desc="Dialog button [?] linking to more information.">
+    Learn more.
+  </message>
 
   <!-- Removable device notifications -->
   <message name="IDS_REMOVABLE_DEVICE_DETECTION_TITLE" desc="Text of notification message which is shown when user inserts removable device (SD card, USB key...)">

chrome/browser/chromeos/ui/echo_dialog_view.cc

@@ -8,88 +8,64 @@
 
 #include "chrome/browser/chromeos/ui/echo_dialog_listener.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/vector_icons/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/font.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/button/image_button_factory.h"
+#include "ui/views/controls/label.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
 
-namespace {
-
-const int kDialogLabelTopInset = 20;
-const int kDialogLabelLeftInset = 20;
-const int kDialogLabelBottomInset = 20;
-const int kDialogLabelRightInset = 100;
-
-const int kDialogLabelPreferredWidth =
-    350 + kDialogLabelLeftInset + kDialogLabelRightInset;
-
-}  // namespace
-
 namespace chromeos {
 
 EchoDialogView::EchoDialogView(EchoDialogListener* listener)
-    : label_(NULL),
-      listener_(listener),
+    : listener_(listener),
+      learn_more_button_(nullptr),
       ok_button_label_id_(0),
       cancel_button_label_id_(0) {
   chrome::RecordDialogCreation(chrome::DialogIdentifier::ECHO);
 }
 
-EchoDialogView::~EchoDialogView() {}
+EchoDialogView::~EchoDialogView() = default;
 
 void EchoDialogView::InitForEnabledEcho(const base::string16& service_name,
                                         const base::string16& origin) {
   ok_button_label_id_ = IDS_OFFERS_CONSENT_INFOBAR_ENABLE_BUTTON;
   cancel_button_label_id_ = IDS_OFFERS_CONSENT_INFOBAR_DISABLE_BUTTON;
 
-  base::string16 link =
-      l10n_util::GetStringUTF16(IDS_OFFERS_CONSENT_INFOBAR_LABEL_LEARN_MORE);
-
-  std::vector<size_t> offsets;
+  size_t offset;
   base::string16 text = l10n_util::GetStringFUTF16(IDS_ECHO_CONSENT_DIALOG_TEXT,
-                                             service_name,
-                                             link,
-                                             &offsets);
+                                                   service_name, &offset);
 
-  label_ = new views::StyledLabel(text, this);
+  views::StyledLabel* label = new views::StyledLabel(text, nullptr);
 
   views::StyledLabel::RangeStyleInfo service_name_style;
+  gfx::FontList font_list = label->GetDefaultFontList();
   service_name_style.custom_font =
-      label_->GetDefaultFontList().DeriveWithStyle(gfx::Font::UNDERLINE);
+      font_list.DeriveWithStyle(gfx::Font::UNDERLINE);
   service_name_style.tooltip = origin;
-  label_->AddStyleRange(
-      gfx::Range(offsets[0], offsets[0] + service_name.length()),
-      service_name_style);
-
-  label_->AddStyleRange(gfx::Range(offsets[1], offsets[1] + link.length()),
-                        views::StyledLabel::RangeStyleInfo::CreateForLink());
+  label->AddStyleRange(gfx::Range(offset, offset + service_name.length()),
+                       service_name_style);
 
-  SetLabelBorderAndBounds();
-
-  AddChildView(label_);
+  SetBorderAndLabel(label, font_list);
 }
 
 void EchoDialogView::InitForDisabledEcho() {
   ok_button_label_id_ = 0;
   cancel_button_label_id_ = IDS_ECHO_CONSENT_DISMISS_BUTTON;
 
-  base::string16 link =
-      l10n_util::GetStringUTF16(IDS_OFFERS_CONSENT_INFOBAR_LABEL_LEARN_MORE);
+  views::Label* label = new views::Label(
+      l10n_util::GetStringUTF16(IDS_ECHO_DISABLED_CONSENT_DIALOG_TEXT));
+  label->SetMultiLine(true);
+  label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
-  size_t offset;
-  base::string16 text = l10n_util::GetStringFUTF16(
-      IDS_ECHO_DISABLED_CONSENT_DIALOG_TEXT, link, &offset);
-
-  label_ = new views::StyledLabel(text, this);
-  label_->AddStyleRange(gfx::Range(offset, offset + link.length()),
-                        views::StyledLabel::RangeStyleInfo::CreateForLink());
-
-  SetLabelBorderAndBounds();
-
-  AddChildView(label_);
+  SetBorderAndLabel(label, label->font_list());
 }
 
 void EchoDialogView::Show(gfx::NativeWindow parent) {
@@ -100,8 +76,14 @@ void EchoDialogView::Show(gfx::NativeWindow parent) {
   GetWidget()->Show();
 }
 
-int EchoDialogView::GetDefaultDialogButton() const {
-  return ui::DIALOG_BUTTON_NONE;
+views::View* EchoDialogView::CreateExtraView() {
+  learn_more_button_ = views::CreateVectorImageButton(this);
+  views::SetImageFromVectorIcon(learn_more_button_,
+                                vector_icons::kHelpOutlineIcon);
+  learn_more_button_->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_CHROMEOS_ACC_LEARN_MORE));
+  learn_more_button_->SetFocusForPlatform();
+  return learn_more_button_;
 }
 
 int EchoDialogView::GetDialogButtons() const {
@@ -113,29 +95,29 @@ int EchoDialogView::GetDialogButtons() const {
   return buttons;
 }
 
-bool EchoDialogView::Accept() {
-  if (listener_) {
-    listener_->OnAccept();
-    listener_ = NULL;
-  }
-  return true;
+base::string16 EchoDialogView::GetDialogButtonLabel(
+    ui::DialogButton button) const {
+  if (button == ui::DIALOG_BUTTON_OK)
+    return l10n_util::GetStringUTF16(ok_button_label_id_);
+  if (button == ui::DIALOG_BUTTON_CANCEL)
+    return l10n_util::GetStringUTF16(cancel_button_label_id_);
+  return base::string16();
 }
 
 bool EchoDialogView::Cancel() {
   if (listener_) {
     listener_->OnCancel();
-    listener_ = NULL;
+    listener_ = nullptr;
   }
   return true;
 }
 
-base::string16 EchoDialogView::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DIALOG_BUTTON_OK && ok_button_label_id_)
-    return l10n_util::GetStringUTF16(ok_button_label_id_);
-  if (button == ui::DIALOG_BUTTON_CANCEL && cancel_button_label_id_)
-    return l10n_util::GetStringUTF16(cancel_button_label_id_);
-  return base::string16();
+bool EchoDialogView::Accept() {
+  if (listener_) {
+    listener_->OnAccept();
+    listener_ = nullptr;
+  }
+  return true;
 }
 
 ui::ModalType EchoDialogView::GetModalType() const {
@@ -146,36 +128,42 @@ bool EchoDialogView::ShouldShowWindowTitle() const {
   return false;
 }
 
-bool EchoDialogView::ShouldShowWindowIcon() const {
+bool EchoDialogView::ShouldShowCloseButton() const {
   return false;
 }
 
-void EchoDialogView::StyledLabelLinkClicked(views::StyledLabel* label,
-                                            const gfx::Range& range,
-                                            int event_flags) {
-  if (!listener_)
+void EchoDialogView::ButtonPressed(views::Button* sender,
+                                   const ui::Event& event) {
+  if (!listener_ || sender != learn_more_button_)
     return;
   listener_->OnMoreInfoLinkClicked();
 }
 
 gfx::Size EchoDialogView::CalculatePreferredSize() const {
-  gfx::Size size =
-      gfx::Size(kDialogLabelPreferredWidth,
-                label_->GetHeightForWidth(kDialogLabelPreferredWidth));
-  gfx::Insets insets = GetInsets();
-  size.Enlarge(insets.width(), insets.height());
-  return size;
+  const int default_width = views::LayoutProvider::Get()->GetDistanceMetric(
+      DISTANCE_MODAL_DIALOG_PREFERRED_WIDTH);
+  return gfx::Size(
+      default_width,
+      GetLayoutManager()->GetPreferredHeightForWidth(this, default_width));
 }
 
-void EchoDialogView::SetLabelBorderAndBounds() {
-  label_->SetBorder(views::CreateEmptyBorder(
-      kDialogLabelTopInset, kDialogLabelLeftInset, kDialogLabelBottomInset,
-      kDialogLabelRightInset));
+void EchoDialogView::SetBorderAndLabel(views::View* label,
+                                       const gfx::FontList& label_font_list) {
+  SetLayoutManager(std::make_unique<views::FillLayout>());
+
+  // Without a title, top padding isn't correctly calculated.  This adds the
+  // text's internal leading to the top padding.  See
+  // FontList::DeriveWithHeightUpperBound() for font padding details.
+  int top_inset_padding =
+      label_font_list.GetBaseline() - label_font_list.GetCapHeight();
+
+  gfx::Insets insets =
+      views::LayoutProvider::Get()->GetDialogInsetsForContentType(views::TEXT,
+                                                                  views::TEXT);
+  insets += gfx::Insets(top_inset_padding, 0, 0, 0);
+  SetBorder(views::CreateEmptyBorder(insets));
 
-  label_->SetBounds(label_->x(),
-                    label_->y(),
-                    kDialogLabelPreferredWidth,
-                    label_->GetHeightForWidth(kDialogLabelPreferredWidth));
+  AddChildView(label);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/ui/echo_dialog_view.h

@@ -7,12 +7,17 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "ui/views/controls/styled_label_listener.h"
+#include "ui/views/controls/button/image_button.h"
 #include "ui/views/window/dialog_delegate.h"
 
 namespace views {
-class StyledLabel;
-}
+class ImageButton;
+class View;
+}  // namespace views
+
+namespace gfx {
+class FontList;
+}  // namespace gfx
 
 namespace chromeos {
 
@@ -24,7 +29,7 @@ class EchoDialogListener;
 // extension is not allowed by policy to redeem offers, the dialog informs user
 // about this.
 class EchoDialogView : public views::DialogDelegateView,
-                       public views::StyledLabelListener {
+                       public views::ButtonListener {
  public:
   explicit EchoDialogView(EchoDialogListener* listener);
   ~EchoDialogView() override;
@@ -50,8 +55,8 @@ class EchoDialogView : public views::DialogDelegateView,
   friend class ExtensionEchoPrivateApiTest;
 
   // views::DialogDelegate overrides.
+  View* CreateExtraView() override;
   int GetDialogButtons() const override;
-  int GetDefaultDialogButton() const override;
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Cancel() override;
   bool Accept() override;
@@ -59,22 +64,20 @@ class EchoDialogView : public views::DialogDelegateView,
   // views::WidgetDelegate overrides.
   ui::ModalType GetModalType() const override;
   bool ShouldShowWindowTitle() const override;
-  bool ShouldShowWindowIcon() const override;
+  bool ShouldShowCloseButton() const override;
 
-  // views::LinkListener override.
-  void StyledLabelLinkClicked(views::StyledLabel* label,
-                              const gfx::Range& range,
-                              int event_flags) override;
+  // views::ButtonListener overrides.
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // views::View override.
   gfx::Size CalculatePreferredSize() const override;
 
-  // Sets the border and bounds for the styled label containing the dialog
-  // text.
-  void SetLabelBorderAndBounds();
+  // Sets the border and label view.
+  void SetBorderAndLabel(views::View* label,
+                         const gfx::FontList& label_font_list);
 
-  views::StyledLabel* label_;
   EchoDialogListener* listener_;
+  views::ImageButton* learn_more_button_;
   int ok_button_label_id_;
   int cancel_button_label_id_;