Add action for each button on opt-in IME menu.

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

 // Copyright 2016 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 "ash/common/system/chromeos/ime_menu/ime_menu_tray.h"
 
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/chromeos/ime_menu/ime_list_view.h"
 #include "ash/common/system/tray/fixed_sized_scroll_view.h"
 #include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/system_tray_notifier.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_popup_header_button.h"
 #include "ash/common/system/tray/tray_utils.h"
 #include "ash/common/wm_lookup.h"
 #include "ash/common/wm_root_window_controller.h"
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "base/strings/utf_string_conversions.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
+#include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 
 namespace ash {
 
 namespace {
 
 // Returns the max height of ImeListView.
@@ skipping 40 matching lines @@
   button->SetBorder(views::Border::CreateSolidSidedBorder(0, 0, 0, right_border,
                                                           kBorderDarkColor));
   return button;
 }
 
 // The view that contains buttons shown on the bottom of IME menu.
 class ImeButtonsView : public views::View,
                        public views::ButtonListener,
                        public ViewClickListener {
  public:
-  ImeButtonsView(bool show_emoji_button,
+  ImeButtonsView(ImeMenuTray* ime_menu_tray,
+                 bool show_emoji_button,
                  bool show_voice_button,
                  bool show_handwriting_button,
-                 bool show_settings_button) {
+                 bool show_settings_button)
+      : ime_menu_tray_(ime_menu_tray) {
     SetBorder(

[James Cook, 2016/09/12 03:21:54]

nit: DCHECK(ime_menu_tray_) helps the reader understand that it cannot be null
(without adding a comment!)

[Azure Wei, 2016/09/12 13:12:56]

Done.

         views::Border::CreateSolidSidedBorder(1, 0, 0, 0, kBorderDarkColor));
 
     // If there's only one settings button, the bottom should be a label with
     // normal background. Otherwise, show button icons with header background.
     if (show_settings_button && !show_emoji_button && !show_voice_button &&
         !show_handwriting_button) {
       ShowOneSettingButton();
     } else {
       ShowButtons(show_emoji_button, show_voice_button, show_handwriting_button,
                   show_settings_button);
     }
     // TODO(azurewei): Add logic of switching between the two states when the
     // menu is shown.
   }
 
   ~ImeButtonsView() override {}
 
   // views::View:
   gfx::Size GetPreferredSize() const override {
     int size = GetTrayConstant(TRAY_POPUP_ITEM_HEIGHT);
     return gfx::Size(size, size);
   }
   int GetHeightForWidth(int width) const override {
     return GetTrayConstant(TRAY_POPUP_ITEM_HEIGHT);
   }
 
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override {
+    if (settings_button_ && sender == settings_button_) {

[James Cook, 2016/09/12 03:21:54]

nit: |sender| should never be null, so you don't need the |settings_button_ &&|
part. (Also below.)

[Azure Wei, 2016/09/12 13:12:56]

Done. Thanks.

+      ime_menu_tray_->HideImeMenuBubble();
+      ShowIMESettings();
+      return;
+    }
+
+    std::string appended_url;

[James Cook, 2016/09/12 03:21:54]

nit: This isn't a URL. It should be called something like |keyboard_type|.

[Azure Wei, 2016/09/12 13:12:56]

Done. Renamed as keyset.

     if (emoji_button_ && sender == emoji_button_) {

[James Cook, 2016/09/12 03:21:54]

nit: {} not needed for single-line if()

[Azure Wei, 2016/09/12 13:12:57]

Done.

-      // TODO(azurewei): Opens emoji palette.
+      appended_url = "emoji";
     } else if (voice_button_ && sender == voice_button_) {
-      // TODO(azurewei): Brings virtual keyboard for emoji input.
+      appended_url = "voice";
     } else if (handwriting_button_ && sender == handwriting_button_) {
-      // TODO(azurewei): Brings virtual keyboard for handwriting input.
-    } else if (settings_button_ && sender == settings_button_) {
-      ShowIMESettings();
+      appended_url = "handwriting";
     }
+    if (appended_url.empty())

[James Cook, 2016/09/12 03:21:54]

nit: This should never happen. I would do:

else
  NOTREACHED();

[Azure Wei, 2016/09/12 13:12:57]

Done.

+      return;
+    ime_menu_tray_->HideImeMenuBubble();
+    chromeos::input_method::InputMethodManager::Get()->ShowInputViewWithUrl(

[James Cook, 2016/09/12 03:21:54]

I think it would be better for this function to take a keyboard type string (or
better yet, keyboard type enum). This level of the code should not need to know
about URL formats. (For example, this assumes the URL has other query parameters
on it and it's OK for the string to start with "&keyboard".)

[Azure Wei, 2016/09/12 13:12:56]

Done.

+        "&keyboard=" + appended_url);
   }
 
   // ViewClickListener:
   void OnViewClicked(views::View* sender) override {
     if (one_settings_button_view_ && sender == one_settings_button_view_) {
+      ime_menu_tray_->HideImeMenuBubble();
       ShowIMESettings();
     }
   }
 
  private:
   // Shows the UI of one settings button.
   void ShowOneSettingButton() {
     auto* box_layout =
         new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
     box_layout->SetDefaultFlex(1);
@@ skipping 13 matching lines @@
                    bool show_settings_button) {
     set_background(
         views::Background::CreateSolidBackground(kHeaderBackgroundColor));
     auto* box_layout = new views::BoxLayout(
         views::BoxLayout::kHorizontal, kTrayImeBottomRowPadding,
         kTrayImeBottomRowPadding, kTrayImeBottomRowPaddingBetweenItems);
     box_layout->SetDefaultFlex(1);
     SetLayoutManager(box_layout);
 
     if (show_emoji_button) {
       // TODO(azurewei): Creates the proper button with icons.

[James Cook, 2016/09/12 03:21:54]

How did you test this without creating buttons? Could you add placeholder
buttons (with intentionally incorrect art)?

[Azure Wei, 2016/09/12 13:12:57]

Added codes to show the buttons if features::kEHVInputOnImeMenu is enabled.

     }
 
     if (show_voice_button) {
       // TODO(azurewei): Creates the proper button with icons.
     }
 
     if (show_handwriting_button) {
       // TODO(azurewei): Creates the proper button with icons.
     }
 
     if (show_settings_button) {
       settings_button_ = CreateImeMenuButton(
           this, IDR_AURA_UBER_TRAY_SETTINGS, IDR_AURA_UBER_TRAY_SETTINGS,
           IDR_AURA_UBER_TRAY_SETTINGS, IDR_AURA_UBER_TRAY_SETTINGS,
           IDS_ASH_STATUS_TRAY_SETTINGS, IDS_ASH_STATUS_TRAY_SETTINGS, 0);
       AddChildView(settings_button_);
     }
   }
 
+  ImeMenuTray* ime_menu_tray_;
   TrayPopupHeaderButton* emoji_button_;
   TrayPopupHeaderButton* voice_button_;
   TrayPopupHeaderButton* handwriting_button_;
   TrayPopupHeaderButton* settings_button_;
   HoverHighlightView* one_settings_button_view_;
 
   DISALLOW_COPY_AND_ASSIGN(ImeButtonsView);
 };
 
 }  // namespace
@@ skipping 32 matching lines @@
     ime_list_view_->scroller()->SetFixedSize(
         gfx::Size(kTrayPopupMaxWidth, GetImeListViewMaxHeight()));
   }
   bubble_view->AddChildView(ime_list_view_);
 
   // Adds IME buttons to the bubble if needed.
   LoginStatus login =
       WmShell::Get()->system_tray_delegate()->GetUserLoginStatus();
   if (login != LoginStatus::NOT_LOGGED_IN && login != LoginStatus::LOCKED &&
       !WmShell::Get()->GetSessionStateDelegate()->IsInSecondaryLoginScreen())
-    bubble_view->AddChildView(new ImeButtonsView(false, false, false, true));
+    bubble_view->AddChildView(
+        new ImeButtonsView(this, false, false, false, true));
 
   bubble_.reset(new TrayBubbleWrapper(this, bubble_view));
   SetDrawBackgroundAsActive(true);
 }
 
+void ImeMenuTray::HideImeMenuBubble() {
+  bubble_.reset();
+  ime_list_view_ = nullptr;
+  SetDrawBackgroundAsActive(false);
+}
+
 bool ImeMenuTray::IsImeMenuBubbleShown() {
   return !!bubble_;
 }
 
 void ImeMenuTray::SetShelfAlignment(ShelfAlignment alignment) {
   TrayBackgroundView::SetShelfAlignment(alignment);
   if (!MaterialDesignController::IsShelfMaterial())
     tray_container()->SetBorder(views::Border::NullBorder());
 }
 
@@ skipping 89 matching lines @@
 void ImeMenuTray::UpdateTrayLabel() {
   WmShell::Get()->system_tray_delegate()->GetCurrentIME(&current_ime_);
 
   // Updates the tray label based on the current input method.
   if (current_ime_.third_party)
     label_->SetText(current_ime_.short_name + base::UTF8ToUTF16("*"));
   else
     label_->SetText(current_ime_.short_name);
 }
 
-void ImeMenuTray::HideImeMenuBubble() {
-  bubble_.reset();
-  ime_list_view_ = nullptr;
-  SetDrawBackgroundAsActive(false);
-}
-
 }  // namespace ash