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.

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

Please update the CL title to something more descriptive of the bug you're
fixing, such as "Request focus on IME menu rows after keyboard selection to
trigger spoken feedback"

[Azure Wei, 2016/12/09 00:23:54]

Done.

 
 #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;
@@ skipping 16 matching lines @@
 
   // 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();
 
+  // Closes the view.
+  void CloseImeListView();
+
+  void SetLastItemSelectedWithKeyboard(bool last_item_selected_with_keyboard) {

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

Please use this_naming_style() instead of CamelCase for function names when
you're inlining them in the header (here and on line 47).

[Azure Wei, 2016/12/09 00:23:54]

Done. Renamed as set_last_item_selected_with_keyboard().

+    last_item_selected_with_keyboard_ = last_item_selected_with_keyboard;
+  }
+
+  void SetFocusCurrentIme(const bool focus_current_ime) {
+    focus_current_ime_ = focus_current_ime;

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

I find the name |focus_current_ime_| a bit ambiguous, and therefore I think it
makes readability a bit difficult at call sites. Can I suggest the alternate
namings:

* |should_focus_ime_after_selection_with_keyboard_| instead of
|focus_current_ime_|

* set_should_focus_ime_after_selection_with_keyboard() instead of
SetFocusCurrentIme()

* should_focus_ime_after_selection_with_keyboard() instead of
focus_current_ime()

[Azure Wei, 2016/12/09 00:23:54]

Done.

+  }
+
+  bool focus_current_ime() const { return focus_current_ime_; }
+
   // 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.
   void AppendIMEList(const IMEInfoList& list);
 
   // Appends the IME listed to the scrollable area of the detailed view.
   void AppendIMEProperties(const IMEPropertyInfoList& property_list);
 
   // Appends the IMEs and properties to the scrollable area  in the material
   // design IME menu.
   void AppendImeListAndProperties(const IMEInfoList& list,
                                   const IMEPropertyInfoList& property_list);
 
   // Appends the on-screen keyboard status to the last area of the detailed
   // view.
   void AppendKeyboardStatus();
 
   // Inserts the material on-screen keyboard status in the detailed view.
   void PrependMaterialKeyboardStatus();
 
+  // Requests focus on the current IME if it was selected with keyboard so that
+  // accessible text will alert the user of the IME change.
+  void FocusCurrentImeIfNeeded();
+
   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_;
 
+  // The id of the last item selected with keyboard. It will be empty if the
+  // item is not selected with keyboard.
+  std::string last_selected_item_id_;
+
+  // True if the last item is selected with keyboard.
+  bool last_item_selected_with_keyboard_;
+
+  // True if supporting automatically focus to the selected IME when switching
+  // with keyboard.

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

nit: suggested wording: "True if focus should be requested after switching IMEs
with keyboard in order to trigger spoken feedback with ChromeVox enabled." or
similar.

[Azure Wei, 2016/12/09 00:23:54]

Done.

+  bool focus_current_ime_;
+
   DISALLOW_COPY_AND_ASSIGN(ImeListView);
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_SYSTEM_CHROMEOS_IME_MENU_IME_LIST_VIEW_H_