[Switch Access] Add back button focus ring

The Switch Access UI specification (see bug) indicates that the back
button should have a perfectly round focus ring exactly around the back
button. The general accessibility focus ring infrastructure is not set
up to be round, or to draw exactly adjacent to a UI element, so the
preferred approach is to modify the back button's appearance to include
the focus ring, when the node should be focused.

AX-Relnotes: n/a
Bug: 973719
Change-Id: I80aaa036d13192ce8b60fb2abaeb573a3f8ac5a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2181428
Commit-Queue: Anastasia Helfinstein <anastasi@google.com>
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766483}

ash/system/accessibility/switch_access_back_button_bubble_controller.cc

@@ -15,29 +15,6 @@
 
 namespace ash {
 
-namespace {
-
-constexpr int kBackButtonRadiusDip = 18;
-constexpr int kBackButtonDiameterDip = 2 * kBackButtonRadiusDip;
-
-// The back button should display above and to the right of the anchor rect
-// provided. Because the TrayBubbleView defaults to showing the right edges
-// lining up (rather than appearing off to the side) we'll add the width of the
-// button to the anchor rect's width.
-gfx::Rect AdjustAnchorRect(const gfx::Rect& anchor) {
-  gfx::Rect adjusted_anchor(anchor.x(), anchor.y(),
-                            anchor.width() + kBackButtonDiameterDip,
-                            anchor.height());
-
-  // Don't allow our new rect to extend past the edge of the display.
-  display::Display display =
-      display::Screen::GetScreen()->GetDisplayMatching(anchor);
-  adjusted_anchor.Intersect(display.bounds());
-  return adjusted_anchor;
-}
-
-}  // namespace
-
 SwitchAccessBackButtonBubbleController::
     SwitchAccessBackButtonBubbleController() {}
 
@@ -48,8 +25,12 @@ SwitchAccessBackButtonBubbleController::
 }
 
 void SwitchAccessBackButtonBubbleController::ShowBackButton(
-    const gfx::Rect& anchor) {
+    const gfx::Rect& anchor,
+    bool showFocusRing) {
   if (!widget_) {
+    back_button_view_ = new SwitchAccessBackButtonView();
+    back_button_view_->SetBackground(UnifiedSystemTrayView::CreateBackground());
+
     TrayBubbleView::InitParams init_params;
     init_params.delegate = this;
     // Anchor within the overlay container.
@@ -59,17 +40,9 @@ void SwitchAccessBackButtonBubbleController::ShowBackButton(
     init_params.anchor_mode = TrayBubbleView::AnchorMode::kRect;
     init_params.is_anchored_to_status_area = false;
     init_params.has_shadow = false;
-
-    // The back button is a circle, so the preferred width and height are the
-    // diameter, and the corner radius is the circle radius.
-    init_params.corner_radius = kBackButtonRadiusDip;
-    init_params.preferred_width = kBackButtonDiameterDip;
-    init_params.max_height = kBackButtonDiameterDip;
+    init_params.preferred_width = back_button_view_->size().width();
 
     bubble_view_ = new TrayBubbleView(init_params);
-
-    back_button_view_ = new SwitchAccessBackButtonView(kBackButtonDiameterDip);
-    back_button_view_->SetBackground(UnifiedSystemTrayView::CreateBackground());
     bubble_view_->AddChildView(back_button_view_);
     bubble_view_->set_color(SK_ColorTRANSPARENT);
     bubble_view_->layer()->SetFillsBoundsOpaquely(false);
@@ -80,10 +53,15 @@ void SwitchAccessBackButtonBubbleController::ShowBackButton(
   }
 
   DCHECK(bubble_view_);
+  back_button_view_->SetFocusRing(showFocusRing);
   bubble_view_->ChangeAnchorRect(AdjustAnchorRect(anchor));
   widget_->Show();
 }
 
+void SwitchAccessBackButtonBubbleController::HideFocusRing() {
+  back_button_view_->SetFocusRing(false);
+}
+
 void SwitchAccessBackButtonBubbleController::Hide() {
   if (widget_)
     widget_->Hide();
@@ -95,4 +73,23 @@ void SwitchAccessBackButtonBubbleController::BubbleViewDestroyed() {
   widget_ = nullptr;
 }
 
+// The back button should display above and to the right of the anchor rect
+// provided. Because the TrayBubbleView defaults to showing the right edges
+// lining up (rather than appearing off to the side) we'll add the width of the
+// button to the anchor rect's width.
+gfx::Rect SwitchAccessBackButtonBubbleController::AdjustAnchorRect(
+    const gfx::Rect& anchor) {
+  DCHECK(back_button_view_);
+
+  gfx::Rect adjusted_anchor(anchor.x(), anchor.y(),
+                            anchor.width() + back_button_view_->size().width(),
+                            anchor.height());
+
+  // Don't allow our new rect to extend past the edge of the display.
+  display::Display display =
+      display::Screen::GetScreen()->GetDisplayMatching(anchor);
+  adjusted_anchor.Intersect(display.bounds());
+  return adjusted_anchor;
+}
+
 }  // namespace ash

ash/system/accessibility/switch_access_back_button_bubble_controller.h

@@ -23,7 +23,8 @@ class ASH_EXPORT SwitchAccessBackButtonBubbleController
   SwitchAccessBackButtonBubbleController& operator=(
       const SwitchAccessBackButtonBubbleController&) = delete;
 
-  void ShowBackButton(const gfx::Rect& anchor);
+  void ShowBackButton(const gfx::Rect& anchor, bool showFocusRing);
+  void HideFocusRing();
   void Hide();
 
   // TrayBubbleView::Delegate:
@@ -32,6 +33,8 @@ class ASH_EXPORT SwitchAccessBackButtonBubbleController
  private:
   friend class SwitchAccessMenuBubbleControllerTest;
 
+  gfx::Rect AdjustAnchorRect(const gfx::Rect& anchor);
+
   // Owned by views hierarchy.
   SwitchAccessBackButtonView* back_button_view_ = nullptr;
   TrayBubbleView* bubble_view_ = nullptr;

ash/system/accessibility/switch_access_back_button_view.cc

@@ -21,21 +21,42 @@
 namespace ash {
 
 namespace {
+// The width of a single color focus ring, in density-independent pixels.
+constexpr int kFocusRingSingleColorWidthDp = 2;
+// Additional buffer needed to prevent clipping at the focus ring's edges.
+constexpr int kFocusRingBufferDp = 1;
+
 constexpr char kUniqueId[] = "switch_access_back_button";
+constexpr int kRadiusDp = 18;
 }  // namespace
 
-SwitchAccessBackButtonView::SwitchAccessBackButtonView(int diameter)
-    : diameter_(diameter),
-      back_button_(
+SwitchAccessBackButtonView::SwitchAccessBackButtonView()
+    : back_button_(
           new FloatingMenuButton(this,
                                  kSwitchAccessBackIcon,
                                  IDS_ASH_SWITCH_ACCESS_BACK_BUTTON_DESCRIPTION,
                                  /*flip_for_rtl=*/false,
-                                 diameter)) {
-  std::unique_ptr<views::BoxLayout> layout = std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kHorizontal, gfx::Insets());
-  SetLayoutManager(std::move(layout));
+                                 2 * kRadiusDp)) {
+  views::BoxLayout* layout =
+      SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::Orientation::kHorizontal, gfx::Insets()));
+  layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kCenter);
   AddChildView(back_button_);
