[ash-md] Add on-screen keyboard toggle row in IME menu view.

ash/common/system/chromeos/ime_menu/ime_list_view.cc

Index: ash/common/system/chromeos/ime_menu/ime_list_view.cc
diff --git a/ash/common/system/chromeos/ime_menu/ime_list_view.cc b/ash/common/system/chromeos/ime_menu/ime_list_view.cc
index d3f13487349fddf83ca07b882259621ecbb82116..d6178f80203d0d93fe55f75375175bf419fd51ba 100644
--- a/ash/common/system/chromeos/ime_menu/ime_list_view.cc
+++ b/ash/common/system/chromeos/ime_menu/ime_list_view.cc
@@ -7,11 +7,14 @@
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/ime_info.h"
+#include "ash/common/system/tray/system_menu_button.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_details_view.h"
 #include "ash/common/system/tray/tray_popup_header_button.h"
 #include "ash/common/system/tray/tray_popup_item_style.h"
+#include "ash/common/system/tray/tray_popup_utils.h"
+#include "ash/common/system/tray/tri_view.h"
 #include "ash/common/wm_shell.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
@@ -23,15 +26,24 @@
 #include "ui/gfx/vector_icons_public.h"
 #include "ui/keyboard/keyboard_util.h"
 #include "ui/views/border.h"
