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] Remove trailing spaces in filter input #5279

Merged
merged 4 commits into from
Jun 24, 2022

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Jun 21, 2022

Fix #5275

The dev proposed that when entering "is ok " the filter applies with "is ok" (without the last space)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@mui-bot
Copy link

mui-bot commented Jun 21, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 287.8 581.8 487.2 437.46 112.818
Sort 100k rows ms 479.4 983.3 913.2 826.94 185.574
Select 100k rows ms 129.9 264.6 215.1 213.18 48.366
Deselect 100k rows ms 112.7 199.3 134.4 151.44 33.061

Generated by 🚫 dangerJS against 31c8f03

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@alexfauquette alexfauquette marked this pull request as ready for review June 22, 2022 11:40
@alexfauquette alexfauquette added the feature: Filtering Related to the data grid Filtering feature label Jun 22, 2022
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.

Wouldn't be better to trim inside the operator? If filterModel.items.value contains space it shows no result: https://codesandbox.io/s/controlledfilters-demo-mui-x-forked-84qzhq?file=/demo.tsx

@alexfauquette
Copy link
Member Author

@m4theushw My concern was about how to disable the trim if I want to take into account the last space?

We can simply pass prop to the Input component, but accessing to the filtering engine seems a bit more complicated

@m4theushw
Copy link
Member

We can add an argument to getGridStringOperators. To disable the trim, developers only need to override the operators. What do you think?

diff --git a/packages/grid/x-data-grid/src/colDef/gridStringOperators.ts b/packages/grid/x-data-grid/src/colDef/gridStringOperators.ts
index b6ccd38b5..f97a44e06 100644
--- a/packages/grid/x-data-grid/src/colDef/gridStringOperators.ts
+++ b/packages/grid/x-data-grid/src/colDef/gridStringOperators.ts
@@ -15,7 +15,7 @@ export const getGridStringQuickFilterFn = (value: any) => {
   };
 };
 
-export const getGridStringOperators = (): GridFilterOperator<
+export const getGridStringOperators = (disableTrim: boolean = false): GridFilterOperator<
   any,
   number | string | null,
   any

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@alexfauquette alexfauquette requested a review from m4theushw June 24, 2022 07:37
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.

Much cleaner now.

@alexfauquette alexfauquette merged commit 11726a2 into mui:master Jun 24, 2022
@alexfauquette alexfauquette deleted the trimFilterValue branch June 24, 2022 13:05
joserodolfofreitas pushed a commit to joserodolfofreitas/mui-x that referenced this pull request Jul 15, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
alexfauquette added a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Filtering Related to the data grid Filtering feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Filter value with trailing spaces
3 participants