+
+  // Calculate the side length of the bounding box, with room for the two-color
+  // focus ring on either side.
+  int focus_ring_width_per_side =
+      2 * kFocusRingSingleColorWidthDp + kFocusRingBufferDp;
+  int side_length = 2 * (kRadiusDp + focus_ring_width_per_side);
+  gfx::Size size(side_length, side_length);
+  SetSize(size);
+}
+
+void SwitchAccessBackButtonView::SetFocusRing(bool should_show) {
+  if (show_focus_ring_ == should_show)
+    return;
+  show_focus_ring_ = should_show;
+  SchedulePaint();
 }
 
 void SwitchAccessBackButtonView::ButtonPressed(views::Button* sender,
@@ -50,6 +71,10 @@ void SwitchAccessBackButtonView::GetAccessibleNodeData(
   node_data->html_attributes.push_back(std::make_pair("id", kUniqueId));
 }
 
+int SwitchAccessBackButtonView::GetHeightForWidth(int w) const {
+  return w;
+}
+
 const char* SwitchAccessBackButtonView::GetClassName() const {
   return "SwitchAccessBackButtonView";
 }
@@ -60,7 +85,20 @@ void SwitchAccessBackButtonView::OnPaint(gfx::Canvas* canvas) {
   flags.setAntiAlias(true);
   flags.setColor(gfx::kGoogleGrey800);
   flags.setStyle(cc::PaintFlags::kFill_Style);
-  canvas->DrawCircle(gfx::PointF(rect.CenterPoint()), diameter_ / 2.f, flags);
+  canvas->DrawCircle(gfx::PointF(rect.CenterPoint()), kRadiusDp, flags);
+
+  if (!show_focus_ring_)
+    return;
+
+  flags.setColor(gfx::kGoogleBlue300);
+  flags.setStyle(cc::PaintFlags::kStroke_Style);
+  flags.setStrokeWidth(kFocusRingSingleColorWidthDp);
+  canvas->DrawCircle(gfx::PointF(rect.CenterPoint()),
+                     kRadiusDp + kFocusRingSingleColorWidthDp, flags);
+
+  flags.setColor(SK_ColorBLACK);
+  canvas->DrawCircle(gfx::PointF(rect.CenterPoint()),
+                     kRadiusDp + (2 * kFocusRingSingleColorWidthDp), flags);
 }
 
 }  // namespace ash

