Improves Search Box Arrow Key Focus Traversal

Bug: 908621, 916724
Change-Id: Ib08120d4f98081d4c5317a481e282c0c06d25e98
Reviewed-on: https://chromium-review.googlesource.com/c/1449045
Commit-Queue: Kevin Strohbehn <ginko@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628539}

ash/app_list/app_list_util.cc

@@ -43,17 +43,12 @@ bool IsUnhandledArrowKeyEvent(const ui::KeyEvent& event) {
          event.key_code() == ui::VKEY_LEFT || event.key_code() == ui::VKEY_UP;
 }
 
-bool ProcessLeftRightKeyTraversalForTextfield(views::Textfield* textfield,
-                                              const ui::KeyEvent& key_event) {
+bool LeftRightKeyEventShouldExitText(views::Textfield* textfield,
+                                     const ui::KeyEvent& key_event) {
   DCHECK(IsUnhandledLeftRightKeyEvent(key_event));
 
-  const bool move_focus_reverse = base::i18n::IsRTL()
-                                      ? key_event.key_code() == ui::VKEY_RIGHT
-                                      : key_event.key_code() == ui::VKEY_LEFT;
-  if (textfield->text().empty()) {
-    textfield->GetFocusManager()->AdvanceFocus(move_focus_reverse);
+  if (textfield->text().empty())
     return true;
-  }
 
   if (textfield->HasSelection())
     return false;
@@ -79,6 +74,20 @@ bool ProcessLeftRightKeyTraversalForTextfield(views::Textfield* textfield,
     return false;
   }
 
+  return true;
+}
+
+bool ProcessLeftRightKeyTraversalForTextfield(views::Textfield* textfield,
+                                              const ui::KeyEvent& key_event) {
+  DCHECK(IsUnhandledLeftRightKeyEvent(key_event));
+
+  if (!LeftRightKeyEventShouldExitText(textfield, key_event))
+    return false;
+
+  const bool move_focus_reverse = base::i18n::IsRTL()
+                                      ? key_event.key_code() == ui::VKEY_RIGHT
+                                      : key_event.key_code() == ui::VKEY_LEFT;
+
   // Move focus outside the textfield.
   textfield->GetFocusManager()->AdvanceFocus(move_focus_reverse);
   return true;

ash/app_list/app_list_util.h

@@ -26,7 +26,14 @@ APP_LIST_EXPORT bool IsUnhandledUpDownKeyEvent(const ui::KeyEvent& event);
 // (unmodified by ctrl, shift, or alt)
 APP_LIST_EXPORT bool IsUnhandledArrowKeyEvent(const ui::KeyEvent& event);
 
-// Processes left/right key traversal for the given Textfield. Returns true
+// Returns true if the arrow key event should move focus away from the
+// |textfield|. This is usually when the insertion point would move away from
+// text.
+APP_LIST_EXPORT bool LeftRightKeyEventShouldExitText(
+    views::Textfield* textfield,
+    const ui::KeyEvent& key_event);
+
+// Processes left/right key traversal for the given |textfield|. Returns true
 // if focus is moved.
 APP_LIST_EXPORT bool ProcessLeftRightKeyTraversalForTextfield(
     views::Textfield* textfield,

ash/app_list/views/app_list_view_unittest.cc

@@ -801,7 +801,10 @@ TEST_F(AppListViewFocusTest, VerticalFocusTraversalInHalfState) {
       contents_view()
           ->search_result_tile_item_list_view_for_test()
           ->tile_views_for_test();
-  forward_view_list.push_back(tile_views[0]);
+  // We skip the first view when coming from the search box. This is because
+  // the first view is initially highlighted, and would already be activated
+  // upon pressing enter. Hence, we skip adding the tile view to the expected
+  // view list.
   forward_view_list.push_back(contents_view()
                                   ->search_result_answer_card_view_for_test()
                                   ->GetAnswerCardResultViewForTest());

ash/app_list/views/search_box_view.cc

@@ -247,21 +247,25 @@ void SearchBoxView::OnKeyEvent(ui::KeyEvent* event) {
   if (!IsUnhandledUpDownKeyEvent(*event))
     return;
 
-  // If focus is in search box view, up key moves focus to the last element of
-  // contents view if new style launcher is not enabled while it moves focus to
-  // expand arrow if the feature is enabled. Down key moves focus to the first
-  // element of contents view.
+  // Handles arrow key events from the search box while the search box is
+  // inactive. This covers both folder traversal and apps grid traversal. Search
+  // result traversal is handled in |HandleKeyEvent|
   AppListPage* page =
       contents_view_->GetPageView(contents_view_->GetActivePageIndex());
   views::View* arrow_view = contents_view_->expand_arrow_view();
-  views::View* v = event->key_code() == ui::VKEY_UP
-                       ? (arrow_view && arrow_view->IsFocusable()
-                              ? arrow_view
-                              : page->GetLastFocusableView())
-                       : page->GetFirstFocusableView();
-
-  if (v)
-    v->RequestFocus();
+  views::View* next_view = nullptr;
+
+  if (event->key_code() == ui::VKEY_UP) {
+    if (arrow_view && arrow_view->IsFocusable())
+      next_view = arrow_view;
+    else
+      next_view = page->GetLastFocusableView();
+  } else {
+    next_view = page->GetFirstFocusableView();
+  }
+
+  if (next_view)
+    next_view->RequestFocus();
   event->SetHandled();
 }
 
@@ -518,8 +522,33 @@ void SearchBoxView::UpdateQuery(const base::string16& new_query) {
 
 bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
                                    const ui::KeyEvent& key_event) {
-  if (search_box()->HasFocus() && is_search_box_active() &&
-      !search_box()->text().empty() && ShouldProcessAutocomplete()) {
+  if (key_event.type() == ui::ET_KEY_PRESSED &&
+      key_event.key_code() == ui::VKEY_RETURN) {
+    if (!IsSearchBoxTrimmedQueryEmpty()) {
+      // Hitting Enter when focus is on search box opens the first result.
+      ui::KeyEvent event(key_event);
+      views::View* first_result_view =
+          contents_view_->search_results_page_view()->first_result_view();
+      if (first_result_view)
+        first_result_view->OnKeyEvent(&event);
+      return true;
+    }
+
+    if (!is_search_box_active()) {
+      SetSearchBoxActive(true, key_event.type());
+      return true;
+    }
+    return false;
+  }
+
+  // Events occurring over an inactive search box are handled elsewhere.
+  if (!is_search_box_active())
+    return false;
+
+  // Handles autocomplete text confirmation/deletion
+  // TODO(ginko) fix logic for arrow keys in autocomplete
+  if (search_box()->HasFocus() && !search_box()->text().empty() &&
+      ShouldProcessAutocomplete()) {
     // If the search box has no text in it currently, autocomplete should not
     // work.
     last_key_pressed_ = key_event.key_code();
@@ -538,28 +567,59 @@ bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
       }
     }
   }
-  if (key_event.type() == ui::ET_KEY_PRESSED &&
-      key_event.key_code() == ui::VKEY_RETURN) {
-    if (!IsSearchBoxTrimmedQueryEmpty()) {
-      // Hitting Enter when focus is on search box opens the first result.
-      ui::KeyEvent event(key_event);
-      views::View* first_result_view =
-          contents_view_->search_results_page_view()->first_result_view();
-      if (first_result_view)
-        first_result_view->OnKeyEvent(&event);
-      return true;
-    }
 
-    if (!is_search_box_active()) {
-      SetSearchBoxActive(true, key_event.type());
+  // Only arrow key events intended for traversal within search results should
+  // be handled from here.
+  if (!IsUnhandledArrowKeyEvent(key_event))
+    return false;
+
+  SearchResultPageView* search_page =
+      contents_view_->search_results_page_view();
+
+  // Left/Right arrow keys are handled elsewhere, unless the first result is a
+  // tile, in which case right will be handled below.
+  if (key_event.key_code() == ui::VKEY_LEFT ||
+      (key_event.key_code() == ui::VKEY_RIGHT &&
+       !search_page->IsFirstResultTile())) {
+    return ProcessLeftRightKeyTraversalForTextfield(search_box(), key_event);
+  }
+
+  // Right arrow key should not be handled if the cursor is within text.
+  if (key_event.key_code() == ui::VKEY_RIGHT &&
+      !LeftRightKeyEventShouldExitText(search_box(), key_event)) {
+    return false;
+  }
+
+  views::View* result_view = nullptr;
+
+  // The up arrow will loop focus to the last result.
+  // The down and right arrows will be treated the same, moving focus along to
+  // the 'next' result. If a result is highlighted, we treat that result as
+  // though it already had focus.
+  if (key_event.key_code() == ui::VKEY_UP) {
+    result_view = search_page->GetLastFocusableView();
+  } else if (search_page->IsFirstResultHighlighted()) {
+    result_view = search_page->GetFirstFocusableView();
+
+    // Give the parent container a chance to handle the event. This lets the
+    // down arrow escape the tile result container.
+    if (!result_view->parent()->OnKeyPressed(key_event)) {
+      // If the parent container doesn't handle |key_event|, get the next
+      // focusable view.
+      result_view = result_view->GetFocusManager()->GetNextFocusableView(
+          result_view, result_view->GetWidget(), false, false);
+    } else {
+      // Return early if the parent container handled the event.
       return true;
     }
-    return false;
+  } else {
+    result_view = search_page->GetFirstFocusableView();
   }
 
-  if (IsUnhandledLeftRightKeyEvent(key_event))
-    return ProcessLeftRightKeyTraversalForTextfield(search_box(), key_event);
-  return false;
+  if (result_view)
+    result_view->RequestFocus();
+
+  return true;
 }
 
 bool SearchBoxView::HandleMouseEvent(views::Textfield* sender,

ash/app_list/views/search_result_page_view.cc

@@ -203,6 +203,19 @@ void SearchResultPageView::AddSearchResultContainerView(
   result_container->set_delegate(this);
 }
 
+bool SearchResultPageView::IsFirstResultTile() const {
+  // |kRecommendation| result type refers to tiles in Zero State.
+  return first_result_view_->result()->display_type() ==
+             ash::SearchResultDisplayType::kTile ||
+         first_result_view_->result()->display_type() ==
+             ash::SearchResultDisplayType::kRecommendation;
+}
+
+bool SearchResultPageView::IsFirstResultHighlighted() const {
+  DCHECK(first_result_view_);
+  return first_result_view_->background_highlighted();
+}
+
 bool SearchResultPageView::OnKeyPressed(const ui::KeyEvent& event) {
   // Let the FocusManager handle Left/Right keys.
   if (!IsUnhandledUpDownKeyEvent(event))

ash/app_list/views/search_result_page_view.h

@@ -34,6 +34,9 @@ class APP_LIST_EXPORT SearchResultPageView
     return result_container_views_;
   }
 
+  bool IsFirstResultTile() const;
+  bool IsFirstResultHighlighted() const;
+
   // Overridden from views::View:
   bool OnKeyPressed(const ui::KeyEvent& event) override;
   const char* GetClassName() const override;