[DataGrid] Fix controllable filters and select all rows with filters (m…

…ui#1020)
heyfirst · Feb 10, 2021 · 0f7d253 · 0f7d253
1 parent a7f81ec
commit 0f7d253
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 33 deletions.
diff --git a/packages/grid/_modules_/grid/components/CheckboxRenderer.tsx b/packages/grid/_modules_/grid/components/CheckboxRenderer.tsx
@@ -1,15 +1,16 @@
-import { useContext } from 'react';
 import * as React from 'react';
 import Checkbox from '@material-ui/core/Checkbox';
 import { useGridSelector } from '../hooks/features/core/useGridSelector';
+import { visibleSortedRowsSelector } from '../hooks/features/filter/filterSelector';
 import { rowCountSelector } from '../hooks/features/rows/rowsSelector';
 import { selectedRowsCountSelector } from '../hooks/features/selection/selectionSelector';
 import { ColParams } from '../models/params/colParams';
 import { CellParams } from '../models/params/cellParams';
 import { ApiContext } from './api-context';
 
 export const HeaderCheckbox: React.FC<ColParams> = () => {
-  const apiRef = useContext(ApiContext);
+  const apiRef = React.useContext(ApiContext);
+  const visibleRows = useGridSelector(apiRef, visibleSortedRowsSelector);
 
   const totalSelectedRows = useGridSelector(apiRef, selectedRowsCountSelector);
   const totalRows = useGridSelector(apiRef, rowCountSelector);
@@ -30,7 +31,10 @@ export const HeaderCheckbox: React.FC<ColParams> = () => {
 
   const handleChange = (event: React.ChangeEvent<HTMLInputElement>, checked: boolean) => {
     setChecked(checked);
-    apiRef!.current.selectRows(apiRef!.current.getAllRowIds(), checked);
+    apiRef!.current.selectRows(
+      visibleRows.map((row) => row.id),
+      checked,
+    );
   };
 
   return (
@@ -48,7 +52,7 @@ HeaderCheckbox.displayName = 'HeaderCheckbox';
 
 export const CellCheckboxRenderer: React.FC<CellParams> = React.memo((props) => {
   const { row, getValue, field } = props;
-  const apiRef = useContext(ApiContext);
+  const apiRef = React.useContext(ApiContext);
 
   const handleChange = (event: React.ChangeEvent<HTMLInputElement>, checked: boolean) => {
     apiRef!.current.selectRow(row.id, checked, true);

diff --git a/packages/grid/_modules_/grid/hooks/features/filter/useFilter.ts b/packages/grid/_modules_/grid/hooks/features/filter/useFilter.ts
@@ -293,7 +293,7 @@ export const useFilter = (apiRef: ApiRef, rowsProp: RowsProp): void => {
   React.useEffect(() => {
     const filterModel = options.filterModel;
     const oldFilterModel = apiRef.current.state.filter;
-    if (filterModel && filterModel.items.length > 0 && !isDeepEqual(filterModel, oldFilterModel)) {
+    if (filterModel && !isDeepEqual(filterModel, oldFilterModel)) {
       logger.debug('filterModel prop changed, applying filters');
       // we use apiRef to avoid watching setFilterModel as it will trigger an update on every state change
       apiRef.current.setFilterModel(filterModel);

diff --git a/packages/grid/data-grid/src/tests/toolbar.DataGrid.test.tsx b/packages/grid/data-grid/src/tests/toolbar.DataGrid.test.tsx
@@ -6,7 +6,7 @@ import {
   // @ts-expect-error need to migrate helpers to TypeScript
   screen,
 } from 'test/utils';
-import { getColumnHeaders } from 'test/utils/helperFn';
+import { getColumnHeadersTextContent } from 'test/utils/helperFn';
 import { expect } from 'chai';
 import { DataGrid, GridToolbar } from '@material-ui/data-grid';
 import {
@@ -142,12 +142,12 @@ describe('<DataGrid /> - Toolbar', () => {
         </div>,
       );
 
-      expect(getColumnHeaders()).to.deep.equal(['id', 'brand']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['id', 'brand']);
 
       fireEvent.click(getByText('Columns'));
       fireEvent.click(document.querySelector('[role="tooltip"] [name="id"]'));
 
-      expect(getColumnHeaders()).to.deep.equal(['brand']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['brand']);
     });
 
     it('should hide all columns when clicking "HIDE ALL" from the column selector', () => {
@@ -163,12 +163,12 @@ describe('<DataGrid /> - Toolbar', () => {
         </div>,
       );
 
-      expect(getColumnHeaders()).to.deep.equal(['id', 'brand']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['id', 'brand']);
 
       fireEvent.click(getByText('Columns'));
       fireEvent.click(getByText('Hide All'));
 
-      expect(getColumnHeaders()).to.deep.equal([]);
+      expect(getColumnHeadersTextContent()).to.deep.equal([]);
     });
 
     it('should show all columns when clicking "SHOW ALL" from the column selector', () => {
@@ -199,7 +199,7 @@ describe('<DataGrid /> - Toolbar', () => {
       fireEvent.click(getByText('Columns'));
       fireEvent.click(getByText('Show All'));
 
-      expect(getColumnHeaders()).to.deep.equal(['id', 'brand']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['id', 'brand']);
     });
   });
 });
diff --git a/packages/grid/x-grid/src/tests/filtering.XGrid.test.tsx b/packages/grid/x-grid/src/tests/filtering.XGrid.test.tsx
@@ -1,9 +1,20 @@
-import { ApiRef, FilterModel, LinkOperator, useApiRef, XGrid } from '@material-ui/x-grid';
 import { expect } from 'chai';
 import * as React from 'react';
 import { useFakeTimers } from 'sinon';
-import { getColumnValues } from 'test/utils/helperFn';
-import { createClientRenderStrictMode } from 'test/utils';
+import { getColumnHeaderCell, getColumnValues } from 'test/utils/helperFn';
+import {
+  createClientRenderStrictMode,
+  // @ts-expect-error need to migrate helpers to TypeScript
+  fireEvent,
+} from 'test/utils';
+import {
+  ApiRef,
+  FilterModel,
+  GridComponentProps,
+  LinkOperator,
+  useApiRef,
+  XGrid,
+} from '@material-ui/x-grid';
 
 describe('<XGrid /> - Filter', () => {
   let clock;
@@ -28,7 +39,7 @@ describe('<XGrid /> - Filter', () => {
 
   let apiRef: ApiRef;
 
-  const TestCase = (props: { rows?: any[]; model: FilterModel }) => {
+  const TestCase = (props: Partial<GridComponentProps>) => {
     const baselineProps = {
       rows: [
         {
@@ -47,16 +58,16 @@ describe('<XGrid /> - Filter', () => {
       columns: [{ field: 'brand' }],
     };
 
-    const { model, rows } = props;
+    const { rows, ...other } = props;
     apiRef = useApiRef();
     return (
       <div style={{ width: 300, height: 300 }}>
         <XGrid
           apiRef={apiRef}
           {...baselineProps}
           rows={rows || baselineProps.rows}
-          filterModel={model}
           disableColumnFilter={false}
+          {...other}
         />
       </div>
     );
@@ -73,13 +84,13 @@ describe('<XGrid /> - Filter', () => {
   };
 
   it('should apply the filterModel prop correctly', () => {
-    render(<TestCase model={model} />);
+    render(<TestCase filterModel={model} />);
 
     expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
   });
 
   it('should apply the filterModel prop correctly on ApiRef setRows', () => {
-    render(<TestCase model={model} />);
+    render(<TestCase filterModel={model} />);
 
     const newRows = [
       {
@@ -101,15 +112,15 @@ describe('<XGrid /> - Filter', () => {
   });
 
   it('should apply the filterModel prop correctly on ApiRef update row data', () => {
-    render(<TestCase model={model} />);
+    render(<TestCase filterModel={model} />);
     apiRef.current.updateRows([{ id: 1, brand: 'Fila' }]);
     apiRef.current.updateRows([{ id: 0, brand: 'Patagonia' }]);
     clock.tick(100);
     expect(getColumnValues()).to.deep.equal(['Patagonia', 'Fila', 'Puma']);
   });
 
   it('should allow apiRef to setFilterModel', () => {
-    render(<TestCase model={model} />);
+    render(<TestCase filterModel={model} />);
     apiRef.current.setFilterModel({
       items: [
         {
@@ -137,13 +148,12 @@ describe('<XGrid /> - Filter', () => {
         },
       ],
     };
-    render(<TestCase model={newModel} />);
-
+    render(<TestCase filterModel={newModel} />);
     expect(getColumnValues()).to.deep.equal(['Puma']);
   });
 
   it('should allow multiple filter via apiRef', () => {
-    render(<TestCase model={model} />);
+    render(<TestCase filterModel={model} />);
     const newModel = {
       items: [
         {
@@ -178,7 +188,40 @@ describe('<XGrid /> - Filter', () => {
       ],
       linkOperator: LinkOperator.Or,
     };
-    render(<TestCase model={newModel} />);
+    render(<TestCase filterModel={newModel} />);
     expect(getColumnValues()).to.deep.equal(['Adidas', 'Puma']);
   });
+
+  it('should only select visible rows', () => {
+    const newModel: FilterModel = {
+      items: [
+        {
+          columnField: 'brand',
+          value: 'a',
+          operatorValue: 'startsWith',
+        },
+      ],
+      linkOperator: LinkOperator.Or,
+    };
+    render(<TestCase checkboxSelection filterModel={newModel} />);
+    const checkAllCell = getColumnHeaderCell(1).querySelector('input');
+    fireEvent.click(checkAllCell);
+    expect(apiRef.current.getState().selection).to.deep.equal({ 1: true });
+  });
+
+  it('should allow to clear filters by passing an empty filter model', () => {
+    const newModel: FilterModel = {
+      items: [
+        {
+          columnField: 'brand',
+          value: 'a',
+          operatorValue: 'startsWith',
+        },
+      ],
+    };
+    const { setProps } = render(<TestCase filterModel={newModel} />);
+    expect(getColumnValues()).to.deep.equal(['Adidas']);
+    setProps({ filterModel: { items: [] } });
+    expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
+  });
 });
diff --git a/packages/grid/x-grid/src/tests/reorder.XGrid.test.tsx b/packages/grid/x-grid/src/tests/reorder.XGrid.test.tsx
@@ -5,7 +5,7 @@ import {
   // @ts-expect-error need to migrate helpers to TypeScript
   act,
 } from 'test/utils';
-import { getColumnHeaders, raf } from 'test/utils/helperFn';
+import { getColumnHeadersTextContent, raf } from 'test/utils/helperFn';
 import { ApiRef, useApiRef, XGrid } from '@material-ui/x-grid';
 
 describe('<XGrid /> - Reorder', () => {
@@ -49,13 +49,13 @@ describe('<XGrid /> - Reorder', () => {
 
       const { setProps } = render(<TestCase width={300} />);
 
-      expect(getColumnHeaders()).to.deep.equal(['id', 'brand']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['id', 'brand']);
       act(() => {
         apiRef!.current.moveColumn('id', 1);
       });
       setProps({ width: 200 });
       await raf();
-      expect(getColumnHeaders()).to.deep.equal(['brand', 'id']);
+      expect(getColumnHeadersTextContent()).to.deep.equal(['brand', 'id']);
     });
   });
 
@@ -75,10 +75,10 @@ describe('<XGrid /> - Reorder', () => {
     };
 
     const { forceUpdate } = render(<Test />);
-    expect(getColumnHeaders()).to.deep.equal(['brand', 'desc', 'type']);
+    expect(getColumnHeadersTextContent()).to.deep.equal(['brand', 'desc', 'type']);
     apiRef!.current.moveColumn('brand', 2);
-    expect(getColumnHeaders()).to.deep.equal(['desc', 'type', 'brand']);
+    expect(getColumnHeadersTextContent()).to.deep.equal(['desc', 'type', 'brand']);
     forceUpdate(); // test stability
-    expect(getColumnHeaders()).to.deep.equal(['desc', 'type', 'brand']);
+    expect(getColumnHeadersTextContent()).to.deep.equal(['desc', 'type', 'brand']);
   });
 });
diff --git a/packages/storybook/src/stories/grid-filter.stories.tsx b/packages/storybook/src/stories/grid-filter.stories.tsx
@@ -614,7 +614,7 @@ export function DemoMultiFilteringGrid() {
 
   return (
     <div style={{ height: 400, width: '100%' }}>
-      <XGrid filterModel={demoFilterModel} {...data} />
+      <XGrid filterModel={demoFilterModel} {...data} checkboxSelection />
     </div>
   );
 }
diff --git a/test/utils/helperFn.ts b/test/utils/helperFn.ts
@@ -44,7 +44,15 @@ export function getColumnValues(colIndex: number = 0) {
   );
 }
 
-export function getColumnHeaders() {
+export function getColumnHeaderCell(colIndex: number): HTMLElement {
+  const columnHeader = document.querySelector(`[role="columnheader"][aria-colindex="${colIndex}"]`);
+  if (columnHeader == null) {
+    throw new Error(`columnheader ${colIndex} not found`);
+  }
+  return columnHeader as HTMLElement;
+}
+
+export function getColumnHeadersTextContent() {
   return Array.from(document.querySelectorAll('[role="columnheader"]')).map(
     (node) => node!.textContent,
   );