Request focus on IME menu rows after keyboard selection

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

 // 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.
 
 #ifndef ASH_COMMON_SYSTEM_CHROMEOS_IME_MENU_IME_LIST_VIEW_H_
 #define ASH_COMMON_SYSTEM_CHROMEOS_IME_MENU_IME_LIST_VIEW_H_
 
 #include "ash/common/system/tray/ime_info.h"
 #include "ash/common/system/tray/tray_details_view.h"
 #include "ui/views/controls/button/button.h"
 
 namespace ash {
 class MaterialKeyboardStatusRowView;
 
 // The detailed view for showing IME list.
 class ImeListView : public TrayDetailsView {
  public:
   enum SingleImeBehavior {
     // Shows the IME menu if there's only one IME in system.
     SHOW_SINGLE_IME,
     // Hides the IME menu if there's only one IME in system.
     HIDE_SINGLE_IME
   };
 
   ImeListView(SystemTrayItem* owner,

[tdanderson, 2016/12/08 00:42:11]

Changes to ImeListView will also affect the IME detailed view in the system
menu. If you haven't already can you test that your CL works for the detailed
view too?

[Azure Wei, 2016/12/08 13:14:55]

Added a new bool member ImeListView::focus_current_ime_ to control whether we
will support this feature.

[tdanderson, 2016/12/08 22:56:37]

Thanks. Now that I try it out, I see that this is a problem for all detailed
views with checkable rows such as a11y, IME, audio, etc (though this was not a
regression caused by MD). I'll keep that on my radar for future work.

               bool show_keyboard_toggle,
               SingleImeBehavior single_ime_behavior);
 
   ~ImeListView() override;
 
+  // Focuses to the current IME if the IME is selected with keyboard.

[tdanderson, 2016/12/08 00:42:11]

I think you can make this private. Also it would be nice to have a bit more
description in the documentation here, such as "Requests focus on the current
IME if it was selected with keyboard so that accessible text will alert the user
of the IME change."

[Azure Wei, 2016/12/08 13:14:55]

Done.

+  void FocusCurrentImeIfNeeded();
+
   // Updates the view.
   virtual void Update(const IMEInfoList& list,
                       const IMEPropertyInfoList& property_list,
                       bool show_keyboard_toggle,
                       SingleImeBehavior single_ime_behavior);
 
   // Removes (and destroys) all child views.
   virtual void ResetImeListView();
 
+  // Handles the event that a view has been pressed.
+  void HandleViewPressed(views::View* view);

[tdanderson, 2016/12/08 00:42:11]

I think this can be made private. And to avoid confusion between
HandleViewPressed() and HandleViewClicked(), consider re-naming
HandleViewPressed() to be something like HandleViewClickedInternal() to signify
it is a helper that is called from HandleViewClicked().

[Azure Wei, 2016/12/08 13:14:55]

This is declared public since it's called by ImeListItemView.
To make it clear, I've removed the new HandleViewPressed() method and added
CloseImeListView() method.

[tdanderson, 2016/12/08 22:56:37]

Acknowledged.

+
   // TrayDetailsView:
   void HandleViewClicked(views::View* view) override;
   void HandleButtonPressed(views::Button* sender,
                            const ui::Event& event) override;
 
  private:
   // To allow the test class to access |ime_map_|.
   friend class ImeMenuTrayTest;
 
   // Appends the IMEs to the scrollable area of the detailed view.
@@ skipping 13 matching lines @@
 
   // Inserts the material on-screen keyboard status in the detailed view.
   void PrependMaterialKeyboardStatus();
 
   std::map<views::View*, std::string> ime_map_;
   std::map<views::View*, std::string> property_map_;
   // On-screen keyboard view which is not used in material design.
   views::View* keyboard_status_;
   // On-screen keyboard view which is only used in material design.
   MaterialKeyboardStatusRowView* material_keyboard_status_view_;
+  std::string last_pressed_item_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ImeListView);
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_SYSTEM_CHROMEOS_IME_MENU_IME_LIST_VIEW_H_