Tab Search: Tab item focus and navigation

Enable focus navigation for tab items and their buttons.

Related CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2388711

Bug: 1113470
Change-Id: I32b1ed98f0f832ff4c7dba5747e3437a860ab0d0
Reviewed-on: https://chrome-internal-review.googlesource.com/c/chrome/browser/resources/tab_search/+/3209477Reviewed-by: John Lee <johntlee@chromium.org>
Reviewed-by: Tom Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819595}

chrome/browser/resources/tab_search_merge/BUILD.gn

@@ -25,6 +25,7 @@ js_library("app") {
     ":tab_search_api_proxy",
     ":tab_search_item",
     ":tab_search_search_field",
+    "//third_party/polymer/v3_0/components-chromium/iron-selector:iron-selector",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
     "//ui/webui/resources/js:cr.m",
     "//ui/webui/resources/js:util.m",

chrome/browser/resources/tab_search_merge/app.html

@@ -3,6 +3,7 @@
     max-height: 280px;
     overflow-x: hidden;
     overflow-y: auto;
+    position: relative;
   }
 
   #no-results {
@@ -54,15 +55,16 @@
   </svg>
 </iron-iconset-svg>
 <tab-search-search-field id="searchField" autofocus clear-label="Clear search"
-    label="Search tabs [[getKeyboardShortcut_()]]" on-keydown="onKeyDown_"
-    on-search-changed="onSearchChanged_" >
+    label="Search tabs [[getKeyboardShortcut_()]]" on-keydown="onSearchKeyDown_"
+    on-search-changed="onSearchChanged_">
 </tab-search-search-field>
 <div id="tabs">
-  <iron-selector id="selector" selected="{{selectedIndex_}}"
-      selected-class="selected">
+  <iron-selector id="selector" on-keydown="onItemKeyDown_"
+      selected="{{selectedIndex_}}" selected-class="selected">
     <template id="tabsList" is="dom-repeat" items="[[filteredOpenTabs_]]">
       <tab-search-item id="[[item.tabId]]" class="mwb-list-item" data="[[item]]"
-          on-click="onItemClick_" on-close="onItemClose_" tabindex="-1" >
+          on-click="onItemClick_" on-close="onItemClose_"
+          on-focus="onItemFocus_" tabindex="0" >
       </tab-search-item>
     </template>
   </iron-selector>
@@ -70,7 +72,8 @@
 <div id="no-results" hidden="[[filteredOpenTabs_.length]]">
   No results found
 </div>
-<button id="feedback-footer" class="mwb-list-item" on-click="onFeedbackClick_">
+<button id="feedback-footer" class="mwb-list-item" on-click="onFeedbackClick_"
+    on-focus="onFeedbackFocus_">
   <iron-icon id="feedback-icon" icon="ts:feedback"></iron-icon>
   <div id="feedback-text">Submit feedback</div>
 </button>

chrome/browser/resources/tab_search_merge/app.js

@@ -19,6 +19,8 @@ import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/poly
 import {fuzzySearch} from './fuzzy_search.js';
 import {TabSearchApiProxy, TabSearchApiProxyImpl} from './tab_search_api_proxy.js';
 
