[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..513740edfdafc607bb7fa0a5952177138972f37b 100644
--- a/ash/common/system/chromeos/ime_menu/ime_list_view.cc
+++ b/ash/common/system/chromeos/ime_menu/ime_list_view.cc
@@ -7,6 +7,7 @@
 #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"
@@ -24,14 +25,25 @@
 #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/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 kKeyboardRowkVerticalInset = 4;
+const int kKeyboardRowSeparatorThickness = 1;
+const int kKeyboardRowChildSpacing = 14;
+const int kFocusBorderInset = 1;
+
+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 =
@@ -152,8 +164,107 @@ class ImeListItemView : public ActionableView {
   DISALLOW_COPY_AND_ASSIGN(ImeListItemView);
 };
 
+// The view that contains on-screen keyboard image and label.
+class KeyboardButtonView : public views::View {
+ public:
+  KeyboardButtonView() {
+    views::BoxLayout* box_layout =
+        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
+    SetLayoutManager(box_layout);
+
+    ui::NativeTheme* theme = GetNativeTheme();
+    SystemMenuButton* on_screen_keyboard_ = new SystemMenuButton(

[tdanderson, 2016/11/01 21:42:11]

I checked with Sebastien, and the toggle button is the only thing that should be
clickable on this row. It's also the only thing that should have a ripple (which
it already has baked-in). The keyboard icon should just be an image view instead
of a SystemMenuButton.

[Azure Wei, 2016/11/02 15:01:41]

Updated this as ImageButton.

+        nullptr, SystemMenuButton::InkDropStyle::SQUARE,
+        kImeMenuOnScreenKeyboardIcon,
+        IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD);

[tdanderson, 2016/11/01 21:42:11]

In terms of accessibility, the a11y string should be on the toggle button
itself. I filed crbug.com/661319 to track that work; I'll leave it to you if you
want to include that change as part of this CL or defer to a follow-up CL.

[Azure Wei, 2016/11/02 15:01:41]

Sets the toggle button's a11y text here.

+    const SkColor focus_color =
+        theme->GetSystemColor(ui::NativeTheme::kColorId_FocusedBorderColor);
+    on_screen_keyboard_->SetFocusForPlatform();
+    on_screen_keyboard_->SetFocusPainter(
+        views::Painter::CreateSolidFocusPainter(
+            focus_color, gfx::Insets(kFocusBorderInset)));
+    AddChildView(on_screen_keyboard_);
+
+    views::Label* label_ = new views::Label(
+        ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD));
+    label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    AddChildView(label_);
+    box_layout->SetFlexForView(label_, 1);
+  }
+
+  ~KeyboardButtonView() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(KeyboardButtonView);
+};
+
 }  // namespace
 
+// The view that contains a |KeyboardButtonView| and a toggle button.
+class ImeListView::MaterialKeyboardStatusRowView : public views::View {
+ public:
+  MaterialKeyboardStatusRowView(views::ButtonListener* listener, bool enabled)
+      : views::View(), listener_(listener), toggle_(nullptr) {
+    Init();
+    SetKeyboardStatusEnabled(enabled);
+  }
+
+  ~MaterialKeyboardStatusRowView() override {}
+
+  void SetKeyboardStatusEnabled(bool enabled) {
+    toggle_->SetIsOn(enabled, true);
+  }
+
+  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 + kKeyboardRowkVerticalInset * 2);
+    return size;
+  }
+
+  int GetHeightForWidth(int w) const override {
+    return GetPreferredSize().height();
+  }
+
+ private:
+  void Init() {
+    SetBorder(views::Border::CreateSolidSidedBorder(
+        kKeyboardRowSeparatorThickness, 0, 0, 0, kKeyboardRowSeparatorColor));
+    views::View* container = new views::View;
+    container->SetBorder(views::Border::CreateEmptyBorder(
+        kKeyboardRowkVerticalInset, 0, kKeyboardRowkVerticalInset, 0));
+    views::FillLayout* layout = new views::FillLayout;
+    SetLayoutManager(layout);

[tdanderson, 2016/11/01 21:42:11]

Instead of adding this layout code and introducing the separate class
KeyboardButtonView, this seems like a good candidate for the TriView class.
TriView was designed to handle the layout of rows in the system menu. You should
be able to do something like the following:

TriView* tri_view = TrayPopupUtils::CreateDefaultRowView();
AddChildView(tri_view);
SetLayoutManager(new views::FillLayout);
tri_view->AddView(TriView::Container::START, icon);
tri_view->AddView(TriView::Container::CENTER, label);
tri_view->AddView(TriView::Container::END, toggle);

See tray_item_more.cc in https://chromiumcodereview.appspot.com/2468533002/ as a
concrete example of where and how TriView is used.

[Azure Wei, 2016/11/02 15:01:41]

Thanks for the detailed advice of using TriView.
I've updated this as well as the ImeListItemView. Please take another look.

+    AddChildView(container);
+
+    views::BoxLayout* container_layout = new views::BoxLayout(
+        views::BoxLayout::kHorizontal, 0, 0, kKeyboardRowChildSpacing);
+    container_layout->set_cross_axis_alignment(
+        views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
+    container->SetLayoutManager(container_layout);
+    KeyboardButtonView* keyboard_button_view = new KeyboardButtonView();
+    container->AddChildView(keyboard_button_view);
+    container_layout->SetFlexForView(keyboard_button_view, 1);
+
+    toggle_ = new views::ToggleButton(listener_);
+    toggle_->SetFocusForPlatform();
+    container->AddChildView(toggle_);
+  }
+
+  // ButtonListener to notify when |toggle_| is clicked.
+  views::ButtonListener* listener_;
+
+  // 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)
@@ -173,6 +284,11 @@ void ImeListView::Update(const IMEInfoList& list,
                          bool show_keyboard_toggle,
                          SingleImeBehavior single_ime_behavior) {
   Reset();
+  if (show_keyboard_toggle &&
+      MaterialDesignController::IsSystemTrayMenuMaterial()) {
+    AppendMaterialKeyboardStatus();
+  }
+
   ime_map_.clear();
   property_map_.clear();
   CreateScrollableList();
@@ -188,7 +304,8 @@ void ImeListView::Update(const IMEInfoList& list,
     }
   }
 
-  if (show_keyboard_toggle) {
+  if (show_keyboard_toggle &&
+      !MaterialDesignController::IsSystemTrayMenuMaterial()) {
     if (list.size() > 1 || !property_list.empty())
       AddScrollSeparator();
     AppendKeyboardStatus();
@@ -265,6 +382,13 @@ void ImeListView::AppendKeyboardStatus() {
   keyboard_status_ = container;
 }
 
+void ImeListView::AppendMaterialKeyboardStatus() {
+  MaterialKeyboardStatusRowView* view =
+      new MaterialKeyboardStatusRowView(this, keyboard::IsKeyboardEnabled());
+  AddChildView(view);
+  material_keyboard_statuts_view_ = view;
+}
+
 void ImeListView::HandleViewClicked(views::View* view) {
   if (view == keyboard_status_) {
     WmShell::Get()->ToggleIgnoreExternalKeyboard();
@@ -289,4 +413,12 @@ 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() &&
+      material_keyboard_statuts_view_->is_toggled()) {
+    WmShell::Get()->ToggleIgnoreExternalKeyboard();
+  }
+}
+
 }  // namespace ash