Autoclick menu supports RTL languages.

Autoclick menu icons should not be mirrored in RTL languages because
they have a directionality: The click types correspond to mouse buttons
left and right, and the positions correspond to corners of the screen.

Also adds a default screen position. This will be the same as where
the volume and brightness sliders show up on the screen in LTR/RTL
languages, and change with language direction. However, as soon as the
user explicitly picks another position using the autoclick menu
position button, the position will be fixed no matter if the user
changes their system language direction.
See go/chromeos-dwell-menu-design under the "Positioning" subheading.

Bug: 958998
Change-Id: I28b3bf44b1f9081d6709e0b8255913f69ccac3c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594654
Commit-Queue: Katie Dektar <katie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657375}

ash/public/cpp/ash_constants.h

@@ -5,7 +5,7 @@
 #ifndef ASH_PUBLIC_CPP_ASH_CONSTANTS_H_
 #define ASH_PUBLIC_CPP_ASH_CONSTANTS_H_
 
-#include "ash/public/interfaces/accessibility_controller.mojom.h"
+#include "ash/public/interfaces/accessibility_controller_enums.mojom.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/color_palette.h"
 
@@ -49,7 +49,7 @@ constexpr int kDefaultAutoclickMovementThreshold = 20;
 
 // The default automatic click menu position.
 constexpr mojom::AutoclickMenuPosition kDefaultAutoclickMenuPosition =
-    mojom::AutoclickMenuPosition::kBottomRight;
+    mojom::AutoclickMenuPosition::kSystemDefault;
 
 // The default frame color.
 constexpr SkColor kDefaultFrameColor = SkColorSetRGB(0xFD, 0xFE, 0xFF);

ash/public/interfaces/accessibility_controller_enums.mojom

@@ -117,4 +117,9 @@ enum AutoclickMenuPosition {
 
   // The top right of the screen.
   kTopRight,
+
+  // The default position. This will be either the bottom right in LTR languages
+  // or the bottom right in RTL languages. Once the user explicitly picks
+  // a position it will no longer change with language direction.
+  kSystemDefault,
 };

ash/system/accessibility/autoclick_menu_bubble_controller.cc

@@ -25,11 +25,14 @@ namespace {
 const int kAutoclickMenuWidth = 321;
 }  // namespace
 
