Tab search: Selected tab-item index logic changes.

A search text change should reset the selected index to the first
element of the list, if such an item exists, while a open tabs change
should retain the current selected index, if it exists.

Bug: 1099917
Change-Id: I07997c15dff33e7b5e62ab752cd3007f49199dcd
Reviewed-on: https://chrome-internal-review.googlesource.com/c/chrome/browser/resources/tab_search/+/3209263Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Tom Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819586}

chrome/browser/resources/tab_search_merge/app.js

@@ -15,15 +15,6 @@ import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/poly
 import {listenOnce} from 'chrome://resources/js/util.m.js';
 import {TabSearchApiProxy, TabSearchApiProxyImpl} from './tab_search_api_proxy.js';
 
-/**
- * @param {string} searchText
- * @param {!tabSearch.mojom.Tab} item
- * @return {boolean}
- */
-function filterFunc(searchText, item) {
-  return item.title.toLowerCase().includes(searchText.toLowerCase());
-};
-
 export class TabSearchAppElement extends PolymerElement {
   static get is() {
     return 'tab-search-app';
@@ -48,12 +39,15 @@ export class TabSearchAppElement extends PolymerElement {
       selectedIndex_: {type: Number, value: -1},
 
       /** @private {?Array<!tabSearch.mojom.WindowTabs>} */
-      openTabs_: Array,
+      openTabs_: {
+        type: Array,
+        observer: 'openTabsChanged_',
+      },
 
       /** @private {!Array<!tabSearch.mojom.Tab>} */
       filteredOpenTabs_: {
         type: Array,
-        computed: 'getFilteredTabs_(openTabs_, searchText_)',
+        value: [],
       },
     };
   }
@@ -133,30 +127,17 @@ export class TabSearchAppElement extends PolymerElement {
     return this.selectedIndex_;
   }
 
-  /**
-   * @param {?Array<!tabSearch.mojom.WindowTabs>} windows
-   * @param {string} searchText
-   * @return {!Array<!tabSearch.mojom.Tab>}
-   * @private
-   */
-  getFilteredTabs_(windows, searchText) {
-    const result = [];
-    if (windows) {
-      windows.forEach(window => {
-        result.push(...window.tabs.filter(filterFunc.bind(null, searchText)));
-      });
-    }
-
-    this.selectedIndex_ = result.length > 0 ? 0 : -1;
-    return result;
-  }
-
   /**
    * @param {!CustomEvent<string>} e
    * @private
    */
   onSearchChanged_(e) {
     this.searchText_ = e.detail;
+
+    this.updateFilteredTabs_(this.openTabs_ || []);
+    // Reset the selected item whenever a search query is provided.
+    this.selectedIndex_ = this.filteredOpenTabs_.length > 0 ? 0 : -1;
+    this.$.tabs.scrollTop = 0;
   }
 
   /**
@@ -177,6 +158,21 @@ export class TabSearchAppElement extends PolymerElement {
     this.apiProxy_.closeTab(tabId);
   }
 
+  /**
+   * @param {!Array<!tabSearch.mojom.WindowTabs>} newOpenTabs
+   * @private
+   */
+  openTabsChanged_(newOpenTabs) {
+    this.updateFilteredTabs_(newOpenTabs);
+
+    // If there was no previously selected index, set the first item as
+    // 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);
+  }
+
   /**
    * @param {number} index A valid index for an element present in the
    *     filteredOpenTabs_ array.
@@ -263,6 +259,23 @@ export class TabSearchAppElement extends PolymerElement {
   getKeyboardShortcut_() {
     return (isMac ? 'Cmd' : 'Ctrl') + '+Shift+E';
   }
+
+  /**
+   * @param {!Array<!tabSearch.mojom.WindowTabs>} windowTabs
+   * @private
+   */
+  updateFilteredTabs_(windowTabs) {
+    const lowerCaseSearchText = this.searchText_.toLowerCase();
+    this.filteredOpenTabs_ = !!this.searchText_ ?
+        windowTabs
+            .map(window => {
+              return window.tabs.filter(item => {
+                return item.title.toLowerCase().includes(lowerCaseSearchText);
+              });
+            })
+            .flat() :
+        windowTabs.map(window => window.tabs).flat();
+  }
 }
 
 customElements.define(TabSearchAppElement.is, TabSearchAppElement);

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

