Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] Add quick filtering engine #4317

Merged
merged 49 commits into from
May 10, 2022

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Mar 30, 2022

Fix #2842

The quick filter model I took is the following:

It uses a list of values to filter and it can returns

  • rows that contain all the values (default behavior)
  • rows that contain at least one of the values

It's applied after the basic filtering because it makes more comparisons than the basic one.

For now, they both work together (filtered rows must pass both the quick and the basic filtering) This decision is to avoid strange behaviors such as: "When a user set a quick filter value, it reset the filter panel" or "When the quick filter is activated, the filter panel does not have effect"

For now, I added properties in the filterModel. I'm not sure if it is the correct way to do that or if filtering should add another model quickFilterModel

To support row grouping, the signature of the internal method shouldApplyFilter is modified. instead of taking a filter item as an input, it takes the columnField

Demo: https://deploy-preview-4317--material-ui-x.netlify.app/x/react-data-grid/filtering/#quick-filter

TODO

  • Tests
  • support singleSelect columns
  • Evaluate the feasibility of date columns

Changelog

You can now add a quick filtering search bar to your grid. To so, either pass showQuickFilter prop to the <GridToolbar /> or use the <GridToolbarQuickFilter /> component in your custom toolbar.

More information about how to customize the filtering logic in the documentation

image

@mui-bot
Copy link

mui-bot commented Mar 30, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 280.4 684.6 433.2 459.52 138.851
Sort 100k rows ms 509.7 1,223 1,223 928.64 258.539
Select 100k rows ms 192.1 300.5 235.9 249.66 41.074
Deselect 100k rows ms 122.4 267.1 193.6 198.68 59.742

Generated by 🚫 dangerJS against 754022a

@alexfauquette alexfauquette marked this pull request as ready for review March 31, 2022 12:31
Comment on lines +229 to +230
colDef: GridStateColDef,
apiRef: React.MutableRefObject<GridApiCommunity>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a use case where colDef and apiRef need to be accessed?

colDef could be redundant since the function is defined inside the column definition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For most of the cases it's uselesse. But I had to add those arguments for the single-select column.

From a word in the quick search, I search in valueOptions the values that have a label corresponding to the word
So I need colDef to access the valueOptions. and I need the api to apply the valueParser as follow

valueFormatter({ value: option, field, api }) 

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is taken form how the GridFilterInputSingleSelect renders options

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case maybe deprecate the apiRef (or api) since we are going to remove it from GridValueFormatterParams in v6

Copy link
Member

@m4theushw m4theushw Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is taken form how the GridFilterInputSingleSelect renders options

This makes me think that maybe singleSelect should have a default value formatter in v6. This would avoid having to duplicate code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m4theushw I added this to #3287

packages/grid/x-data-grid/src/models/gridFilterItem.ts Outdated Show resolved Hide resolved
packages/grid/x-data-grid/src/models/gridFilterModel.ts Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 28, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 29, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 3, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor visual suggestions. Otherwise, it looks good to me to be released. Good job! How about enabling it in the demo in https://mui.com/x/react-data-grid/demo/?

@@ -27,6 +27,7 @@ const GridToolbarContainerRoot = styled('div', {
})(({ theme }) => ({
display: 'flex',
alignItems: 'center',
flexWrap: 'wrap',
padding: theme.spacing(0.5, 0.5, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input bottom border touches the column headers if we leave as 0px.

image

Suggested change
padding: theme.spacing(0.5, 0.5, 0),
padding: theme.spacing(0.5),

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do that, it will also modify the Toolbar even when there is no quick filter

What about adding paddingBottom:theme.spacing(0.5) to the quick filter component?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the bottom padding should be there since the beginning. The already existent buttons also touch the border.

image

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 6, 2022
@alexfauquette
Copy link
Member Author

The quick filter seems to work well with demo data. It will be even more interesting when we will get real data 👍

@alexfauquette alexfauquette merged commit 192fb98 into mui:master May 10, 2022
@alexfauquette alexfauquette deleted the quick-filter branch May 10, 2022 09:16
import * as React from 'react';
import PropTypes from 'prop-types';
import TextField, { TextFieldProps } from '@mui/material/TextField';
import SearchIcon from '@mui/icons-material/Search';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our app started failing to compile this morning because it couldn't resolve @mui/icons-material, seems like it is not added in the dependencies of this module. We had to separately install @mui/icons-material to get our app working again. We are not using Material Icons in our project at all either, is this an absolute necessity to fetch and use the SearchIcon from Material Icons in this package?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been reported in #4906
The fix will be available in next release

alexfauquette added a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
Co-authored-by: José Rodolfo Freitas <joserodolfo.freitas@gmail.com>
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Implement Quick Filter
7 participants