-AutoclickMenuBubbleController::AutoclickMenuBubbleController() {}
+AutoclickMenuBubbleController::AutoclickMenuBubbleController() {
+  Shell::Get()->locale_update_controller()->AddObserver(this);
+}
 
 AutoclickMenuBubbleController::~AutoclickMenuBubbleController() {
   if (bubble_widget_ && !bubble_widget_->IsClosed())
     bubble_widget_->CloseNow();
+  Shell::Get()->locale_update_controller()->RemoveObserver(this);
 }
 
 void AutoclickMenuBubbleController::SetEventType(
@@ -40,15 +43,24 @@ void AutoclickMenuBubbleController::SetEventType(
 }
 
 void AutoclickMenuBubbleController::SetPosition(
-    mojom::AutoclickMenuPosition position) {
+    mojom::AutoclickMenuPosition new_position) {
   if (!menu_view_ || !bubble_view_ || !bubble_widget_)
     return;
 
-  // No need to update the menu view's UX if it's already in the right
-  // AutoclickMenuPosition.
-  if (position_ != position) {
-    menu_view_->UpdatePosition(position);
-    position_ = position;
+  // Update the menu view's UX if the position has changed, or if it's not the
+  // default position (because that can change with language direction).
+  if (position_ != new_position ||
+      new_position == mojom::AutoclickMenuPosition::kSystemDefault) {
+    menu_view_->UpdatePosition(new_position);
+  }
+  position_ = new_position;
+
+  // If this is the default system position, pick the position based on the
+  // language direction.
+  if (new_position == mojom::AutoclickMenuPosition::kSystemDefault) {
+    new_position = base::i18n::IsRTL()
+                       ? mojom::AutoclickMenuPosition::kBottomLeft
+                       : mojom::AutoclickMenuPosition::kBottomRight;
   }
 
   // Calculates the ideal bounds.
@@ -58,7 +70,7 @@ void AutoclickMenuBubbleController::SetPosition(
   gfx::Rect work_area =
       WorkAreaInsets::ForWindow(window)->user_work_area_bounds();
   gfx::Rect new_bounds;
-  switch (position) {
+  switch (new_position) {
     case mojom::AutoclickMenuPosition::kBottomRight:
       new_bounds = gfx::Rect(work_area.right(), work_area.bottom(), 0, 0);
       break;
@@ -73,6 +85,8 @@ void AutoclickMenuBubbleController::SetPosition(
       // Setting the top to 1 instead of 0 so that the view is drawn on screen.
       new_bounds = gfx::Rect(work_area.right(), 1, 0, 0);
       break;
+    case mojom::AutoclickMenuPosition::kSystemDefault:
+      return;
   }
 
   // Update the preferred bounds based on other system windows.
@@ -172,4 +186,11 @@ void AutoclickMenuBubbleController::BubbleViewDestroyed() {
   menu_view_ = nullptr;
 }
 
+void AutoclickMenuBubbleController::OnLocaleChanged() {
+  // Layout update is needed when language changes between LTR and RTL, if the
+  // position is the system default.
+  if (position_ == mojom::AutoclickMenuPosition::kSystemDefault)
+    SetPosition(position_);
+}
+
 }  // namespace ash

ash/system/accessibility/autoclick_menu_bubble_controller.h

@@ -7,12 +7,14 @@
 
 #include "ash/public/cpp/ash_constants.h"
 #include "ash/system/accessibility/autoclick_menu_view.h"
+#include "ash/system/locale/locale_update_controller.h"
 #include "ash/system/tray/tray_bubble_view.h"
 
 namespace ash {
 
 // Manages the bubble which contains an AutoclickMenuView.
-class AutoclickMenuBubbleController : public TrayBubbleView::Delegate {
+class AutoclickMenuBubbleController : public TrayBubbleView::Delegate,
+                                      public LocaleChangeObserver {
  public:
   AutoclickMenuBubbleController();
   ~AutoclickMenuBubbleController() override;
@@ -38,6 +40,9 @@ class AutoclickMenuBubbleController : public TrayBubbleView::Delegate {
   // TrayBubbleView::Delegate:
   void BubbleViewDestroyed() override;
 
+  // LocaleChangeObserver:
+  void OnLocaleChanged() override;
+
  private:
   friend class AutoclickMenuBubbleControllerTest;
   friend class AutoclickTest;

ash/system/accessibility/autoclick_menu_bubble_controller_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "ash/accessibility/accessibility_controller.h"
 #include "ash/autoclick/autoclick_controller.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "base/command_line.h"
@@ -191,4 +192,26 @@ TEST_F(AutoclickMenuBubbleControllerTest, CanChangePosition) {
   }
 }
 
+TEST_F(AutoclickMenuBubbleControllerTest, DefaultChangesWithTextDirection) {
+  AccessibilityController* controller =
+      Shell::Get()->accessibility_controller();
+  gfx::Rect window_bounds = Shell::GetPrimaryRootWindow()->bounds();
+
+  // RTL should position the menu on the bottom left.
+  base::i18n::SetRTLForTesting(true);
+  // Force a layout.
+  controller->UpdateAutoclickMenuBoundsIfNeeded();
+  EXPECT_LT(
+      GetMenuViewBounds().ManhattanDistanceToPoint(window_bounds.bottom_left()),
+      kMenuViewBoundsBuffer);
+
+  // LTR should position the menu on the bottom right.
+  base::i18n::SetRTLForTesting(false);
+  // Force a layout.
+  controller->UpdateAutoclickMenuBoundsIfNeeded();
+  EXPECT_LT(GetMenuViewBounds().ManhattanDistanceToPoint(
+                window_bounds.bottom_right()),
+            kMenuViewBoundsBuffer);
+}
+
 }  // namespace ash

ash/system/accessibility/autoclick_menu_view.cc

@@ -47,6 +47,7 @@ class AutoclickMenuButton : public TopShortcutButton {
       : TopShortcutButton(listener, accessible_name_id),
         icon_(&icon),
         size_(size) {
+    EnableCanvasFlippingForRTLUI(false);
     SetPreferredSize(gfx::Size(size_, size_));
     UpdateImage();
   }
@@ -234,6 +235,11 @@ void AutoclickMenuView::UpdatePosition(mojom::AutoclickMenuPosition position) {
     case mojom::AutoclickMenuPosition::kTopRight:
       position_button_->SetVectorIcon(kAutoclickPositionTopRightIcon);
       return;
+    case mojom::AutoclickMenuPosition::kSystemDefault:
+      position_button_->SetVectorIcon(base::i18n::IsRTL()
+                                          ? kAutoclickPositionBottomLeftIcon
+                                          : kAutoclickPositionBottomRightIcon);
+      return;
   }
 }
 
@@ -256,6 +262,11 @@ void AutoclickMenuView::ButtonPressed(views::Button* sender,
       case mojom::AutoclickMenuPosition::kTopRight:
         new_position = mojom::AutoclickMenuPosition::kBottomRight;
         break;
+      case mojom::AutoclickMenuPosition::kSystemDefault:
+        new_position = base::i18n::IsRTL()
+                           ? mojom::AutoclickMenuPosition::kTopLeft
+                           : mojom::AutoclickMenuPosition::kBottomLeft;
+        break;
     }
     Shell::Get()->accessibility_controller()->SetAutoclickMenuPosition(
         new_position);