ash/system/accessibility/switch_access_back_button_view.h

@@ -16,23 +16,26 @@ class FloatingMenuButton;
 class SwitchAccessBackButtonView : public views::View,
                                    public views::ButtonListener {
  public:
-  explicit SwitchAccessBackButtonView(int button_size);
+  explicit SwitchAccessBackButtonView();
   ~SwitchAccessBackButtonView() override = default;
 
   SwitchAccessBackButtonView(const SwitchAccessBackButtonView&) = delete;
   SwitchAccessBackButtonView& operator=(const SwitchAccessBackButtonView&) =
       delete;
 
+  void SetFocusRing(bool should_show);
+
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // views::View:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+  int GetHeightForWidth(int w) const override;
   const char* GetClassName() const override;
   void OnPaint(gfx::Canvas* canvas) override;
 
  private:
-  int diameter_;
+  bool show_focus_ring_;
 
   // Owned by views hierarchy.
   FloatingMenuButton* back_button_;

ash/system/accessibility/switch_access_menu_bubble_controller.cc

@@ -25,8 +25,7 @@ SwitchAccessMenuBubbleController::~SwitchAccessMenuBubbleController() {
 }
 
 void SwitchAccessMenuBubbleController::ShowBackButton(const gfx::Rect& anchor) {
-  HideMenuBubble();
-  back_button_controller_->ShowBackButton(anchor);
+  back_button_controller_->ShowBackButton(anchor, /*showFocusRing=*/true);
 }
 
 void SwitchAccessMenuBubbleController::ShowMenu(
@@ -80,11 +79,15 @@ void SwitchAccessMenuBubbleController::ShowMenu(
 
   widget_->SetBounds(resting_bounds);
   widget_->Show();
-  back_button_controller_->ShowBackButton(resting_bounds);
+  back_button_controller_->ShowBackButton(resting_bounds,
+                                          /*showFocusRing=*/false);
 }
 
 void SwitchAccessMenuBubbleController::HideBackButton() {
-  back_button_controller_->Hide();
+  if (menu_view_ && menu_view_->IsDrawn())
+    back_button_controller_->HideFocusRing();
+  else
+    back_button_controller_->Hide();
 }
 
 void SwitchAccessMenuBubbleController::HideMenuBubble() {

ash/system/accessibility/switch_access_menu_bubble_controller_unittest.cc

@@ -78,8 +78,8 @@ TEST_F(SwitchAccessMenuBubbleControllerTest, ShowBackButton) {
   GetBubbleController()->ShowBackButton(anchor_rect);
 
   gfx::Rect bounds = GetBackButtonBounds();
-  EXPECT_EQ(bounds.width(), 36);
-  EXPECT_EQ(bounds.height(), 36);
+  EXPECT_GT(bounds.width(), 36);
+  EXPECT_GT(bounds.height(), 36);
 }
 
 TEST_F(SwitchAccessMenuBubbleControllerTest, ShowMenu) {