From d4a45e987ea0e03db92d9bf64aadde35214c4311 Mon Sep 17 00:00:00 2001
From: Wolfgang Beyer <wolfi@chromium.org>
Date: Wed, 25 Jan 2023 13:27:42 +0000
Subject: [PATCH] DataGrid: when filtering, ignore content of invisible cells.

Fixed: 1410161
Change-Id: I111acc0ee8295edd46a994ae83159198526a5bb4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4194330
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Wolfgang Beyer <wolfi@chromium.org>
---
 .../data_grid/DataGridController.ts           |  7 +++---
 .../ui/components/data_grid/DataGridUtils.ts  |  2 +-
 .../ui/components/DataGridController_test.ts  | 25 +++++++++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/front_end/ui/components/data_grid/DataGridController.ts b/front_end/ui/components/data_grid/DataGridController.ts
index d2c2874978..63205cddb2 100644
--- a/front_end/ui/components/data_grid/DataGridController.ts
+++ b/front_end/ui/components/data_grid/DataGridController.ts
@@ -123,7 +123,7 @@ export class DataGridController extends HTMLElement {
     this.#render();
   }
 
-  #testRowWithFilter(row: Row, filter: TextUtils.TextUtils.ParsedFilter): boolean {
+  #testRowWithFilter(row: Row, filter: TextUtils.TextUtils.ParsedFilter, visibleColumnIds: Set<string>): boolean {
     let rowMatchesFilter = false;
 
     const {key, text, negative, regex} = filter;
@@ -132,7 +132,7 @@ export class DataGridController extends HTMLElement {
     if (key) {
       dataToTest = getStringifiedCellValues([getRowEntryForColumnId(row, key)]);
     } else {
-      dataToTest = getStringifiedCellValues(row.cells);
+      dataToTest = getStringifiedCellValues(row.cells.filter(cell => visibleColumnIds.has(cell.columnId)));
     }
 
     if (regex) {
@@ -155,11 +155,12 @@ export class DataGridController extends HTMLElement {
       return [...rows];
     }
 
+    const visibleColumnIds = new Set(this.#columns.filter(column => column.visible).map(column => column.id));
     return rows.map(row => {
       // We assume that the row should be visible by default.
       let rowShouldBeVisible = true;
       for (const filter of filters) {
-        const rowMatchesFilter = this.#testRowWithFilter(row, filter);
+        const rowMatchesFilter = this.#testRowWithFilter(row, filter, visibleColumnIds);
         // If there are multiple filters, if any return false we hide the row.
         // So if we get a false from testRowWithFilter, we can break early and return false.
         if (!rowMatchesFilter) {
diff --git a/front_end/ui/components/data_grid/DataGridUtils.ts b/front_end/ui/components/data_grid/DataGridUtils.ts
index 02aacd89d2..64fe8a4f45 100644
--- a/front_end/ui/components/data_grid/DataGridUtils.ts
+++ b/front_end/ui/components/data_grid/DataGridUtils.ts
@@ -40,7 +40,7 @@ export type CellValue = string|number|boolean|IconButton.Icon.Icon|null;
 
 /**
  * A cell contains a `columnId`, which is the ID of the column the cell
- * reprsents, and the `value`, which is a string value for that cell.
+ * represents, and the `value`, which is a string value for that cell.
  *
  * Note that currently cells cannot render complex data (e.g. nested HTML) but
  * in future we may extend the DataGrid to support this.
diff --git a/test/unittests/front_end/ui/components/DataGridController_test.ts b/test/unittests/front_end/ui/components/DataGridController_test.ts
index 21fde97eea..52adc75fd4 100644
--- a/test/unittests/front_end/ui/components/DataGridController_test.ts
+++ b/test/unittests/front_end/ui/components/DataGridController_test.ts
@@ -262,6 +262,31 @@ describe('DataGridController', () => {
       ]);
     });
 
+    it('only compares visible columns when matching filter', async () => {
+      const component = new DataGrid.DataGridController.DataGridController();
+      component.data = {rows, columns, filters: [createPlainTextFilter('e')]};
+      renderElementIntoDOM(component);
+      assertShadowRoot(component.shadowRoot);
+      await coordinator.done();
+      const internalDataGridShadow = getInternalDataGridShadowRoot(component);
+      let renderedRowValues = getValuesOfAllBodyRows(internalDataGridShadow, {onlyVisible: true});
+      assert.deepEqual(renderedRowValues, [
+        ['Letter A', 'Alpha'],
+        ['Letter B', 'Bravo'],
+        ['Letter C', 'Charlie'],
+      ]);
+
+      const columnsWithInvisible = structuredClone(columns);
+      columnsWithInvisible[0].visible = false;
+      component.data = {
+        ...component.data,
+        columns: columnsWithInvisible,
+      };
+      await coordinator.done();
+      renderedRowValues = getValuesOfAllBodyRows(internalDataGridShadow, {onlyVisible: true});
+      assert.deepEqual(renderedRowValues, [['Charlie']]);
+    });
+
     it('renders only visible rows, but maintains proper aria-rowindexes for the rows that are rendered', async () => {
       const component = new DataGrid.DataGridController.DataGridController();
       component.data = {rows, columns, filters: [createPlainTextFilter('bravo')]};