[DataGrid] Allow to ignore diacritics when filtering #10569

cherniavskii · 2023-10-03T20:52:50Z

Closes #10376
Makes the workaround in #9817 (comment) unnecessary.

Preview: https://deploy-preview-10569--material-ui-x.netlify.app/x/react-data-grid/filtering/quick-filter/#ignore-diacritics-accents

TODO:

Add unit tests

mui-bot · 2023-10-03T20:57:43Z

Deploy preview: https://deploy-preview-10569--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against 4f11d1f

packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts

MBilalShafi

Nice enhancement, thanks for working on this. 👍

docs/data/data-grid/filtering/quick-filter.md

MBilalShafi · 2023-10-10T10:30:57Z

docs/data/data-grid/filtering/quick-filter.md

+{{"demo": "QuickFilteringDiacritics.js", "bg": "inline", "defaultCodeOpen": false}}
+
+:::warning
+Note that `ignoreDiacritics` affects [regular filters](/x/react-data-grid/filtering/), [Quick filter](/x/react-data-grid/filtering/quick-filter/) and [Header filters](/x/react-data-grid/filtering/header-filters/) simultaneously.


Probably this warning won't be necessary if we move this section to the customization page

Suggested change

Note that `ignoreDiacritics` affects [regular filters](/x/react-data-grid/filtering/), [Quick filter](/x/react-data-grid/filtering/quick-filter/) and [Header filters](/x/react-data-grid/filtering/header-filters/) simultaneously.

Note that `ignoreDiacritics` affects [normal filters](/x/react-data-grid/filtering/), [quick filter](/x/react-data-grid/filtering/quick-filter/) and [header filters](/x/react-data-grid/filtering/header-filters/) simultaneously.

I wanted to make it explicit. Not everything on Customization page applies to all filter types

packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts

MBilalShafi · 2023-10-10T10:57:16Z

packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts

-          fn: getApplyQuickFilterFnV7(value, column, apiRef),
-        })),
+        appliers: quickFilterValues.map((quickFilterValue) => {
+          const value = column.ignoreDiacritics


Minor: Does it make sense to make this a grid root prop? I guess not as it's only supported with string types.
I am considering a less usual scenario with hundreds of columns with accented data where such a search is needed but users have to add ignoreDiacritics to all of the colDefs, a root-level prop may save some time/hassle.
But I'm not sure about making this a root prop, since it's string only. 🤷‍♂️

Doesn't necessarily just apply to string columns. Not sure what's the default date format but e.g. "February" is "février" in French so we can get diacritics elsewhere, and those are supposed to be searchable through the quickfilter. Any column can return a formatted localized (aka with diacritics) value with the .valueFormatter also.

I think it might make sense to make it a root-level option.

Indeed, for quick filters, it could apply to any column type.
For regular filters and header filters, it will only apply to the string column type.

I was thinking about where to put this option and decided to put it in GridColDef to allow per-column configuration.
But it's a bit of a pain if you have a lot of columns and want to ignore diacritics in all of them.
This seems to emphasize a larger problem - we don't provide a way to pass something to all columns at once (or all columns of a certain type at once).

Regarding ignoreDiacritics specifically, enabling it on the root level should cover most, if not all, use cases.
So this makes sense to me, I will update the demo and the docs 👍🏻

resatakks · 2023-10-30T11:24:57Z

@cherniavskii hi,gm.Any update?maybe u can help,i watching this issue for few weeks

cherniavskii · 2023-10-30T15:05:12Z

@resatakks Thanks for the reminder, I'll update the PR to address the feedback from the team 👍🏻

cherniavskii · 2023-10-31T18:24:28Z

packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts

@@ -175,6 +182,13 @@ const getFilterCallbackFromItem = (
  } else {
    parsedValue = filterItem.value;
  }
+
+  const { ignoreDiacriticsInFiltering } = apiRef.current.getRootProps();


I have added the getRootProps method to be able to access ignoreDiacriticsInFiltering here, otherwise, I had to pass the prop through in ~20 places 😅
What do you think?

Makes sense. Could have even been just a field like apiRef.current.rootProps, this is private anyway.

Changed to apiRef.current.rootProps instead 👍🏻

What do you think?

Nice one, it could also be useful in some other places in the future too.

romgrk · 2023-11-01T06:58:35Z

packages/grid/x-data-grid-premium/src/DataGridPremium/DataGridPremium.tsx

+  /**
+   * If `true`, the diacritics (accents) are ignored when filtering or quick filtering.
+   * * E.g. when filter value is `cafe`, the rows with `café` will be visible.
+   * @default false
+   */
+  ignoreDiacriticsInFiltering: PropTypes.bool,


Not sure I like the naming, but I don't have anything better to propose atm.

Nit: To me personally, simply ignoreDiacritics also makes sense, as the JSDoc comment will be sufficient to explain the relation with filtering. Also, the user using it should already be aware of what it does so maybe a little bit less explanatory title would make the prop name a bit simple?

romgrk · 2023-11-01T07:06:08Z

docs/data/data-grid/filtering/QuickFilteringDiacritics.tsx

+      params.value.toLocaleDateString('fr-FR', {
+        day: 'numeric',
+        month: 'long',
+        year: 'numeric',
+      }),


Sidenote but this is slow for formatters, not sure if we should show the more performant way in this example though. (using Intl.DateTimeFormat directly)

Interesting, I didn't think about performance in this case.
However, the difference seems to be minimal (toLocaleDateString being a bit faster): https://jsperf.app/lohope

Am I missing something?

.toLocaleDateString builds a new Intl.DateTimeFormat instance on each call. It's best to build an instance outside the loop, and re-use it for each call.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString

Another sidenote but I have also noticed in previous performance benchmarking that using the native Intl formatter vs using a JS based one is much slower. Crossing the JS/C++ boundary is usually quite bad for performance.

Oh, gotcha!
Here's a benchmark for the singleton formatter: https://jsperf.app/hutiwe

I'll update the demo.

using the native Intl formatter vs using a JS based one is much slower

I didn't know that, thanks for the heads-up 👍🏻
I think for demo purposes Intl.DateTimeFormat is fine.

cherniavskii added 3 commits October 3, 2023 21:36

add ignoreDiacritics to gridColDef

ca452f7

avoid calling both getRowValue and getRowFormattedValue

c14f23c

support ignoreDiacritics during quick filtering

b5f54d6

cherniavskii added component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature customization: logic Logic customizability labels Oct 3, 2023

Merge branch 'master' into ignore-diacritics

9e66af1

cherniavskii mentioned this pull request Oct 5, 2023

Unable to Use Local Characters (e.g., "İ", "Ş") in Material-UI DataGrid Filter #10376

Closed

2 tasks

cherniavskii added 3 commits October 5, 2023 21:54

add unit tests

06a5e3f

add unit tests for quick filtering

bfe5306

add a demo

ae4e190

cherniavskii requested review from MBilalShafi and romgrk October 5, 2023 21:19

romgrk reviewed Oct 5, 2023

View reviewed changes

packages/grid/x-data-grid/src/hooks/features/filter/gridFilterUtils.ts Outdated Show resolved Hide resolved

romgrk approved these changes Oct 5, 2023

View reviewed changes

cherniavskii added 2 commits October 6, 2023 18:54

rename argument

92b2830

do not wait for flag

a56bd73

resatakks approved these changes Oct 9, 2023

View reviewed changes

cherniavskii marked this pull request as ready for review October 9, 2023 18:27

MBilalShafi approved these changes Oct 10, 2023

View reviewed changes

Merge branch 'master' into ignore-diacritics

17cd875

cherniavskii added 5 commits October 30, 2023 16:07

Merge branch 'master' into ignore-diacritics

1f2e3e8

change function signature to accept arguments in object

1d32a5a

add root-level ignoreDiacriticsInFiltering prop

37f12fb

yarn proptypes

38463f5

yarn docs:api

ef0be36

cherniavskii added 3 commits October 31, 2023 16:46

add getRootProps method to apiRef

767294a

use getRootProps method instead of passing ignoreDiacriticsInFiltering

0ff8b5a

Merge branch 'master' into ignore-diacritics

7a49da6

cherniavskii requested review from MBilalShafi and romgrk October 31, 2023 16:43

cherniavskii added 4 commits October 31, 2023 19:09

update demo

6c02040

update jsdoc

0f94386

update demo

0092a91

update docs

6d4fd1b

cherniavskii commented Oct 31, 2023

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 1, 2023

This comment was marked as resolved.

Sign in to view

romgrk reviewed Nov 1, 2023

View reviewed changes

romgrk approved these changes Nov 1, 2023

View reviewed changes

romgrk reviewed Nov 1, 2023

View reviewed changes

cherniavskii added 2 commits November 1, 2023 20:18

Merge branch 'master' into ignore-diacritics

d1ba202

getRootProps => rootProps

9453a3c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 1, 2023

MBilalShafi approved these changes Nov 1, 2023

View reviewed changes

cherniavskii added 3 commits November 2, 2023 14:25

Merge branch 'master' into ignore-diacritics

28dc32d

rename the prop

15b524f

extract formatter

4f11d1f

cherniavskii merged commit b0cb729 into mui:master Nov 2, 2023
5 checks passed

cherniavskii deleted the ignore-diacritics branch November 2, 2023 13:56

cherniavskii mentioned this pull request Nov 3, 2023

[DataGrid] Fix cell value type in quick filtering v7 #10884

Merged

alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Nov 6, 2023

[DataGrid] Allow to ignore diacritics when filtering (mui#10569)

5cfa7fe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Allow to ignore diacritics when filtering #10569

[DataGrid] Allow to ignore diacritics when filtering #10569

cherniavskii commented Oct 3, 2023 •

edited

Loading

mui-bot commented Oct 3, 2023 •

edited

Loading

MBilalShafi left a comment

MBilalShafi Oct 10, 2023

cherniavskii Oct 13, 2023

MBilalShafi Oct 10, 2023

romgrk Oct 11, 2023

cherniavskii Oct 13, 2023

resatakks commented Oct 30, 2023

cherniavskii commented Oct 30, 2023

cherniavskii Oct 31, 2023

romgrk Nov 1, 2023

cherniavskii Nov 1, 2023

MBilalShafi Nov 1, 2023

This comment was marked as resolved.

romgrk Nov 1, 2023

MBilalShafi Nov 1, 2023

romgrk Nov 1, 2023

cherniavskii Nov 1, 2023

romgrk Nov 2, 2023

romgrk Nov 2, 2023 •

edited

Loading

cherniavskii Nov 2, 2023

	Note that `ignoreDiacritics` affects [regular filters](/x/react-data-grid/filtering/), [Quick filter](/x/react-data-grid/filtering/quick-filter/) and [Header filters](/x/react-data-grid/filtering/header-filters/) simultaneously.
	Note that `ignoreDiacritics` affects [normal filters](/x/react-data-grid/filtering/), [quick filter](/x/react-data-grid/filtering/quick-filter/) and [header filters](/x/react-data-grid/filtering/header-filters/) simultaneously.

[DataGrid] Allow to ignore diacritics when filtering #10569

[DataGrid] Allow to ignore diacritics when filtering #10569

Conversation

cherniavskii commented Oct 3, 2023 • edited Loading

mui-bot commented Oct 3, 2023 • edited Loading

MBilalShafi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

resatakks commented Oct 30, 2023

cherniavskii commented Oct 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romgrk Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cherniavskii commented Oct 3, 2023 •

edited

Loading

mui-bot commented Oct 3, 2023 •

edited

Loading

romgrk Nov 2, 2023 •

edited

Loading