app_list: Improve keyboard navigation.

- Left arrow key on the first item on the first page should be a no-op;
- Arrow keys select the first item on the current page by default;
- Space key should not leave item in pressed state;

BUG=171321
TEST=Covered by new test cases.

R=sky@chromium.org


Review URL: https://chromiumcodereview.appspot.com/12035108

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@179006 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 61 insertions, 16 deletions

diff --git a/ui/app_list/views/app_list_item_view.cc b/ui/app_list/views/app_list_item_view.cc
index 3a004f2..3f3de67 100644
--- a/ui/app_list/views/app_list_item_view.cc
+++ b/ui/app_list/views/app_list_item_view.cc
@@ -270,6 +270,17 @@ bool AppListItemView::OnMousePressed(const ui::MouseEvent& event) {
   return true;
 }
 
+bool AppListItemView::OnKeyPressed(const ui::KeyEvent& event) {
+  // Disable space key to press the button. The keyboard events received
+  // by this view are forwarded from a Textfield (SearchBoxView) and key
+  // released events are not forwarded. This leaves the button in pressed
+  // state.
+  if (event.key_code() == ui::VKEY_SPACE)
+    return false;
+
+  return CustomButton::OnKeyPressed(event);
+}
+
 void AppListItemView::OnMouseReleased(const ui::MouseEvent& event) {
   CustomButton::OnMouseReleased(event);
   apps_grid_view_->EndDrag(false);
diff --git a/ui/app_list/views/app_list_item_view.h b/ui/app_list/views/app_list_item_view.h
index e0c3fbd..7cabc5f 100644
--- a/ui/app_list/views/app_list_item_view.h
+++ b/ui/app_list/views/app_list_item_view.h
@@ -80,6 +80,7 @@ class APP_LIST_EXPORT AppListItemView : public views::CustomButton,
   virtual bool ShouldEnterPushedState(const ui::Event& event) OVERRIDE;
 
   // views::View overrides:
+  virtual bool OnKeyPressed(const ui::KeyEvent& event) OVERRIDE;
   virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnMouseCaptureLost() OVERRIDE;
diff --git a/ui/app_list/views/apps_grid_view.cc b/ui/app_list/views/apps_grid_view.cc
index d882e31..a749981 100644
--- a/ui/app_list/views/apps_grid_view.cc
+++ b/ui/app_list/views/apps_grid_view.cc
@@ -389,8 +389,8 @@ void AppsGridView::SetSelectedItemByIndex(const Index& index) {
   if (selected_view_)
     selected_view_->SchedulePaint();
 
+  EnsureViewVisible(new_selection);
   selected_view_ = new_selection;
-  EnsureViewVisible(selected_view_);
   selected_view_->SchedulePaint();
   if (GetWidget()) {
     GetWidget()->NotifyAccessibilityEvent(
@@ -423,16 +423,18 @@ views::View* AppsGridView::GetViewAtIndex(const Index& index) const {
 
 void AppsGridView::MoveSelected(int page_delta, int slot_delta) {
   if (!selected_view_)
-    return SetSelectedItemByIndex(Index(0, 0));
+    return SetSelectedItemByIndex(Index(pagination_model_->selected_page(), 0));
 
   const Index& selected = GetIndexOfView(selected_view_);
   int target_slot = selected.slot + slot_delta;
   if (target_slot < 0) {
     page_delta += (target_slot + 1) / tiles_per_page() - 1;
-    target_slot = tiles_per_page() + (target_slot + 1) % tiles_per_page() - 1;
-  } else if (target_slot > tiles_per_page()) {
+    if (selected.page > 0)
+      target_slot = tiles_per_page() + (target_slot + 1) % tiles_per_page() - 1;
+  } else if (target_slot >= tiles_per_page()) {
     page_delta += target_slot / tiles_per_page();
-    target_slot %= tiles_per_page();
+    if (selected.page < pagination_model_->total_pages() - 1)
+      target_slot %= tiles_per_page();
   }
 
   int target_page = std::min(pagination_model_->total_pages() - 1,
@@ -771,6 +773,7 @@ void AppsGridView::SelectedPageChanged(int old_selected, int new_selected) {
     Layout();
     MaybeStartPageFlipTimer(last_drag_point_);
   } else {
+    ClearSelectedView(selected_view_);
     Layout();
   }
 }
diff --git a/ui/app_list/views/apps_grid_view_unittest.cc b/ui/app_list/views/apps_grid_view_unittest.cc
index 5991107..27994bf 100644
--- a/ui/app_list/views/apps_grid_view_unittest.cc
+++ b/ui/app_list/views/apps_grid_view_unittest.cc
@@ -372,23 +372,53 @@ TEST_F(AppsGridViewTest, SimultaneousDrag) {
 }
 
 TEST_F(AppsGridViewTest, HighlightWithKeyboard) {
-  const int kTotalitems = 2;
-  PopulateApps(kTotalitems);
-  apps_grid_view_->SetSelectedView(GetItemViewAt(0));
+  const int kPages = 2;
+  PopulateApps(kPages * kTilesPerPage);
+
+  const int first_index = 0;
+  const int last_index = kPages * kTilesPerPage - 1;
+  const int last_index_on_page1 = kTilesPerPage - 1;
+  const int first_index_on_page2 = kTilesPerPage;
 
-  // Try moving off the item to an invalid location.
+  // Try moving off the item beyond the first one.
+  apps_grid_view_->SetSelectedView(GetItemViewAt(first_index));
   SimulateKeyPress(ui::VKEY_UP);
-  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(0)));
-  SimulateKeyPress(ui::VKEY_DOWN);
-  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(0)));
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(first_index)));
   SimulateKeyPress(ui::VKEY_LEFT);
-  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(0)));
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(first_index)));
 
-  // Move to the second item and try to go past it.
+  // Move to the last item and try to go past it.
+  apps_grid_view_->SetSelectedView(GetItemViewAt(last_index));
+  SimulateKeyPress(ui::VKEY_DOWN);
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(last_index)));
   SimulateKeyPress(ui::VKEY_RIGHT);
-  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(1)));
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(last_index)));
+
+  // Move right on last item on page 1 should get to first item on page 2
+  // and vice vesa.
+  apps_grid_view_->SetSelectedView(GetItemViewAt(last_index_on_page1));
   SimulateKeyPress(ui::VKEY_RIGHT);
-  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(1)));
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(
+      first_index_on_page2)));
+  SimulateKeyPress(ui::VKEY_LEFT);
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(
+      last_index_on_page1)));
+
+  // Up/down changes page similarly to the above left/right cases.
+  apps_grid_view_->SetSelectedView(GetItemViewAt(last_index_on_page1));
+  SimulateKeyPress(ui::VKEY_DOWN);
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(
+      last_index_on_page1 + kCols)));
+  SimulateKeyPress(ui::VKEY_UP);
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(
+      last_index_on_page1)));
+
+  // After page switch, arrow keys select first item on current page.
+  apps_grid_view_->SetSelectedView(GetItemViewAt(first_index));
+  pagination_model_->SelectPage(1, false);
+  SimulateKeyPress(ui::VKEY_UP);
+  EXPECT_TRUE(apps_grid_view_->IsSelectedView(GetItemViewAt(
+      first_index_on_page2)));
 }
 
 }  // namespace test