Make ActionView focusable.

Adds focus and ink drop behaviors to ActionView to be consistent with
other DialogPlate buttons.

Bug: b:112925587
Change-Id: Ia6fa90478a2d28d56e5fda0bfd0988239df64c01
Reviewed-on: https://chromium-review.googlesource.com/1189445Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#586408}

ash/assistant/ui/dialog_plate/action_view.cc

@@ -9,10 +9,9 @@
 #include "ash/assistant/assistant_controller.h"
 #include "ash/assistant/assistant_interaction_controller.h"
 #include "ash/assistant/ui/logo_view/base_logo_view.h"
-#include "ash/resources/vector_icons/vector_icons.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/gfx/paint_vector_icon.h"
-#include "ui/views/controls/image_view.h"
+#include "ash/assistant/util/views_util.h"
+#include "ui/views/controls/button/image_button.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 
 namespace ash {
@@ -20,15 +19,17 @@ namespace ash {
 namespace {
 
 // Appearance.
-constexpr int kPreferredSizeDip = 22;
+constexpr int kIconSizeDip = 22;
+constexpr int kPreferredSizeDip = 32;
 
 }  // namespace
 
-ActionView::ActionView(AssistantController* assistant_controller,
-                       ActionViewListener* listener)
-    : assistant_controller_(assistant_controller), listener_(listener) {
+ActionView::ActionView(views::ButtonListener* listener,
+                       AssistantController* assistant_controller)
+    : views::Button(/*listener=*/nullptr),
+      assistant_controller_(assistant_controller),
+      listener_(listener) {
   InitLayout();
-  UpdateState(/*animate=*/false);
 
   // The Assistant controller indirectly owns the view hierarchy to which
   // ActionView belongs so is guaranteed to outlive it.
@@ -40,33 +41,51 @@ ActionView::~ActionView() {
 }
 
 gfx::Size ActionView::CalculatePreferredSize() const {
-  return gfx::Size(kPreferredSizeDip, kPreferredSizeDip);
+  return gfx::Size(kPreferredSizeDip, GetHeightForWidth(kPreferredSizeDip));
+}
+
+int ActionView::GetHeightForWidth(int width) const {
+  return kPreferredSizeDip;
+}
+
+void ActionView::RequestFocus() {
+  button_->RequestFocus();
+}
+
+void ActionView::ButtonPressed(views::Button* sender, const ui::Event& event) {
+  if (listener_)
+    listener_->ButtonPressed(this, event);
 }
 
 void ActionView::InitLayout() {
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
-  // Voice action.
-  voice_action_view_ = BaseLogoView::Create();
-  voice_action_view_->SetPreferredSize(
-      gfx::Size(kPreferredSizeDip, kPreferredSizeDip));
-  AddChildView(voice_action_view_);
-}
+  // Button.
+  button_ = assistant::util::CreateButton(this, kPreferredSizeDip);
+  AddChildView(button_);
 
-void ActionView::OnGestureEvent(ui::GestureEvent* event) {
-  if (event->type() != ui::ET_GESTURE_TAP)
-    return;
+  // Voice action container.
+  views::View* voice_action_container_ = new views::View();
+  voice_action_container_->set_can_process_events_within_subtree(false);
+  AddChildView(voice_action_container_);
 
-  event->SetHandled();
+  views::BoxLayout* layout_manager = voice_action_container_->SetLayoutManager(
+      std::make_unique<views::BoxLayout>(
+          views::BoxLayout::Orientation::kVertical));
 
-  if (listener_)
-    listener_->OnActionPressed();
-}
+  layout_manager->set_cross_axis_alignment(
+      views::BoxLayout::CrossAxisAlignment::CROSS_AXIS_ALIGNMENT_CENTER);
 
-bool ActionView::OnMousePressed(const ui::MouseEvent& event) {
-  if (listener_)
-    listener_->OnActionPressed();
-  return true;
+  layout_manager->set_main_axis_alignment(
+      views::BoxLayout::MainAxisAlignment::MAIN_AXIS_ALIGNMENT_CENTER);
+
+  // Voice action.
+  voice_action_view_ = BaseLogoView::Create();
+  voice_action_view_->SetPreferredSize(gfx::Size(kIconSizeDip, kIconSizeDip));
+  voice_action_container_->AddChildView(voice_action_view_);
+
+  // Initialize state.
+  UpdateState(/*animate=*/false);
 }
 
 void ActionView::OnMicStateChanged(MicState mic_state) {
@@ -104,4 +123,12 @@ void ActionView::UpdateState(bool animate) {
   voice_action_view_->SetState(mic_state, animate);
 }
 
+void ActionView::SetAccessibleName(const base::string16& accessible_name) {
+  button_->SetAccessibleName(accessible_name);
+}
+
+void ActionView::SetFocusBehavior(FocusBehavior focus_behavior) {
+  button_->SetFocusBehavior(focus_behavior);
+}
+
 }  // namespace ash

ash/assistant/ui/dialog_plate/action_view.h

@@ -7,7 +7,7 @@
 
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "base/macros.h"
-#include "ui/views/view.h"
+#include "ui/views/controls/button/button.h"
 
 namespace ash {
 
@@ -15,34 +15,31 @@ class ActionView;
 class AssistantController;
 class BaseLogoView;
 
-// Listener which receives notification of action view events.
-class ActionViewListener {
- public:
-  // Invoked when the action is pressed.
-  virtual void OnActionPressed() = 0;
-
- protected:
-  virtual ~ActionViewListener() = default;
-};
-
 // A stateful view belonging to DialogPlate which indicates current user input
 // modality and delivers notification of press events.
-class ActionView : public views::View,
+class ActionView : public views::Button,
+                   public views::ButtonListener,
                    public AssistantInteractionModelObserver {
  public:
-  ActionView(AssistantController* assistant_controller,
-             ActionViewListener* listener);
+  ActionView(views::ButtonListener* listener,
+             AssistantController* assistant_controller);
   ~ActionView() override;
 
-  // views::View:
+  // views::Button:
   gfx::Size CalculatePreferredSize() const override;
-  void OnGestureEvent(ui::GestureEvent* event) override;
-  bool OnMousePressed(const ui::MouseEvent& event) override;
+  int GetHeightForWidth(int width) const override;
+  void RequestFocus() override;
+
+  // views::ButtonListener:
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // AssistantInteractionModelObserver:
   void OnMicStateChanged(MicState mic_state) override;
   void OnSpeechLevelChanged(float speech_level_db) override;
 
+  void SetAccessibleName(const base::string16& accessible_name);
+  void SetFocusBehavior(FocusBehavior focus_behavior);
+
  private:
   void InitLayout();
 
@@ -51,8 +48,9 @@ class ActionView : public views::View,
   void UpdateState(bool animate);
 
   AssistantController* const assistant_controller_;  // Owned by Shell.
-  ActionViewListener* listener_;
+  views::ButtonListener* const listener_;
 
+  views::Button* button_;                   // Owned by view hierarchy.
   BaseLogoView* voice_action_view_;         // Owned by view hierarchy.
 
   // True when speech level goes above a threshold and sets LogoView in

ash/assistant/ui/dialog_plate/dialog_plate.cc

@@ -37,7 +37,6 @@ namespace {
 // Appearance.
 constexpr int kIconSizeDip = 20;
 constexpr int kButtonSizeDip = 32;
-constexpr int kKeyboardLayoutPaddingDip = 16;
 constexpr int kPreferredHeightDip = 48;
 
 // Animation.
@@ -51,6 +50,8 @@ constexpr base::TimeDelta kAnimationTransformInDuration =
     base::TimeDelta::FromMilliseconds(333);
 constexpr int kAnimationTranslationDip = 30;
 
+// Helpers ---------------------------------------------------------------------
+
 bool IsTabletMode() {
   return Shell::Get()
       ->tablet_mode_controller()
@@ -107,10 +108,6 @@ void DialogPlate::ChildVisibilityChanged(views::View* child) {
   PreferredSizeChanged();
 }
 
-void DialogPlate::OnActionPressed() {
-  OnButtonPressed(DialogPlateButtonId::kVoiceInputToggle);
-}
-
 void DialogPlate::ButtonPressed(views::Button* sender, const ui::Event& event) {
   OnButtonPressed(static_cast<DialogPlateButtonId>(sender->id()));
 }
@@ -234,7 +231,9 @@ void DialogPlate::OnUiVisibilityChanged(AssistantVisibility new_visibility,
 }
 
 void DialogPlate::RequestFocus() {
-  textfield_->RequestFocus();
+  SetFocus(assistant_controller_->interaction_controller()
+               ->model()
+               ->input_modality());
 }
 
 void DialogPlate::InitLayout() {
@@ -281,11 +280,13 @@ void DialogPlate::InitKeyboardLayoutContainer() {
   keyboard_layout_container_->layer()->SetFillsBoundsOpaquely(false);
   keyboard_layout_container_->layer()->SetOpacity(0.f);
 
+  constexpr int kHorizontalPaddingDip = 16;
+
   views::BoxLayout* layout_manager =
       keyboard_layout_container_->SetLayoutManager(
           std::make_unique<views::BoxLayout>(
               views::BoxLayout::Orientation::kHorizontal,
-              gfx::Insets(0, kKeyboardLayoutPaddingDip)));
+              gfx::Insets(0, kHorizontalPaddingDip)));
 
   layout_manager->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::CROSS_AXIS_ALIGNMENT_CENTER);
@@ -314,8 +315,7 @@ void DialogPlate::InitKeyboardLayoutContainer() {
   // Voice input toggle.
   voice_input_toggle_ = assistant::util::CreateImageButton(
       this, kMicIcon, kButtonSizeDip, kIconSizeDip,
-      IDS_ASH_ASSISTANT_DIALOG_PLATE_MIC_ACCNAME,
-      /*no use, color icon*/gfx::kGoogleGrey600);
+      IDS_ASH_ASSISTANT_DIALOG_PLATE_MIC_ACCNAME);
   voice_input_toggle_->set_id(
       static_cast<int>(DialogPlateButtonId::kVoiceInputToggle));
   keyboard_layout_container_->AddChildView(voice_input_toggle_);
@@ -353,8 +353,12 @@ void DialogPlate::InitVoiceLayoutContainer() {
   layout_manager->SetFlexForView(spacer, 1);
 
   // Animated voice input toggle.
-  voice_layout_container_->AddChildView(
-      new ActionView(assistant_controller_, this));
+  animated_voice_input_toggle_ = new ActionView(this, assistant_controller_);
+  animated_voice_input_toggle_->set_id(
+      static_cast<int>(DialogPlateButtonId::kVoiceInputToggle));
+  animated_voice_input_toggle_->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_ASH_ASSISTANT_DIALOG_PLATE_MIC_ACCNAME));
+  voice_layout_container_->AddChildView(animated_voice_input_toggle_);
 
   // Spacer.
   spacer = new views::View();
@@ -384,11 +388,11 @@ bool DialogPlate::OnAnimationEnded(
                                      ->model()
                                      ->input_modality();
   SetFocusMode(input_modality);
+  SetFocus(input_modality);
 
   switch (input_modality) {
     case InputModality::kKeyboard:
       keyboard_layout_container_->set_can_process_events_within_subtree(true);
-      textfield_->RequestFocus();
       break;
     case InputModality::kVoice:
       voice_layout_container_->set_can_process_events_within_subtree(true);
@@ -402,17 +406,33 @@ bool DialogPlate::OnAnimationEnded(
   return false;
 }
 
-void DialogPlate::SetFocusMode(InputModality modality) {
-  switch (modality) {
+void DialogPlate::SetFocus(InputModality input_modality) {
+  switch (input_modality) {
+    case InputModality::kKeyboard:
+      textfield_->RequestFocus();
+      break;
+    case InputModality::kVoice:
+      animated_voice_input_toggle_->RequestFocus();
+      break;
+    case InputModality::kStylus:
+      // No action necessary.
+      break;
+  }
+}
+
+void DialogPlate::SetFocusMode(InputModality input_modality) {
+  switch (input_modality) {
     case InputModality::kKeyboard:
       textfield_->SetFocusBehavior(FocusBehavior::ALWAYS);
       voice_input_toggle_->SetFocusBehavior(FocusBehavior::ALWAYS);
       keyboard_input_toggle_->SetFocusBehavior(FocusBehavior::NEVER);
+      animated_voice_input_toggle_->SetFocusBehavior(FocusBehavior::NEVER);
       break;
     case InputModality::kVoice:
       textfield_->SetFocusBehavior(FocusBehavior::NEVER);
       voice_input_toggle_->SetFocusBehavior(FocusBehavior::NEVER);
       keyboard_input_toggle_->SetFocusBehavior(FocusBehavior::ALWAYS);
+      animated_voice_input_toggle_->SetFocusBehavior(FocusBehavior::ALWAYS);
       break;
     case InputModality::kStylus:
       // No action necessary.

ash/assistant/ui/dialog_plate/dialog_plate.h

@@ -28,6 +28,7 @@ class ImageButton;
 namespace ash {
 
 class AssistantController;
+class ActionView;
 
 // DialogPlateButtonId ---------------------------------------------------------
 
@@ -60,7 +61,6 @@ class DialogPlateObserver {
 // interaction as appropriate for the user's current input modality.
 class DialogPlate : public views::View,
                     public views::TextfieldController,
-                    public ActionViewListener,
                     public AssistantInteractionModelObserver,
                     public AssistantUiModelObserver,
                     public views::ButtonListener {
@@ -79,9 +79,6 @@ class DialogPlate : public views::View,
   void ChildVisibilityChanged(views::View* child) override;
   void RequestFocus() override;
 
-  // ActionViewListener:
-  void OnActionPressed() override;
-
   // ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
@@ -106,6 +103,8 @@ class DialogPlate : public views::View,
 
   void OnAnimationStarted(const ui::CallbackLayerAnimationObserver& observer);
   bool OnAnimationEnded(const ui::CallbackLayerAnimationObserver& observer);
+
+  void SetFocus(InputModality modality);
   void SetFocusMode(InputModality modality);
 
   AssistantController* const assistant_controller_;  // Owned by Shell.
@@ -115,6 +114,7 @@ class DialogPlate : public views::View,
   views::View* voice_layout_container_;              // Owned by view hierarchy.
   views::ImageButton* keyboard_input_toggle_;        // Owned by view hierarchy.
   views::ImageButton* voice_input_toggle_;           // Owned by view hierarchy.
+  ActionView* animated_voice_input_toggle_;          // Owned by view hierarchy.
   views::ImageButton* settings_button_;              // Owned by view hierarchy.
   views::Textfield* textfield_;                      // Owned by view hierarchy.
 

ash/assistant/util/views_util.cc

@@ -66,35 +66,48 @@ class DefaultImageButton : public views::ImageButton {
 
 }  // namespace
 
-views::ImageButton* CreateImageButton(views::ButtonListener* listener,
-                                      const gfx::VectorIcon& icon,
+views::ImageButton* CreateButton(views::ButtonListener* listener,
                                  int size_in_dip,
-                                      int icon_size_in_dip,
-                                      int accessible_name_id,
-                                      SkColor icon_color) {
+                                 base::Optional<int> accessible_name_id) {
   constexpr SkColor kInkDropBaseColor = SK_ColorBLACK;
   constexpr float kInkDropVisibleOpacity = 0.06f;
 
   auto* button = new DefaultImageButton(listener);
 
+  if (accessible_name_id.has_value()) {
+    button->SetAccessibleName(
+        l10n_util::GetStringUTF16(accessible_name_id.value()));
+  }
+
   button->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
                             views::ImageButton::ALIGN_MIDDLE);
 
-  button->SetImage(views::Button::STATE_NORMAL,
-                   gfx::CreateVectorIcon(icon, icon_size_in_dip, icon_color));
-
   button->SetInkDropMode(views::Button::InkDropMode::ON);
   button->set_has_ink_drop_action_on_click(true);
   button->set_ink_drop_base_color(kInkDropBaseColor);
   button->set_ink_drop_visible_opacity(kInkDropVisibleOpacity);
 
-  button->SetPreferredSize(gfx::Size(size_in_dip, size_in_dip));
   button->SetFocusForPlatform();
   button->SetFocusPainter(views::Painter::CreateSolidRoundRectPainter(
       SkColorSetA(kInkDropBaseColor, 0xff * kHighlightOpacity),
       size_in_dip / 2 - kInkDropInset, gfx::Insets(kInkDropInset)));
 
-  button->SetAccessibleName(l10n_util::GetStringUTF16(accessible_name_id));
+  button->SetPreferredSize(gfx::Size(size_in_dip, size_in_dip));
+
+  return button;
+}
+
+views::ImageButton* CreateImageButton(views::ButtonListener* listener,
+                                      const gfx::VectorIcon& icon,
+                                      int size_in_dip,
+                                      int icon_size_in_dip,
+                                      int accessible_name_id,
+                                      SkColor icon_color) {
+  auto* button = CreateButton(listener, size_in_dip, accessible_name_id);
+
+  button->SetImage(views::Button::STATE_NORMAL,
+                   gfx::CreateVectorIcon(icon, icon_size_in_dip, icon_color));
+
   return button;
 }
 

ash/assistant/util/views_util.h

@@ -5,7 +5,9 @@
 #ifndef ASH_ASSISTANT_UTIL_VIEWS_UTIL_H_
 #define ASH_ASSISTANT_UTIL_VIEWS_UTIL_H_
 
+#include "base/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/color_palette.h"
 
 namespace gfx {
 struct VectorIcon;
@@ -20,13 +22,19 @@ namespace ash {
 namespace assistant {
 namespace util {
 
+// Creates a button with the default Assistant styles.
+views::ImageButton* CreateButton(
+    views::ButtonListener* listener,
+    int size_in_dip,
+    base::Optional<int> accessible_name_id = base::nullopt);
+
 // Creates an ImageButton with the default Assistant styles.
 views::ImageButton* CreateImageButton(views::ButtonListener* listener,
                                       const gfx::VectorIcon& icon,
                                       int size_in_dip,
                                       int icon_size_in_dip,
                                       int accessible_name_id,
-                                      SkColor icon_color);
+                                      SkColor icon_color = gfx::kGoogleGrey600);
 
 }  // namespace util
 }  // namespace assistant