-#include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/button/toggle_button.h"
+#include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/separator.h"
-#include "ui/views/layout/box_layout.h"
+#include "ui/views/layout/fill_layout.h"
+#include "ui/views/painter.h"
+#include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace ash {
 namespace {
 
+const int kKeyboardRowVerticalInset = 4;
+const int kKeyboardRowSeparatorThickness = 1;
+const int kMinFontSizeDelta = -10;
+
+const SkColor kKeyboardRowSeparatorColor = SkColorSetA(SK_ColorBLACK, 0x1F);
+
 // Creates a separator that will be used between the IME list items.
 views::Separator* CreateListItemSeparator() {
   views::Separator* separator =
@@ -72,41 +84,6 @@ class SelectableHoverHighlightView : public HoverHighlightView {
   DISALLOW_COPY_AND_ASSIGN(SelectableHoverHighlightView);
 };
 
-// The view that contains IME short name and the IME label.
-class ImeInfoView : public views::View {
- public:
-  ImeInfoView(const base::string16& id, const base::string16& text) {
-    views::BoxLayout* box_layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
-    SetLayoutManager(box_layout);
-
-    // TODO(azurewei): Use TrayPopupItemStyle for |id_button|.
-    views::LabelButton* id_button = new views::LabelButton(nullptr, id);
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    id_button->SetFontList(rb.GetFontList(ui::ResourceBundle::MediumBoldFont));
-    id_button->SetTextColor(views::Button::STATE_NORMAL, kMenuIconColor);
-    id_button->SetMaxSize(gfx::Size(kMenuButtonSize, kMenuButtonSize));
-    id_button->SetMinSize(gfx::Size(kMenuButtonSize, kMenuButtonSize));
-    const int button_padding = (kMenuButtonSize - kMenuIconSize) / 2;
-    id_button->SetBorder(views::Border::CreateEmptyBorder(
-        button_padding, button_padding, button_padding, button_padding));
-    AddChildView(id_button);
-
-    views::Label* text_label = new views::Label(text);
-    TrayPopupItemStyle style(
-        GetNativeTheme(), TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
-    style.SetupLabel(text_label);
-    text_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    box_layout->set_cross_axis_alignment(
-        views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
-    AddChildView(text_label);
-    box_layout->SetFlexForView(text_label, 1);
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ImeInfoView);
-};
-
 // The IME list item view used in the material design. It contains IME info
 // (name and label) and a check button if the item is selected. It's also used
 // for IME property item, which has no name but label and a gray checked icon.
@@ -119,23 +96,42 @@ class ImeListItemView : public ActionableView {
                   bool selected,
                   const SkColor button_color)
       : ActionableView(owner), ime_list_view_(list_view) {
-    views::BoxLayout* box_layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
-    SetLayoutManager(box_layout);
+    TriView* tri_view = TrayPopupUtils::CreateDefaultRowView();
+    AddChildView(tri_view);
+    SetLayoutManager(new views::FillLayout);
+
+    // The id button shows the IME short name.
+    views::Label* id_label = new views::Label(id);
+    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+    const gfx::FontList& base_font_list =
+        rb.GetFontList(ui::ResourceBundle::MediumBoldFont);
+    id_label->SetFontList(base_font_list);
+    id_label->SizeToFit(kMenuIconSize);
+    id_label->SetMaximumWidth(kMenuIconSize);
+
+    // For IMEs whose short name are more than 2 characters (INTL, EXTD, etc.),
+    // |kMenuIconSize| is not enough. The label will trigger eliding as "I..."
+    // or "...". So we shrink the font size until it fits within the bounds.

[tdanderson, 2016/11/04 19:01:04]

Nice comment.

+    int size_delta = -1;
+    while (id_label->GetPreferredSize().width() > kMenuIconSize &&
+           size_delta >= kMinFontSizeDelta) {
+      id_label->SetFontList(base_font_list.DeriveWithSizeDelta(size_delta));
+      // In case we don't run into endless loop

[tdanderson, 2016/11/04 19:01:04]

Since you changed the loop, I think you can remove this comment now.

[Azure Wei, 2016/11/05 02:26:19]

Done.

+      --size_delta;
+    }
+    tri_view->AddView(TriView::Container::START, id_label);
 
-    ImeInfoView* info_view = new ImeInfoView(id, label);
-    AddChildView(info_view);
-    box_layout->SetFlexForView(info_view, 1);
+    // The label shows the IME name.
+    label_ = new views::Label(label);
+    label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    tri_view->AddView(TriView::Container::CENTER, label_);
 
     if (selected) {
-      views::ImageButton* check_button = new views::ImageButton(nullptr);
-      gfx::ImageSkia icon_image = gfx::CreateVectorIcon(
-          gfx::VectorIconId::CHECK_CIRCLE, kMenuIconSize, button_color);
-      check_button->SetImage(views::CustomButton::STATE_NORMAL, &icon_image);
-      const int button_padding = (kMenuButtonSize - icon_image.width()) / 2;
-      check_button->SetBorder(views::Border::CreateEmptyBorder(
-          button_padding, button_padding, button_padding, button_padding));
-      AddChildView(check_button);
+      // The checked button indicates the IME is selected.
+      views::ImageView* checked_image = TrayPopupUtils::CreateMainImageView();
+      checked_image->SetImage(gfx::CreateVectorIcon(
+          gfx::VectorIconId::CHECK_CIRCLE, kMenuIconSize, button_color));
+      tri_view->AddView(TriView::Container::END, checked_image);
     }
   }
 
@@ -147,13 +143,119 @@ class ImeListItemView : public ActionableView {
     return true;
   }
 
+  // views::View:
+  void OnNativeThemeChanged(const ui::NativeTheme* theme) override {
+    UpdateStyle();
+  }
+
  private:
+  // Updates the style of |label_| based on the current native theme.
+  void UpdateStyle() {
+    TrayPopupItemStyle style(
+        GetNativeTheme(), TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
+    style.SetupLabel(label_);
+  }
+
+  // The label shows the IME name.
+  views::Label* label_;
+
   ImeListView* ime_list_view_;
   DISALLOW_COPY_AND_ASSIGN(ImeListItemView);
 };
 
 }  // namespace
 
+// The view that contains a |KeyboardButtonView| and a toggle button.
+class MaterialKeyboardStatusRowView : public views::View {
+ public:
+  MaterialKeyboardStatusRowView(views::ButtonListener* listener, bool enabled)
+      : listener_(listener), label_(nullptr), toggle_(nullptr) {
+    Init();
+    SetKeyboardStatusEnabled(enabled);
+  }
+
+  ~MaterialKeyboardStatusRowView() override {}
+
+  void SetKeyboardStatusEnabled(bool enabled) {
+    toggle_->SetIsOn(enabled, true);
+    SetKeyboardToggleText();
+  }
+
+  void SetKeyboardToggleText() {
+    // TODO (azurewei): Update this as 'On-screen keyboard', as the toggle will
+    // be updated as 'on/off switch'. (crbug.com/652677).

[tdanderson, 2016/11/04 19:01:04]

nit: When you make this change, you should be able to remove the helper function
SetKeyboardToggleText() and just set the text once during Init(). It won't need
to change when the toggle changes. (you can also remove line 423)

+    toggle_->SetTooltipText(

[tdanderson, 2016/11/04 19:01:04]

We've decided that we're not going to have tooltip text on toggle buttons, but
of course we still need to have accessible text. So here instead of calling
SetTooltipText(), please call SetAccessibleName(). This sets the spoken feedback
but no tooltip.

[Azure Wei, 2016/11/05 02:26:19]

Done.

+        toggle_->is_on()
+            ? l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_DISABLE_KEYBOARD)
+            : l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_ENABLE_KEYBOARD));
+  }
+
+  const views::Button* toggle() const { return toggle_; }
+  bool is_toggled() const { return toggle_->is_on(); }
+
+ protected:
+  // views::View:
+  gfx::Size GetPreferredSize() const override {
+    gfx::Size size = views::View::GetPreferredSize();
+    size.set_height(kMenuButtonSize + kKeyboardRowVerticalInset * 2);
+    return size;
+  }
+
+  int GetHeightForWidth(int w) const override {
+    return GetPreferredSize().height();
+  }
+
+  void OnNativeThemeChanged(const ui::NativeTheme* theme) override {
+    UpdateStyle();
+  }
+
+ private:
+  void Init() {
+    SetBorder(views::Border::CreateSolidSidedBorder(
+        kKeyboardRowSeparatorThickness, 0, 0, 0, kKeyboardRowSeparatorColor));
+    TriView* tri_view = TrayPopupUtils::CreateDefaultRowView();
+    AddChildView(tri_view);
+    SetLayoutManager(new views::FillLayout);
+
+    // The on-screen keyboard image button.
+    views::ImageView* keyboard_image = TrayPopupUtils::CreateMainImageView();
+    keyboard_image->SetImage(gfx::CreateVectorIcon(
+        kImeMenuOnScreenKeyboardIcon, kMenuIconSize, kMenuIconColor));
+    tri_view->AddView(TriView::Container::START, keyboard_image);
+
+    // The on-screen keyboard label.
+    label_ = new views::Label(
+        ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD));
+    label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    tri_view->AddView(TriView::Container::CENTER, label_);
+
+    // The on-screen keyboard toggle button.
+    toggle_ = new views::ToggleButton(listener_);
+    toggle_->SetFocusForPlatform();
+    SetKeyboardToggleText();
+    tri_view->AddView(TriView::Container::END, toggle_);
+  }
+
+  // Updates the style of |label_| based on the current native theme.
+  void UpdateStyle() {

[tdanderson, 2016/11/04 19:01:05]

Thanks for changing this. I'm 99% sure this is the right thing to do as compared
to what you had in the last patch set, but did you happen to double-check that
the font style/size still looks correct?

[Azure Wei, 2016/11/05 02:26:19]

Added calling of UpdateStyle() in Init() to make sure this will be run.

+    TrayPopupItemStyle style(
+        GetNativeTheme(), TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
+    style.SetupLabel(label_);
+  }
+
+  // ButtonListener to notify when |toggle_| is clicked.
+  views::ButtonListener* listener_;
+
+  // Label to with text 'On-screen keyboard'.
+  views::Label* label_;
+
+  // ToggleButton to toggle keyboard on or off.
+  views::ToggleButton* toggle_;
+
+  DISALLOW_COPY_AND_ASSIGN(MaterialKeyboardStatusRowView);
+};
+
 ImeListView::ImeListView(SystemTrayItem* owner,
                          bool show_keyboard_toggle,
                          SingleImeBehavior single_ime_behavior)
@@ -172,7 +274,12 @@ void ImeListView::Update(const IMEInfoList& list,
                          const IMEPropertyInfoList& property_list,
                          bool show_keyboard_toggle,
                          SingleImeBehavior single_ime_behavior) {
-  Reset();
+  ResetImeListView();
+  if (show_keyboard_toggle &&
+      MaterialDesignController::IsSystemTrayMenuMaterial()) {
+    AppendMaterialKeyboardStatus();
+  }
+
   ime_map_.clear();
   property_map_.clear();
   CreateScrollableList();
@@ -191,6 +298,7 @@ void ImeListView::Update(const IMEInfoList& list,
   if (show_keyboard_toggle) {
     if (list.size() > 1 || !property_list.empty())
       AddScrollSeparator();
+    DCHECK(!MaterialDesignController::IsSystemTrayMenuMaterial());

[tdanderson, 2016/11/04 19:01:05]

I think you will want to move this to the top of AppendKeyboardStatus(), and
line 298 should be changed back to be:

if (show_keyboard_toggle && !MDC::IsSystemTrayMenuMaterial())

Otherwise with the code as-is you could actually append both rows.

[Azure Wei, 2016/11/05 02:26:19]

Sorry, I've got wrong about the scroll separator. There should be no separator
if no keyboard toggle.

     AppendKeyboardStatus();
   }
 
@@ -198,6 +306,13 @@ void ImeListView::Update(const IMEInfoList& list,
   SchedulePaint();
 }
 
+void ImeListView::ResetImeListView() {
+  // Children are removed from the view hierarchy and deleted in Reset().
+  Reset();
+  material_keyboard_statuts_view_ = nullptr;
+  keyboard_status_ = nullptr;
+}
+
 void ImeListView::AppendIMEList(const IMEInfoList& list) {
   DCHECK(ime_map_.empty());
   for (size_t i = 0; i < list.size(); i++) {
@@ -265,6 +380,17 @@ void ImeListView::AppendKeyboardStatus() {
   keyboard_status_ = container;
 }
 
+void ImeListView::AppendMaterialKeyboardStatus() {
+  DCHECK(MaterialDesignController::IsSystemTrayMenuMaterial());
+  if (material_keyboard_statuts_view_)
+    return;
+  MaterialKeyboardStatusRowView* view =
+      new MaterialKeyboardStatusRowView(this, keyboard::IsKeyboardEnabled());
+  view->SetKeyboardStatusEnabled(keyboard::IsKeyboardEnabled());
+  AddChildView(view);
+  material_keyboard_statuts_view_ = view;
+}
+
 void ImeListView::HandleViewClicked(views::View* view) {
   if (view == keyboard_status_) {
     WmShell::Get()->ToggleIgnoreExternalKeyboard();
@@ -289,4 +415,13 @@ void ImeListView::HandleViewClicked(views::View* view) {
   GetWidget()->Close();
 }
 
+void ImeListView::ButtonPressed(views::Button* sender, const ui::Event& event) {
+  if (material_keyboard_statuts_view_ &&
+      sender == material_keyboard_statuts_view_->toggle()) {
+    WmShell::Get()->ToggleIgnoreExternalKeyboard();
+    // Set the toggle's a11y text.
+    material_keyboard_statuts_view_->SetKeyboardToggleText();
+  }
+}
+
 }  // namespace ash