+const selectorNavigationKeys = ['ArrowUp', 'ArrowDown', 'Home', 'End'];
+
 export class TabSearchAppElement extends PolymerElement {
   static get is() {
     return 'tab-search-app';
@@ -36,12 +38,6 @@ export class TabSearchAppElement extends PolymerElement {
         value: '',
       },
 
-      /**
-       * The seleted item's index, or -1 if no item selected.
-       * @private {number}
-       */
-      selectedIndex_: {type: Number, value: -1},
-
       /** @private {?Array<!tabSearch.mojom.WindowTabs>} */
       openTabs_: {
         type: Array,
@@ -132,9 +128,15 @@ export class TabSearchAppElement extends PolymerElement {
     }
   }
 
-  /** @return {number} */
+  /**
+   * The seleted item's index, or -1 if no item selected.
+   * @return {number}
+   */
   getSelectedIndex() {
-    return this.selectedIndex_;
+    const selector = /** @type {!IronSelectorElement} */ (this.$.selector);
+    return selector.selected !== undefined ?
+        /** @type {number} */ (selector.selected) :
+        -1;
   }
 
   /**
@@ -146,7 +148,8 @@ export class TabSearchAppElement extends PolymerElement {
 
     this.updateFilteredTabs_(this.openTabs_ || []);
     // Reset the selected item whenever a search query is provided.
-    this.selectedIndex_ = this.filteredOpenTabs_.length > 0 ? 0 : -1;
+    this.$.selector.selected =
+        this.filteredOpenTabs_.length > 0 ? 0 : undefined;
     this.$.tabs.scrollTop = 0;
   }
 
@@ -155,6 +158,11 @@ export class TabSearchAppElement extends PolymerElement {
     this.apiProxy_.showFeedbackPage();
   }
 
+  /** @private */
+  onFeedbackFocus_() {
+    this.$.selector.selected = undefined;
+  }
+
   /**
    * @param {!Event} e
    * @private
@@ -184,86 +192,99 @@ export class TabSearchAppElement extends PolymerElement {
     // selected; else retain the currently selected index. If the list
     // shrunk above the selected index, select the last index in the list.
     // If there are no matching results, set the selected index value to none.
-    this.selectedIndex_ = Math.min(
-        Math.max(this.selectedIndex_, 0), this.filteredOpenTabs_.length - 1);
+    this.$.selector.selectIndex(Math.min(
+        Math.max(this.getSelectedIndex(), 0),
+        this.filteredOpenTabs_.length - 1));
   }
 
   /**
-   * @param {number} index A valid index for an element present in the
-   *     filteredOpenTabs_ array.
-   * @return {?HTMLElement}
+   * @param {!Event} e
    * @private
    */
-  getTabSearchItem_(index) {
-    const tabItemId = assert(this.filteredOpenTabs_[index]).tabId;
-    return this.shadowRoot.getElementById(tabItemId.toString());
+  onItemFocus_(e) {
+    this.$.selector.selected =
+        e.currentTarget.parentNode.indexOf(e.currentTarget);
+    this.updateScrollView_();
+  }
+
+  /**
+   * @param {string} key Keyboard event key value.
+   * @private
+   */
+  selectorNavigate_(key) {
+    const selector = /** @type {!IronSelectorElement} */ (this.$.selector);
+    switch (key) {
+      case 'ArrowUp':
+        selector.selectPrevious();
+        break;
+      case 'ArrowDown':
+        selector.selectNext();
+        break;
+      case 'Home':
+        selector.selected = 0;
+        break;
+      case 'End':
+        selector.selected = this.filteredOpenTabs_.length - 1;
+        break;
+    }
   }
 
   /**
-   * TODO(crbug.com/1113470): Tab Search item and buttons focus and navigation.
+   * Handles key events when list item elements have focus.
    * @param {!KeyboardEvent} e
    * @private
    */
-  onKeyDown_(e) {
-    // Do not interfere with the search field's management of text selection.
+  onItemKeyDown_(e) {
     if (e.shiftKey) {
       return;
     }
 
     e.stopPropagation();
 
-    if (this.selectedIndex_ === -1) {
+    if (this.getSelectedIndex() === -1) {
       // No tabs matching the search text criteria.
       return;
     }
 
-    switch (e.key) {
-      case 'ArrowUp':
-        this.selectItem_(-1);
-        e.preventDefault();
-        break;
-      case 'ArrowDown':
-        this.selectItem_(1);
-        e.preventDefault();
-        break;
-      case 'Home':
-        this.selectItem_(-this.selectedIndex_);
-        e.preventDefault();
-        break;
-      case 'End':
-        this.selectItem_(
-            this.filteredOpenTabs_.length - 1 - this.selectedIndex_);
-        e.preventDefault();
-        break;
-      case 'Enter':
-        const selectedItem = this.filteredOpenTabs_[this.selectedIndex_];
-        this.apiProxy_.switchToTab({tabId : selectedItem.tabId},
-                                   !!this.searchText_);
-        break;
+    if (selectorNavigationKeys.includes(e.key)) {
+      this.selectorNavigate_(e.key);
+      /** @type {!HTMLElement} */ (this.$.selector.selectedItem).focus({
+        preventScroll: true
+      });
+      e.preventDefault();
+    } else if (e.key === 'Enter' || e.key === ' ') {
+      const selectedItem = this.filteredOpenTabs_[this.getSelectedIndex()];
+      this.apiProxy_.switchToTab(
+          {tabId: selectedItem.tabId}, !!this.searchText_);
     }
   }
 
   /**
-   * @param {number} offset Distance from the desired item to select and the
-   *     currently selected item.
+   * Handles key events when the search field has focus.
+   * @param {!KeyboardEvent} e
    * @private
    */
-  selectItem_(offset) {
-    const length = assert(this.filteredOpenTabs_.length);
-    this.selectedIndex_ = (this.selectedIndex_ + offset + length) % length;
-
-    // Ensure the scroll view can fully display a preceding or following tab
-    // item if existing. Use Math.sign to identify any such preceding or
-    // following item.
-    if (this.selectedIndex_ === 0 ||
-        this.selectedIndex_ === this.filteredOpenTabs_.length - 1) {
-      this.getTabSearchItem_(this.selectedIndex_).scrollIntoView({
-        behavior: 'smooth'
-      });
-    } else {
-      this.getTabSearchItem_(this.selectedIndex_ + Math.sign(offset))
-          .scrollIntoView(
-              {behavior: 'smooth', block: offset > 0 ? 'end' : 'start'});
+  onSearchKeyDown_(e) {
+    // Do not interfere with the search field's management of text selection.
+    if (e.shiftKey) {
+      return;
+    }
+
+    e.stopPropagation();
+
+    if (this.getSelectedIndex() === -1) {
+      // No tabs matching the search text criteria.
+      return;
+    }
+
+    if (selectorNavigationKeys.includes(e.key)) {
+      this.selectorNavigate_(e.key);
+      this.updateScrollView_();
+      e.preventDefault();
+    } else if (e.key === 'Enter') {
+      const selectedItem = this.filteredOpenTabs_[this.getSelectedIndex()];
+      this.apiProxy_.switchToTab(
+          {tabId: selectedItem.tabId}, !!this.searchText_);
     }
   }
 
@@ -292,6 +313,35 @@ export class TabSearchAppElement extends PolymerElement {
         0);
     this.filteredOpenTabs_ = fuzzySearch(this.searchText_, result);
   }
+
+  /**
+   * Ensure the scroll view can fully display a preceding or following tab item
+   * if existing.
+   * @private
+   */
+  updateScrollView_() {
+    const selectedIndex = this.getSelectedIndex();
+    if (selectedIndex === 0 ||
+        selectedIndex === this.filteredOpenTabs_.length - 1) {
+      /** @type {!Element} */ (this.$.selector.selectedItem).scrollIntoView({
+        behavior: 'smooth'
+      });
+    } else {
+      const previousItem = this.$.selector.items[this.$.selector.selected - 1];
+      if (previousItem.offsetTop < this.$.tabs.scrollTop) {
+        /** @type {!Element} */ (previousItem)
+            .scrollIntoView({behavior: 'smooth', block: 'nearest'});
+        return;
+      }
+
+      const nextItem = this.$.selector.items[this.$.selector.selected + 1];
+      if (nextItem.offsetTop + nextItem.offsetHeight >
+          this.$.tabs.scrollTop + this.$.tabs.offsetHeight) {
+        /** @type {!Element} */ (nextItem).scrollIntoView(
+            {behavior: 'smooth', block: 'nearest'});
+      }
+    }
+  }
 }
 
 customElements.define(TabSearchAppElement.is, TabSearchAppElement);

chrome/browser/resources/tab_search_merge/tab_search_item.html

 <style>
-  :host(:hover, .selected) .button-container {
-    display: flex;
+  :host(:focus) {
+    outline-color: rgba(var(--mwb-background-color), 0.24);
   }
 
-  .button-container {
-    display: none;
+  :host(:hover, .selected) .button-container cr-icon-button {
+    --cr-icon-button-fill-color: var(--google-grey-refresh-700);
+  }
+
+  @media (prefers-color-scheme: dark) {
+    :host(:hover, .selected) .button-container cr-icon-button {
+      --cr-icon-button-fill-color: var(--google-grey-refresh-300);
+    }
+  }
+
+  .button-container cr-icon-button {
+    --cr-icon-button-fill-color: transparent;
   }
 
   img {
@@ -50,7 +60,7 @@
   <div class="secondary-text">[[urlHostname_(data.url)]]</div>
 </div>
 <div class="button-container">
-  <cr-icon-button aria-labelledby="label" iron-icon="cr:cancel"
-      on-click="onItemCancel_" title="$i18n{close}">
+  <cr-icon-button id="closeButton" aria-labelledby="label"
+      iron-icon="cr:close" on-click="onItemClose_" title="$i18n{close}">
   </cr-icon-button>
 </div>

chrome/browser/resources/tab_search_merge/tab_search_item.js

@@ -30,7 +30,7 @@ export class TabSearchItem extends PolymerElement {
    * @param {!Event} e
    * @private
    */
-  onItemCancel_(e) {
+  onItemClose_(e) {
     this.dispatchEvent(new CustomEvent('close'));
     e.stopPropagation();
   }

chrome/browser/resources/tab_search_merge/test/BUILD.gn

@@ -14,6 +14,7 @@ js_type_check("closure_compile") {
   deps = [
     ":fuzzy_search_test",
     ":tab_search_app_test",
+    ":tab_search_app_focus_test",
   ]
 }
 
@@ -34,6 +35,15 @@ js_library("tab_search_app_test") {
   externs_list = [ "$externs_path/mocha-2.5.js" ]
 }
 
+js_library("tab_search_app_focus_test") {
+  deps = [
+    ":test_tab_search_api_proxy",
+    "../..:chai_assert",
+    "//chrome/browser/resources/tab_search:app",
+  ]
+  externs_list = [ "$externs_path/mocha-2.5.js" ]
+}
+
 js_library("test_tab_search_api_proxy") {
   deps = [
     "//chrome/browser/resources/tab_search:tab_search_api_proxy",

chrome/browser/resources/tab_search_merge/test/tab_search_app_focus_test.js

+// Copyright 2020 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.
+
+import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
+import {keyDownOn} from 'chrome://resources/polymer/v3_0/iron-test-helpers/mock-interactions.js';
+import {TabSearchAppElement} from 'chrome://tab-search/app.js';
+import {TabSearchApiProxy, TabSearchApiProxyImpl} from 'chrome://tab-search/tab_search_api_proxy.js'
+import {TabSearchSearchField} from 'chrome://tab-search/tab_search_search_field.js';
+
+import {assertEquals} from '../../chai_assert.js';
+import {flushTasks} from '../../test_util.m.js';
+
+import {sampleData} from './tab_search_test_data.js';
+import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js';
+
+suite('TabSearchAppFocusTest', () => {
+  /** @type {!TabSearchAppElement} */
+  let tabSearchApp;
+  /** @type {!TestTabSearchApiProxy} */
+  let testProxy;
+
+  setup(async () => {
+    testProxy = new TestTabSearchApiProxy();
+    testProxy.setProfileTabs(sampleData());
+    TabSearchApiProxyImpl.instance_ = testProxy;
+
+    tabSearchApp = /** @type {!TabSearchAppElement} */
+        (document.createElement('tab-search-app'));
+
+    document.body.innerHTML = '';
+    document.body.appendChild(tabSearchApp);
+    await flushTasks();
+  });
+
+  test('KeyNavigation', async () => {
+    // Initially, the search input should have focus.
+    const searchField = /** @type {!TabSearchSearchField} */
+        (tabSearchApp.shadowRoot.querySelector('#searchField'));
+    const searchInput = /** @type {!HTMLInputElement} */
+        (searchField.shadowRoot.querySelector('#searchInput'))
+    assertEquals(searchInput, getDeepActiveElement());
+
+    const tabSearchItems = /** @type {!NodeList<!HTMLElement>} */
+        (tabSearchApp.shadowRoot.querySelectorAll('tab-search-item'));
+
+    tabSearchItems[0].focus();
+    // Once an item is focused, arrow keys should change focus too.
+    keyDownOn(tabSearchItems[0], 0, [], 'ArrowDown');
+    assertEquals(tabSearchItems[1], getDeepActiveElement());
+
+    keyDownOn(tabSearchItems[1], 0, [], 'ArrowUp');
+    assertEquals(tabSearchItems[0], getDeepActiveElement());
+
+    keyDownOn(tabSearchItems[1], 0, [], 'End');
+    assertEquals(
+        tabSearchItems[tabSearchItems.length - 1], getDeepActiveElement());
+
+    keyDownOn(tabSearchItems[tabSearchItems.length - 1], 0, [], 'Home');
+    assertEquals(tabSearchItems[0], getDeepActiveElement());
+  });
+
+  test('KeyPress', async () => {
+    const tabSearchItem = /** @type {!HTMLElement} */
+        (tabSearchApp.shadowRoot.querySelector('tab-search-item'));
+    tabSearchItem.focus();
+
+    keyDownOn(tabSearchItem, 0, [], 'Enter');
+    keyDownOn(tabSearchItem, 0, [], ' ');
+    assertEquals(2, testProxy.getCallCount('switchToTab'));
+
+    const closeButton = /** @type {!HTMLElement} */ (
+        tabSearchItem.shadowRoot.querySelector('#closeButton'));
+    keyDownOn(closeButton, 0, [], 'Enter');
+    assertEquals(1, testProxy.getCallCount('closeTab'));
+  });
+});

chrome/browser/resources/tab_search_merge/test/tab_search_app_test.js

@@ -10,6 +10,7 @@ import {TabSearchApiProxy, TabSearchApiProxyImpl} from 'chrome://tab-search/tab_
 import {assertEquals, assertNotEquals, assertFalse, assertTrue} from '../../chai_assert.js';
 import {flushTasks, waitAfterNextRender} from '../../test_util.m.js';
 
+import {sampleData} from './tab_search_test_data.js';
 import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js';
 
 suite('TabSearchAppTest', () => {
@@ -36,67 +37,6 @@ suite('TabSearchAppTest', () => {
     return tabSearchApp.shadowRoot.querySelectorAll('tab-search-item');
   }
 
-  function sampleData() {
-    const profileTabs = {
-      windows: [
-        {
-          active: true,
-          tabs: [
-            {
-              index: 0,
-              tabId: 1,
-              favIconUrl: '',
-              title: 'Google',
-              url: 'https://www.google.com',
-            },
-            {
-              index: 1,
-              tabId: 5,
-              favIconUrl: '',
-              title: 'Amazon',
-              url: 'https://www.amazon.com',
-            },
-            {
-              index: 2,
-              tabId: 6,
-              favIconUrl: '',
-              title: 'Apple',
-              url: 'https://www.apple.com',
-            }
-          ],
-        },
-        {
-          active: false,
-          tabs: [
-            {
-              index: 0,
-              tabId: 2,
-              favIconUrl: '',
-              title: 'Bing',
-              url: 'https://www.bing.com/',
-            },
-            {
-              index: 1,
-              tabId: 3,
-              favIconUrl: '',
-              title: 'Yahoo',
-              url: 'https://www.yahoo.com',
-            },
-            {
-              index: 2,
-              tabId: 4,
-              favIconUrl: '',
-              title: 'Apple',
-              url: 'https://www.apple.com/',
-            },
-          ]
-        }
-      ]
-    };
-
-    return profileTabs;
-  }
-
   async function setupTest(sampleData) {
     testProxy = new TestTabSearchApiProxy();
     testProxy.setProfileTabs(sampleData);

chrome/browser/resources/tab_search_merge/test/tab_search_interactive_ui_tests.js

+// Copyright 2020 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.
+
+/** @fileoverview Test suite for the WebUI tab search. */
+
+GEN_INCLUDE(['//chrome/test/data/webui/polymer_interactive_ui_test.js']);
+
+GEN('#include "content/public/test/browser_test.h"');
+GEN('#include "chrome/browser/ui/ui_features.h"');
+GEN('#include "services/network/public/cpp/features.h"');
+
+// eslint-disable-next-line no-var
+var TabSearchInteractiveUITest = class extends PolymerInteractiveUITest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://tab-search/test_loader.html?module=tab_search/test/tab_search_app_focus_test.js';
+  }
+
+  get extraLibraries() {
+    return [
+      '//third_party/mocha/mocha.js',
+      '//chrome/test/data/webui/mocha_adapter.js',
+    ];
+  }
+
+  /** @override */
+  get featureList() {
+    return {
+      enabled: [
+        'network::features::kOutOfBlinkCors',
+        'features::kTabSearch',
+      ]
+    };
+  }
+}
+
+TEST_F('TabSearchInteractiveUITest', 'All', function() {
+  mocha.run();
+});

chrome/browser/resources/tab_search_merge/test/tab_search_test_data.js

+// Copyright 2020 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.
+
+export function sampleData() {
+  const profileTabs = {
+    windows: [
+      {
+        active: true,
+        tabs: [
+          {
+            index: 0,
+            tabId: 1,
+            favIconUrl: '',
+            title: 'Google',
+            url: 'https://www.google.com',
+          },
+          {
+            index: 1,
+            tabId: 5,
+            favIconUrl: '',
+            title: 'Amazon',
+            url: 'https://www.amazon.com',
+          },
+          {
+            index: 2,
+            tabId: 6,
+            favIconUrl: '',
+            title: 'Apple',
+            url: 'https://www.apple.com',
+          }
+        ],
+      },
+      {
+        active: false,
+        tabs: [
+          {
+            index: 0,
+            tabId: 2,
+            favIconUrl: '',
+            title: 'Bing',
+            url: 'https://www.bing.com/',
+          },
+          {
+            index: 1,
+            tabId: 3,
+            favIconUrl: '',
+            title: 'Yahoo',
+            url: 'https://www.yahoo.com',
+          },
+          {
+            index: 2,
+            tabId: 4,
+            favIconUrl: '',
+            title: 'Apple',
+            url: 'https://www.apple.com/',
+          },
+        ]
+      }
+    ]
+  };
+
+  return profileTabs;
+}