Add keyboard accessibility for Quick Answers Consent-View

Allow the Quick-Answers UserConsentView to obtain keyboard-focus to pass
on to the settings, acknowledgement and dogfood buttons, using
arrow-keys.

Bug: b:152057976
Test: Tested on Chrome OS VM.
Change-Id: I01a4f6bd3da09e811398fe224b82e0c01f5add0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247156
Commit-Queue: Abhijeet Singh <siabhijeet@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781972}

ash/BUILD.gn

@@ -614,6 +614,8 @@ component("ash") {
     "quick_answers/quick_answers_controller_impl.h",
     "quick_answers/quick_answers_ui_controller.cc",
     "quick_answers/quick_answers_ui_controller.h",
+    "quick_answers/ui/quick_answers_focus_search.cc",
+    "quick_answers/ui/quick_answers_focus_search.h",
     "quick_answers/ui/quick_answers_pre_target_handler.cc",
     "quick_answers/ui/quick_answers_pre_target_handler.h",
     "quick_answers/ui/quick_answers_view.cc",

ash/quick_answers/ui/quick_answers_focus_search.cc

+// Copyright 2020 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/quick_answers/ui/quick_answers_focus_search.h"
+
+namespace ash {
+
+QuickAnswersFocusSearch::QuickAnswersFocusSearch(
+    views::View* view,
+    const GetFocusableViewsCallback& callback)
+    : FocusSearch(/*root=*/view, /*cycle=*/true, /*accessibility_mode=*/true),
+      view_(view),
+      get_focusable_views_callback_(callback) {}
+
+QuickAnswersFocusSearch::~QuickAnswersFocusSearch() = default;
+
+views::View* QuickAnswersFocusSearch::FindNextFocusableView(
+    views::View* starting_view,
+    SearchDirection search_direction,
+    TraversalDirection traversal_direction,
+    StartingViewPolicy check_starting_view,
+    AnchoredDialogPolicy can_go_into_anchored_dialog,
+    views::FocusTraversable** focus_traversable,
+    views::View** focus_traversable_view) {
+  DCHECK_EQ(root(), view_);
+
+  // The callback provided by |view_| polls the currently focusable views.
+  auto focusable_views = get_focusable_views_callback_.Run();
+  if (focusable_views.empty())
+    return nullptr;
+
+  int delta =
+      search_direction == FocusSearch::SearchDirection::kForwards ? 1 : -1;
+  int focusable_views_size = int{focusable_views.size()};
+  for (int i = 0; i < focusable_views_size; ++i) {
+    // If current view from the set is found to be focused, return the view
+    // next (or previous) to it as next focusable view.
+    if (focusable_views[i] == starting_view) {
+      int next_index =
+          (i + delta + focusable_views_size) % focusable_views_size;
+      return focusable_views[next_index];
+    }
+  }
+
+  // Case when none of the views are already focused.
+  return (search_direction == FocusSearch::SearchDirection::kForwards)
+             ? focusable_views.front()
+             : focusable_views.back();
+}
+
+views::FocusSearch* QuickAnswersFocusSearch::GetFocusSearch() {
+  return this;
+}
+
+views::FocusTraversable* QuickAnswersFocusSearch::GetFocusTraversableParent() {
+  return nullptr;
+}
+
+views::View* QuickAnswersFocusSearch::GetFocusTraversableParentView() {
+  return nullptr;
+}
+
+}  // namespace ash

ash/quick_answers/ui/quick_answers_focus_search.h

+// Copyright 2020 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_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_
+#define ASH_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_
+
+#include "base/callback.h"
+#include "ui/views/focus/focus_manager.h"
+#include "ui/views/focus/focus_search.h"
+
+namespace ash {
+
+// This class manages the focus traversal order for elements inside
+// Quick-Answers related views.
+// TODO(siabhijeet): QuickAnswersView is a menu-companion, so ideally should
+// avoid disturbing existing focus. Explore other ways for keyboard
+// accessibility.
+class QuickAnswersFocusSearch : public views::FocusSearch,
+                                public views::FocusTraversable {
+ public:
+  using GetFocusableViewsCallback =
+      base::RepeatingCallback<std::vector<views::View*>(void)>;
+
+  explicit QuickAnswersFocusSearch(views::View* view,
+                                   const GetFocusableViewsCallback& callback);
+
+  ~QuickAnswersFocusSearch() override;
+
+  // views::FocusSearch:
+  views::View* FindNextFocusableView(
+      views::View* starting_view,
+      SearchDirection search_direction,
+      TraversalDirection traversal_direction,
+      StartingViewPolicy check_starting_view,
+      AnchoredDialogPolicy can_go_into_anchored_dialog,
+      views::FocusTraversable** focus_traversable,
+      views::View** focus_traversable_view) override;
+
+  // views::FocusTraversable:
+  views::FocusSearch* GetFocusSearch() override;
+  views::FocusTraversable* GetFocusTraversableParent() override;
+  views::View* GetFocusTraversableParentView() override;
+
+ private:
+  views::View* const view_;
+  const GetFocusableViewsCallback get_focusable_views_callback_;
+};
+
+}  // namespace ash
+
+#endif  // ASH_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_

ash/quick_answers/ui/quick_answers_view.cc

@@ -24,7 +24,6 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/controls/menu/menu_controller.h"
-#include "ui/views/focus/focus_search.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/painter.h"
@@ -120,66 +119,6 @@ View* AddHorizontalUiElements(
 
 }  // namespace
 
-// QuickAnswersFocusSearch ----------------------------------------------------
-
-// This class manages the focus traversal order for elements inside
-// QuickAnswersView.
-// TODO(siabhijeet): QuickAnswersView is a menu-companion, so ideally should
-// avoid disturbing existing focus. Explore other ways for keyboard
-// accessibility.
-class QuickAnswersFocusSearch : public views::FocusSearch {
- public:
-  explicit QuickAnswersFocusSearch(QuickAnswersView* view)
-      : FocusSearch(/*root=*/view, /*cycle=*/true, /*accessibility_mode=*/true),
-        view_(view) {}
-
-  ~QuickAnswersFocusSearch() override = default;
-
-  // views::FocusSearch:
-  views::View* FindNextFocusableView(
-      views::View* starting_view,
-      SearchDirection search_direction,
-      TraversalDirection traversal_direction,
-      StartingViewPolicy check_starting_view,
-      AnchoredDialogPolicy can_go_into_anchored_dialog,
-      views::FocusTraversable** focus_traversable,
-      views::View** focus_traversable_view) override {
-    DCHECK_EQ(root(), view_);
-
-    std::vector<views::View*> focusable_views;
-    // |view_| is not included in focus loop for retry-view.
-    if (!view_->retry_label_)
-      focusable_views.push_back(view_);
-    if (view_->retry_label_ && view_->retry_label_->GetVisible())
-      focusable_views.push_back(view_->retry_label_);
-    if (view_->dogfood_button_ && view_->dogfood_button_->GetVisible())
-      focusable_views.push_back(view_->dogfood_button_);
-    if (focusable_views.empty())
-      return nullptr;
-
-    int delta =
-        search_direction == FocusSearch::SearchDirection::kForwards ? 1 : -1;
-    int focusable_views_size = int{focusable_views.size()};
-    for (int i = 0; i < focusable_views_size; ++i) {
-      // If current view from the set is found to be focused, return the view
-      // next (or previous) to it as next focusable view.
-      if (focusable_views[i] == starting_view) {
-        int next_index =
-            (i + delta + focusable_views_size) % focusable_views_size;
-        return focusable_views[next_index];
-      }
-    }
-
-    // Case when none of the views are already focused.
-    return (search_direction == FocusSearch::SearchDirection::kForwards)
-               ? focusable_views.front()
-               : focusable_views.back();
-  }
-
- private:
-  QuickAnswersView* const view_;
-};
-
 // QuickAnswersView -----------------------------------------------------------
 
 QuickAnswersView::QuickAnswersView(const gfx::Rect& anchor_view_bounds,
@@ -191,7 +130,10 @@ QuickAnswersView::QuickAnswersView(const gfx::Rect& anchor_view_bounds,
       title_(title),
       quick_answers_view_handler_(
           std::make_unique<QuickAnswersPreTargetHandler>(this)),
-      focus_search_(std::make_unique<QuickAnswersFocusSearch>(this)) {
+      focus_search_(std::make_unique<QuickAnswersFocusSearch>(
+          this,
+          base::BindRepeating(&QuickAnswersView::GetFocusableViews,
+                              base::Unretained(this)))) {
   InitLayout();
   InitWidget();
 
@@ -239,7 +181,20 @@ void QuickAnswersView::OnBlur() {
 }
 
 views::FocusTraversable* QuickAnswersView::GetPaneFocusTraversable() {
-  return this;
+  return focus_search_.get();
+}
+
+std::vector<views::View*> QuickAnswersView::GetFocusableViews() {
+  std::vector<views::View*> focusable_views;
+  // The view itself does not gain focus for retry-view and transfers it to the
+  // retry-label, and so is not included when this is the case.
+  if (!retry_label_)
+    focusable_views.push_back(this);
+  if (retry_label_ && retry_label_->GetVisible())
+    focusable_views.push_back(retry_label_);
+  if (dogfood_button_ && dogfood_button_->GetVisible())
+    focusable_views.push_back(dogfood_button_);
+  return focusable_views;
 }
 
 void QuickAnswersView::StateChanged(views::Button::ButtonState old_state) {
@@ -279,18 +234,6 @@ void QuickAnswersView::SetButtonNotifyActionToOnPress(views::Button* button) {
       views::ButtonController::NotifyAction::kOnPress);
 }
 
-views::FocusSearch* QuickAnswersView::GetFocusSearch() {
-  return focus_search_.get();
-}
-
-views::FocusTraversable* QuickAnswersView::GetFocusTraversableParent() {
-  return nullptr;
-}
-
-views::View* QuickAnswersView::GetFocusTraversableParentView() {
-  return nullptr;
-}
-
 void QuickAnswersView::SendQuickAnswersQuery() {
   controller_->OnQuickAnswersViewPressed();
 }

ash/quick_answers/ui/quick_answers_view.h

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "ash/quick_answers/ui/quick_answers_focus_search.h"
 #include "ui/events/event_handler.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/focus/focus_manager.h"
@@ -19,7 +20,6 @@ struct QuickAnswer;
 }  // namespace chromeos
 
 namespace views {
-class FocusSearch;
 class ImageButton;
 class Label;
 class LabelButton;
@@ -32,8 +32,7 @@ class QuickAnswersPreTargetHandler;
 
 // A bubble style view to show QuickAnswer.
 class ASH_EXPORT QuickAnswersView : public views::Button,
-                                    public views::ButtonListener,
-                                    public views::FocusTraversable {
+                                    public views::ButtonListener {
  public:
   QuickAnswersView(const gfx::Rect& anchor_view_bounds,
                    const std::string& title,
@@ -55,11 +54,6 @@ class ASH_EXPORT QuickAnswersView : public views::Button,
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
-  // views::FocusTraversable:
-  views::FocusSearch* GetFocusSearch() override;
-  views::FocusTraversable* GetFocusTraversableParent() override;
-  views::View* GetFocusTraversableParentView() override;
-
   // Called when a click happens to trigger Assistant Query.
   void SendQuickAnswersQuery();
 
@@ -72,8 +66,6 @@ class ASH_EXPORT QuickAnswersView : public views::Button,
   void ShowRetryView();
 
  private:
-  friend class QuickAnswersFocusSearch;
-
   void InitLayout();
   void InitWidget();
   void AddDogfoodButton();
@@ -88,6 +80,10 @@ class ASH_EXPORT QuickAnswersView : public views::Button,
   // mouse-release), since events of former type dismiss the accompanying menu.
   void SetButtonNotifyActionToOnPress(views::Button* button);
 
+  // QuickAnswersFocusSearch::GetFocusableViewsCallback to poll currently
+  // focusable views.
+  std::vector<views::View*> GetFocusableViews();
+
   gfx::Rect anchor_view_bounds_;
   QuickAnswersUiController* const controller_;
   bool has_second_row_answer_ = false;
@@ -100,7 +96,7 @@ class ASH_EXPORT QuickAnswersView : public views::Button,
   views::ImageButton* dogfood_button_ = nullptr;
 
   std::unique_ptr<QuickAnswersPreTargetHandler> quick_answers_view_handler_;
-  std::unique_ptr<views::FocusSearch> focus_search_;
+  std::unique_ptr<QuickAnswersFocusSearch> focus_search_;
   base::WeakPtrFactory<QuickAnswersView> weak_factory_{this};
 };
 }  // namespace ash

ash/quick_answers/ui/user_consent_view.cc

@@ -4,6 +4,7 @@
 
 #include "ash/quick_answers/ui/user_consent_view.h"
 
+#include "ash/accessibility/accessibility_controller_impl.h"
 #include "ash/public/cpp/vector_icons/vector_icons.h"
 #include "ash/quick_answers/quick_answers_ui_controller.h"
 #include "ash/quick_answers/ui/quick_answers_pre_target_handler.h"
@@ -18,14 +19,16 @@
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
-#include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/menu/menu_config.h"
+#include "ui/views/controls/menu/menu_controller.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/tooltip_manager.h"
 #include "ui/views/widget/widget.h"
+#include "ui/wm/core/coordinate_conversion.h"
 
 namespace ash {
 namespace quick_answers {
@@ -53,18 +56,14 @@ constexpr int kDescFontSizeDelta = 1;
 
 // Buttons common.
 constexpr int kButtonSpacingDip = 8;
-constexpr int kButtonBorderRadiusDip = 4;
-constexpr int kButtonBorderThicknessDip = 1;
 constexpr gfx::Insets kButtonBarInsets = {8, 0, 0, 0};
 constexpr gfx::Insets kButtonInsets = {6, 16, 6, 16};
 constexpr int kButtonFontSizeDelta = 1;
 
 // Manage-Settings button.
-constexpr SkColor kSettingsButtonBorderColor = gfx::kGoogleGrey300;
 constexpr SkColor kSettingsButtonTextColor = gfx::kGoogleBlue600;
 
 // Grant-Consent button.
-constexpr SkColor kConsentButtonBgColor = gfx::kGoogleBlue600;
 constexpr SkColor kConsentButtonTextColor = gfx::kGoogleGrey200;
 
 // Dogfood button.
@@ -92,12 +91,14 @@ std::unique_ptr<views::Label> CreateLabel(const base::string16& text,
 
 // views::LabelButton with custom line-height, color and font-list for the
 // underlying label.
-class CustomizedLabelButton : public views::LabelButton {
+class CustomizedLabelButton : public views::MdTextButton {
  public:
   CustomizedLabelButton(views::ButtonListener* listener,
                         const base::string16& text,
                         const SkColor color)
-      : LabelButton(listener, text) {
+      : MdTextButton(listener, views::style::CONTEXT_BUTTON_MD) {
+    SetText(text);
+    SetCustomPadding(kButtonInsets);
     SetEnabledTextColors(color);
     label()->SetLineHeight(kLineHeightDip);
     label()->SetFontList(views::Label::GetDefaultFontList()
@@ -123,10 +124,18 @@ UserConsentView::UserConsentView(const gfx::Rect& anchor_view_bounds,
                                  QuickAnswersUiController* ui_controller)
     : anchor_view_bounds_(anchor_view_bounds),
       event_handler_(std::make_unique<QuickAnswersPreTargetHandler>(this)),
-      ui_controller_(ui_controller) {
+      ui_controller_(ui_controller),
+      focus_search_(std::make_unique<QuickAnswersFocusSearch>(
+          this,
+          base::BindRepeating(&UserConsentView::GetFocusableViews,
+                              base::Unretained(this)))) {
   InitLayout();
   InitWidget();
 
+  // Focus should cycle to each of the buttons the view contains and back to it.
+  SetFocusBehavior(FocusBehavior::ALWAYS);
+  views::FocusRing::Install(this);
+
   // Allow tooltips to be shown despite menu-controller owning capture.
   GetWidget()->SetNativeWindowProperty(
       views::TooltipManager::kGroupingPropertyKey,
@@ -156,6 +165,32 @@ gfx::Size UserConsentView::CalculatePreferredSize() const {
   return gfx::Size(width, GetHeightForWidth(width));
 }
 
+void UserConsentView::OnFocus() {
+  // Unless screen-reader mode is enabled, transfer the focus to an actionable
+  // button, otherwise retain to read out its contents.
+  if (!ash::Shell::Get()->accessibility_controller()->spoken_feedback_enabled())
+    settings_button_->RequestFocus();
+}
+
+views::FocusTraversable* UserConsentView::GetPaneFocusTraversable() {
+  return focus_search_.get();
+}
+
+std::vector<views::View*> UserConsentView::GetFocusableViews() {
+  std::vector<views::View*> focusable_views;
+  // The view itself is not included in focus loop, unless screen-reader is on.
+  if (ash::Shell::Get()
+          ->accessibility_controller()
+          ->spoken_feedback_enabled()) {
+    focusable_views.push_back(this);
+  }
+  focusable_views.push_back(settings_button_);
+  focusable_views.push_back(consent_button_);
+  if (dogfood_button_)
+    focusable_views.push_back(dogfood_button_);
+  return focusable_views;
+}
+
 void UserConsentView::ButtonPressed(views::Button* sender,
                                     const ui::Event& event) {
   if (sender == consent_button_) {
@@ -256,11 +291,6 @@ void UserConsentView::InitButtonBar() {
       l10n_util::GetStringUTF16(
           IDS_ASH_QUICK_ANSWERS_USER_CONSENT_VIEW_MANAGE_SETTINGS_BUTTON),
       kSettingsButtonTextColor);
-  settings_button->SetBorder(views::CreatePaddedBorder(
-      views::CreateRoundedRectBorder(kButtonBorderThicknessDip,
-                                     kButtonBorderRadiusDip,
-                                     kSettingsButtonBorderColor),
-      kButtonInsets));
   settings_button_ = button_bar->AddChildView(std::move(settings_button));
 
   // Grant-Consent button.
@@ -269,21 +299,27 @@ void UserConsentView::InitButtonBar() {
       l10n_util::GetStringUTF16(
           IDS_ASH_QUICK_ANSWERS_USER_CONSENT_VIEW_GRANT_CONSENT_BUTTON),
       kConsentButtonTextColor);
-  consent_button->SetBackground(views::CreateRoundedRectBackground(
-      kConsentButtonBgColor, kButtonBorderRadiusDip));
-  consent_button->SetBorder(views::CreateEmptyBorder(kButtonInsets));
+  consent_button->SetProminent(true);
   consent_button_ = button_bar->AddChildView(std::move(consent_button));
 }
 
 void UserConsentView::InitWidget() {
   views::Widget::InitParams params;
   params.activatable = views::Widget::InitParams::Activatable::ACTIVATABLE_NO;
-  params.context = Shell::Get()->GetRootWindowForNewWindows();
   params.shadow_elevation = 2;
   params.shadow_type = views::Widget::InitParams::ShadowType::kDrop;
   params.type = views::Widget::InitParams::TYPE_POPUP;
   params.z_order = ui::ZOrderLevel::kFloatingUIElement;
 
+  // Parent the widget depending on the context.
+  auto* active_menu_controller = views::MenuController::GetActiveInstance();
+  if (active_menu_controller && active_menu_controller->owner()) {
+    params.parent = active_menu_controller->owner()->GetNativeView();
+    params.child = true;
+  } else {
+    params.context = Shell::Get()->GetRootWindowForNewWindows();
+  }
+
   views::Widget* widget = new views::Widget();
   widget->Init(std::move(params));
   widget->SetContentsView(this);
@@ -304,6 +340,7 @@ void UserConsentView::AddDogfoodButton() {
                             kDogfoodButtonColor));
   dogfood_button->SetTooltipText(l10n_util::GetStringUTF16(
       IDS_ASH_QUICK_ANSWERS_DOGFOOD_BUTTON_TOOLTIP_TEXT));
+  dogfood_button->SetFocusForPlatform();
   dogfood_button_ = dogfood_view->AddChildView(std::move(dogfood_button));
 }
 
@@ -317,7 +354,9 @@ void UserConsentView::UpdateWidgetBounds() {
               .y()) {
     y = anchor_view_bounds_.bottom() + kMarginDip;
   }
-  GetWidget()->SetBounds({{x, y}, size});
+  gfx::Rect bounds({x, y}, size);
+  wm::ConvertRectFromScreen(GetWidget()->GetNativeWindow()->parent(), &bounds);
+  GetWidget()->SetBounds(bounds);
 }
 
 }  // namespace quick_answers

ash/quick_answers/ui/user_consent_view.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "ash/quick_answers/ui/quick_answers_focus_search.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
 
@@ -38,6 +39,8 @@ class UserConsentView : public views::View, public views::ButtonListener {
   // views::View:
   const char* GetClassName() const override;
   gfx::Size CalculatePreferredSize() const override;
+  void OnFocus() override;
+  views::FocusTraversable* GetPaneFocusTraversable() override;
 
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
@@ -52,11 +55,16 @@ class UserConsentView : public views::View, public views::ButtonListener {
   void AddDogfoodButton();
   void UpdateWidgetBounds();
 
+  // QuickAnswersFocusSearch::GetFocusableViewsCallback to poll currently
+  // focusable views.
+  std::vector<views::View*> GetFocusableViews();
+
   // Cached bounds of the anchor this view is tied to.
   gfx::Rect anchor_view_bounds_;
 
   std::unique_ptr<QuickAnswersPreTargetHandler> event_handler_;
   QuickAnswersUiController* const ui_controller_;
+  std::unique_ptr<QuickAnswersFocusSearch> focus_search_;
 
   // Owned by view hierarchy.
   views::View* main_view_ = nullptr;