CrOS Shelf: Allow reordering apps with Ctrl + arrow key

Allow arrow key navigation in the overflow bubble as well, so that
app reordering also works and integrates smoothly with keyboard
navigation in there too.

This does not yet support reordering of app across the "overflow
border" (e.g. last app of main shelf with first app of overflow).

Also refactor a simple |IsArrowKeyCode| function in a place where it's
easy to reuse. Some places in app_list could also use this but that
part of the code has a slightly unusual namespace pattern usage and
I didn't want to mess with BUILD.gn and deps too much.

See this "in action" at:

    https://bugs.chromium.org/p/chromium/issues/detail?id=888637#c28

Bug: 888637
Change-Id: Icfa22870bd603093cd4889ffdea10d91c01abbc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1535304Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644648}

ash/BUILD.gn

@@ -339,6 +339,8 @@ component("ash") {
     "keyboard/ash_keyboard_controller.h",
     "keyboard/ash_keyboard_ui.cc",
     "keyboard/ash_keyboard_ui.h",
+    "keyboard/keyboard_util.cc",
+    "keyboard/keyboard_util.h",
     "keyboard/virtual_keyboard_container_layout_manager.cc",
     "keyboard/virtual_keyboard_container_layout_manager.h",
     "keyboard/virtual_keyboard_controller.cc",

ash/accelerators/magnifier_key_scroller.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "ash/accelerators/key_hold_detector.h"
+#include "ash/keyboard/keyboard_util.h"
 #include "ash/magnifier/magnification_controller.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/shell.h"
@@ -41,10 +42,7 @@ std::unique_ptr<ui::EventHandler> MagnifierKeyScroller::CreateHandler() {
 }
 
 bool MagnifierKeyScroller::ShouldProcessEvent(const ui::KeyEvent* event) const {
-  return IsEnabled() && (event->key_code() == ui::VKEY_UP ||
-                         event->key_code() == ui::VKEY_DOWN ||
-                         event->key_code() == ui::VKEY_LEFT ||
-                         event->key_code() == ui::VKEY_RIGHT);
+  return IsEnabled() && ash::keyboard_util::IsArrowKeyCode(event->key_code());
 }
 
 bool MagnifierKeyScroller::IsStartEvent(const ui::KeyEvent* event) const {

ash/events/keyboard_driven_event_rewriter.cc

@@ -4,6 +4,7 @@
 
 #include "ash/events/keyboard_driven_event_rewriter.h"
 
+#include "ash/keyboard/keyboard_util.h"
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ui/chromeos/events/event_rewriter_chromeos.h"
@@ -54,8 +55,7 @@ ui::EventRewriteStatus KeyboardDrivenEventRewriter::Rewrite(
   const ui::KeyEvent& key_event = static_cast<const ui::KeyEvent&>(event);
   ui::KeyboardCode key_code = key_event.key_code();
 
-  if (key_code != ui::VKEY_LEFT && key_code != ui::VKEY_RIGHT &&
-      key_code != ui::VKEY_UP && key_code != ui::VKEY_DOWN &&
+  if (!ash::keyboard_util::IsArrowKeyCode(key_code) &&
       key_code != ui::VKEY_RETURN && key_code != ui::VKEY_F6) {
     return ui::EVENT_REWRITE_CONTINUE;
   }
@@ -65,8 +65,7 @@ ui::EventRewriteStatus KeyboardDrivenEventRewriter::Rewrite(
       key_event.code(), key_event.GetDomKey(), key_event.key_code()};
 
   if (arrow_to_tab_rewriting_enabled_) {
-    if (key_code == ui::VKEY_LEFT || key_code == ui::VKEY_RIGHT ||
-        key_code == ui::VKEY_UP || key_code == ui::VKEY_DOWN) {
+    if (ash::keyboard_util::IsArrowKeyCode(key_code)) {
       const ui::KeyEvent tab_event(ui::ET_KEY_PRESSED, ui::VKEY_TAB,
                                    ui::EF_NONE);
       state.code = tab_event.code();

ash/keyboard/keyboard_util.cc

+// Copyright (c) 2019 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/keyboard/keyboard_util.h"
+
+namespace ash {
+
+namespace keyboard_util {
+
+bool IsArrowKeyCode(const ui::KeyboardCode key_code) {
+  return key_code == ui::VKEY_DOWN || key_code == ui::VKEY_RIGHT ||
+         key_code == ui::VKEY_LEFT || key_code == ui::VKEY_UP;
+}
+
+}  // namespace keyboard_util
+
+}  // namespace ash

ash/keyboard/keyboard_util.h

+// Copyright (c) 2019 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_KEYBOARD_KEYBOARD_UTIL_H_
+#define ASH_KEYBOARD_KEYBOARD_UTIL_H_
+
+#include "ash/ash_export.h"
+
+#include "ui/events/keycodes/keyboard_codes.h"
+
+namespace ash {
+
+namespace keyboard_util {
+
+// Returns whether the given key code corresponds to one of the 4 arrow keys.
+ASH_EXPORT bool IsArrowKeyCode(const ui::KeyboardCode key_code);
+
+}  // namespace keyboard_util
+
+}  // namespace ash
+
+#endif  // ASH_KEYBOARD_KEYBOARD_UTIL_H_

ash/public/cpp/shelf_types.cc

@@ -23,6 +23,16 @@ bool IsValidShelfItemType(int64_t type) {
          type == TYPE_UNDEFINED;
 }
 
+bool SamePinState(ShelfItemType a, ShelfItemType b) {
+  if ((a != TYPE_PINNED_APP && a != TYPE_APP && a != TYPE_BROWSER_SHORTCUT) ||
+      (b != TYPE_PINNED_APP && b != TYPE_APP && b != TYPE_BROWSER_SHORTCUT)) {
+    return false;
+  }
+  const bool a_unpinned = (a == TYPE_APP);
+  const bool b_unpinned = (b == TYPE_APP);
+  return a_unpinned == b_unpinned;
+}
+
 ShelfID::ShelfID(const std::string& app_id, const std::string& launch_id)
     : app_id(app_id), launch_id(launch_id) {
   DCHECK(launch_id.empty() || !app_id.empty()) << "launch ids require app ids.";

ash/public/cpp/shelf_types.h

@@ -141,6 +141,11 @@ enum ShelfItemType {
 // Returns true if |type| is a valid ShelfItemType.
 ASH_PUBLIC_EXPORT bool IsValidShelfItemType(int64_t type);
 
+// Returns true if types |a| and |b| have the same pin state, i.e. if they
+// are both pinned apps (or a browser shortcut which is always pinned) or both
+// unpinned apps. Returns false if either a or b aren't an app type.
+ASH_PUBLIC_EXPORT bool SamePinState(ShelfItemType a, ShelfItemType b);
+
 // Represents the status of applications in the shelf.
 enum ShelfItemStatus {
   // A closed shelf item, i.e. has no live instance.

ash/shelf/overflow_bubble.cc

@@ -41,10 +41,12 @@ void OverflowBubble::Show(OverflowButton* overflow_button,
       shelf_view->shelf_widget()->GetShelfBackgroundColor());
   overflow_button_ = overflow_button;
 
-  TrayBackgroundView::InitializeBubbleAnimations(bubble_->GetWidget());
-  bubble_->GetWidget()->AddObserver(this);
-  bubble_->GetWidget()->Show();
-  Shell::Get()->focus_cycler()->AddWidget(bubble_->GetWidget());
+  views::Widget* widget = bubble_->GetWidget();
+  TrayBackgroundView::InitializeBubbleAnimations(widget);
+  widget->AddObserver(this);
+  widget->Show();
+  widget->GetFocusManager()->set_arrow_key_traversal_enabled_for_widget(true);
+  Shell::Get()->focus_cycler()->AddWidget(widget);
 }
 
 void OverflowBubble::Hide() {

ash/shelf/shelf_button.cc

@@ -29,6 +29,10 @@ ShelfButton::~ShelfButton() = default;
 ////////////////////////////////////////////////////////////////////////////////
 // views::View
 
+const char* ShelfButton::GetClassName() const {
+  return "ash/ShelfButton";
+}
+
 bool ShelfButton::OnMousePressed(const ui::MouseEvent& event) {
   Button::OnMousePressed(event);
   shelf_view_->PointerPressedOnButton(this, ShelfView::MOUSE, event);
@@ -66,6 +70,16 @@ void ShelfButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   node_data->SetName(title.empty() ? GetAccessibleName() : title);
 }
 
+void ShelfButton::OnFocus() {
+  shelf_view_->set_focused_button(this);
+  Button::OnFocus();
+}
+
+void ShelfButton::OnBlur() {
+  shelf_view_->set_focused_button(nullptr);
+  Button::OnBlur();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // views::Button
 
@@ -89,8 +103,4 @@ std::unique_ptr<views::InkDrop> ShelfButton::CreateInkDrop() {
   return std::move(ink_drop);
 }
 
-const char* ShelfButton::GetClassName() const {
-  return "ash/ShelfButton";
-}
-
 }  // namespace ash

ash/shelf/shelf_button.h

@@ -19,12 +19,15 @@ class ASH_EXPORT ShelfButton : public views::Button {
   ~ShelfButton() override;
 
   // views::View
+  const char* GetClassName() const override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
   void OnMouseCaptureLost() override;
   bool OnMouseDragged(const ui::MouseEvent& event) override;
   void AboutToRequestFocusFromTabTraversal(bool reverse) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+  void OnFocus() override;
+  void OnBlur() override;
 
  protected:
   ShelfView* shelf_view() { return shelf_view_; }
@@ -33,7 +36,6 @@ class ASH_EXPORT ShelfButton : public views::Button {
   void NotifyClick(const ui::Event& event) override;
   bool ShouldEnterPushedState(const ui::Event& event) override;
   std::unique_ptr<views::InkDrop> CreateInkDrop() override;
-  const char* GetClassName() const override;
 
  private:
   // The shelf view hosting this button.

ash/shelf/shelf_view.cc

@@ -9,11 +9,13 @@
 
 #include "ash/drag_drop/drag_image_view.h"
 #include "ash/focus_cycler.h"
+#include "ash/keyboard/keyboard_util.h"
 #include "ash/metrics/user_metrics_recorder.h"
 #include "ash/public/cpp/ash_constants.h"
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/shelf_item_delegate.h"
 #include "ash/public/cpp/shelf_model.h"
+#include "ash/public/cpp/shelf_types.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/scoped_root_window_for_new_windows.h"
 #include "ash/screen_util.h"
@@ -597,6 +599,17 @@ void ShelfView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
     overflow_bubble_->Hide();
 }
 
+bool ShelfView::OnKeyPressed(const ui::KeyEvent& event) {
+  if (event.IsControlDown() &&
+      ash::keyboard_util::IsArrowKeyCode(event.key_code())) {
+    bool swap_with_next = (event.key_code() == ui::VKEY_DOWN ||
+                           event.key_code() == ui::VKEY_RIGHT);
+    SwapButtons(focused_button_, swap_with_next);
+    return true;
+  }
+  return views::View::OnKeyPressed(event);
+}
+
 views::FocusTraversable* ShelfView::GetPaneFocusTraversable() {
   return this;
 }
@@ -1015,6 +1028,45 @@ bool ShelfView::ShouldEventActivateButton(View* view, const ui::Event& event) {
   return !IsRepostEvent(event) || last_pressed_index_ != index;
 }
 
+void ShelfView::SwapButtons(ShelfButton* button_to_swap, bool with_next) {
+  if (!button_to_swap)
+    return;
+  // We don't allow reordering of buttons that aren't app buttons.
+  if (button_to_swap->GetClassName() != ShelfAppButton::kViewClassName)
+    return;
+
+  // Find the index of the button to swap in the view model.
+  int src_index = -1;
+  for (int i = 0; i < view_model_->view_size(); ++i) {
+    View* view = view_model_->view_at(i);
+    if (view == button_to_swap) {
+      src_index = i;
+      break;
+    }
+  }
+
+  const int target_index = with_next ? src_index + 1 : src_index - 1;
+  const int first_swappable_index =
+      std::max(first_visible_index_, kAppListButtonIndex + 1);
+  const int last_swappable_index = last_visible_index_;
+  if (src_index == -1 || (target_index > last_swappable_index) ||
+      (target_index < first_swappable_index)) {
+    return;
+  }
+
+  // Only allow swapping two pinned apps or two unpinned apps.
+  if (!SamePinState(model_->items()[src_index].type,
+                    model_->items()[target_index].type)) {
+    return;
+  }
+
+  // Swapping items in the model is sufficient, everything will then be
+  // reflected in the views.
+  model_->Move(src_index, target_index);
+  AnimateToIdealBounds();
+  // TODO(manucornet): Announce the swap to screen readers.
+}
+
 void ShelfView::PointerPressedOnButton(views::View* view,
                                        Pointer pointer,
                                        const ui::LocatedEvent& event) {

ash/shelf/shelf_view.h

@@ -151,6 +151,8 @@ class ASH_EXPORT ShelfView : public views::View,
     owner_overflow_bubble_ = owner;
   }
 
+  void set_focused_button(ShelfButton* focused) { focused_button_ = focused; }
+
   AppListButton* GetAppListButton() const;
   BackButton* GetBackButton() const;
   OverflowButton* GetOverflowButton() const;
@@ -180,6 +182,8 @@ class ASH_EXPORT ShelfView : public views::View,
   gfx::Size CalculatePreferredSize() const override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
   FocusTraversable* GetPaneFocusTraversable() override;
+  bool OnKeyPressed(const ui::KeyEvent& event) override;
+
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   View* GetTooltipHandlerForPoint(const gfx::Point& point) override;
 
@@ -230,6 +234,10 @@ class ASH_EXPORT ShelfView : public views::View,
   // Used to determine whether a pending ink drop should be shown or not.
   bool ShouldEventActivateButton(views::View* view, const ui::Event& event);
 
+  // Swaps the given button with the next one if |with_next| is true, or with
+  // the previous one if |with_next| is false.
+  void SwapButtons(ShelfButton* button_to_swap, bool with_next);
+
   // The shelf buttons use the Pointer interface to enable item reordering.
   enum Pointer { NONE, DRAG_AND_DROP, MOUSE, TOUCH };
   void PointerPressedOnButton(views::View* view,
@@ -604,6 +612,10 @@ class ASH_EXPORT ShelfView : public views::View,
   // The timestamp of the event which closed the last menu - or 0.
   base::TimeTicks closing_event_time_;
 
+  // The button that is currently focused for keyboard navigation, or null
+  // if nothing is focused.
+  ShelfButton* focused_button_ = nullptr;
+
   // True if a drag and drop operation created/pinned the item in the launcher
   // and it needs to be deleted/unpinned again if the operation gets cancelled.
   bool drag_and_drop_item_pinned_ = false;