@@ -7,7 +7,7 @@ import {TabSearchAppElement} from 'chrome://tab-search/app.js';
 import {TabSearchSearchField} from 'chrome://tab-search/tab_search_search_field.js';
 import {TabSearchApiProxy, TabSearchApiProxyImpl} from 'chrome://tab-search/tab_search_api_proxy.js'
 
-import {assertEquals, assertFalse, assertTrue} from '../../chai_assert.js';
+import {assertEquals, assertNotEquals, assertFalse, assertTrue} from '../../chai_assert.js';
 import {flushTasks, waitAfterNextRender} from '../../test_util.m.js';
 
 import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js';
@@ -115,20 +115,30 @@ suite('TabSearchAppTest', () => {
     verifyTabIds(queryRows(), [1, 5, 6, 2, 3, 4]);
   });
 
-  test('return filtered tabs', async () => {
+  test('Default tab selection when data is present', async () => {
+    await setupTest(sampleData());
+    assertNotEquals(-1, tabSearchApp.getSelectedIndex(),
+        'No default selection in the presence of data');
+  });
+
+  test('Search text changes tab items', async () => {
     await setupTest(sampleData());
     const searchField = /** @type {!TabSearchSearchField} */
       (tabSearchApp.shadowRoot.querySelector("#searchField"));
     searchField.setValue('bing');
     await flushTasks();
     verifyTabIds(queryRows(), [2]);
+    assertEquals(0, tabSearchApp.getSelectedIndex());
   });
 
-  test('Default tab selection when data is present', async () => {
+  test('No tab selected when there are no search matches', async () => {
     await setupTest(sampleData());
-    assertTrue(
-        tabSearchApp.getSelectedIndex() != -1,
-        'No default selection in the precense of data');
+    const searchField = /** @type {!TabSearchSearchField} */
+        (tabSearchApp.shadowRoot.querySelector('#searchField'));
+    searchField.setValue('Twitter');
+    await flushTasks();
+    assertEquals(0, queryRows().length);
+    assertEquals(-1, tabSearchApp.getSelectedIndex());
   });
 
   test('Click on tab item triggers actions', async () => {
@@ -174,10 +184,11 @@ suite('TabSearchAppTest', () => {
   });
 
   test('Keyboard navigation abides by item list range boundaries', async () => {
-    await setupTest(sampleData());
+    const testData = sampleData();
+    await setupTest(testData);
 
     const numTabs =
-        sampleData().windows.reduce((total, w) => total + w.tabs.length, 0);
+        testData.windows.reduce((total, w) => total + w.tabs.length, 0);
     const searchField = /** @type {!TabSearchSearchField} */
         (tabSearchApp.shadowRoot.querySelector("#searchField"));
 
@@ -219,6 +230,28 @@ suite('TabSearchAppTest', () => {
     testProxy.getCallbackRouterRemote().tabsChanged();
     await flushTasks();
     verifyTabIds(queryRows(), []);
+    assertEquals(-1, tabSearchApp.getSelectedIndex());
+  });
+
+  test('On tabs changed, tab item selection preserved or updated', async () => {
+    const testData = sampleData();
+    await setupTest(testData);
+
+    const searchField = /** @type {!TabSearchSearchField} */
+        (tabSearchApp.shadowRoot.querySelector('#searchField'));
+    keyDownOn(searchField, 0, [], 'ArrowDown');
+    assertEquals(1, tabSearchApp.getSelectedIndex());
+
+    testProxy.setProfileTabs({windows: [testData.windows[0]]});
+    testProxy.getCallbackRouterRemote().tabsChanged();
+    await flushTasks();
+    assertEquals(1, tabSearchApp.getSelectedIndex());
+
+    testProxy.setProfileTabs(
+        {windows: [{active: true, tabs: [testData.windows[0].tabs[0]]}]});
+    testProxy.getCallbackRouterRemote().tabsChanged();
+    await flushTasks();
+    assertEquals(0, tabSearchApp.getSelectedIndex());
   });
 
   test('refresh on tab updated', async () => {