[DataGrid] Fix header filters showing clear button while empty #15829

k-rajat19 · 2024-12-10T12:50:28Z

I have followed (at least) the PR section of the contributing guide.

Closes #15778

https://deploy-preview-15829--material-ui-x.netlify.app/x/react-data-grid/filtering/header-filters/

Changelog

Breaking changes

The sanitizeFilterItemValue() utility is not exported anymore.

mui-bot · 2024-12-10T12:54:00Z

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

Updated pages:

docs/data/migration/migration-data-grid-v7/migration-data-grid-v7.md

Generated by 🚫 dangerJS against 1d992fa

packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

k-rajat19 · 2024-12-10T13:11:56Z

packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

@@ -62,17 +75,17 @@ function GridFilterInputValue(props: GridTypeFilterInputValueProps) {

  React.useEffect(() => {
    const itemPlusTag = item as ItemPlusTag;
-    if (itemPlusTag.fromInput !== id || item.value === undefined) {
-      setFilterValueState(String(item.value ?? ''));
+    if (itemPlusTag.fromInput !== id || item.value == null) {


using == to also consider null values

I'd rather explicitly use x === null || x === undefined rather than rely on the == operator, which I think we should not use in any situation.

Thanks for the review, but x == null is only loosely equal to undefined, no matter what is in item.value. If it is not null or undefined, it always returns false. Or are there any other downsides of using == opertaor?

arminmeh · 2024-12-10T17:20:33Z

This makes another change that should not be there.
If you change the operator and clear the value in the field (with backspace) it also resets the operator.
You can check the previous behavior here

k-rajat19 · 2024-12-11T10:13:26Z

This makes another change that should not be there. If you change the operator and clear the value in the field (with backspace) it also resets the operator.

this behavior has existed in previous versions as well, this was accidentally fixed in #15528
I have now fixed it with this commit ae075dd

k-rajat19 · 2024-12-12T17:04:40Z

Hi @arminmeh ,
I'm not able to figure out why the browser test keeps failing, everything is working fine and all unit tests are passing as well.
I would like you to look into this when you have time.
The test has started failing from this commit ae075dd

arminmeh · 2024-12-13T09:14:08Z

@k-rajat19 thanks for the effort so far
I will take a look at the failing test

arminmeh · 2024-12-16T10:22:38Z

packages/x-data-grid-pro/src/components/headerFiltering/GridHeaderFilterCell.tsx

@@ -167,13 +167,9 @@ const GridHeaderFilterCell = React.forwardRef<HTMLDivElement, GridHeaderFilterCe

    const applyFilterChanges = React.useCallback(
      (updatedItem: GridFilterItem) => {
-        if (item.value && updatedItem.value === undefined) {


@mui/xgrid I have removed deletion of the filter item to avoid
#15829 (comment)

do you see any issue with this?

Question for @KenanYusuf
With this change, deleting the value with the keyboard keeps the operator, but clicking on the clear (X) button resets everything
Should this be aligned or it is fine like this?

Should this be aligned or it is fine like this?

Definitely shouldn't reset the operator with keyboard deletion. I'm not certain it should reset upon clicking the clear button either... seems a little unexpected to me 🤔

I have updated it for the clear button as well

@mui/xgrid I have removed deletion of the filter item to avoid
#15829 (comment)

do you see any issue with this?

Doing this changed the behavior of how the filter model updation works.

Here's the console.log output for the arguments passed to onFilterModelChange for the following actions.

Add a filter

Press the clear X button

Current version Updated version (this PR)

Notice the filter item still being in the filterModel, it is a breaking behavior. If we want to make this a default behavior, should we do it for other column types too? (date, singleSelect, etc.)

I'd prefer not to keep the item in the filterModel which is cleared/doesn't have a valid value.

Wdyt?

thanks for pointing it out
I assumed that the clear button was there in the first place to remove the item from the model.

The issue is that the end user does not know that this is the goal of it and sees an empty input with a clear button still present.

should we then preserve the operator in the UI only?
It can update if the filter item operator gets changed programatically
Otherwise, it will keep its last value and once you start typing again, it will add a new item with both value and the last operator

Wdyt?

should we then preserve the operator in the UI only?

IMO, it comes down to the type of interaction the user did.
For removing text letter by letter with Backspace/Delete, the operator preserved makes sense.
However, upon clearing the filter completely (X button), I would expect the filter to be deleted (including the operator).

Consider an example interaction done by a user X.

He searches for names that start from 'John' (startsFrom operator has a connection with the value)
If he made a typo, he'd backspace and retype, but if the search has been completed and another search has to be done, he'd clear the filter (X button) intending to remove the operator too.

The next filter would most likely need another operator, let's say contains. So in my opinion, resetting the filter makes sense in that case.

Does this make sense to you?

That example does make sense @MBilalShafi, but the opposite scenario is also valid, where users are not expecting the operator to reset. The results would be confusing if they re-searched and had not noticed the operator reset. I think it's a design issue, because it doesn't look like the X is associated to the filter operator. I'll bear it in mind as I'm currently experimenting on an updated design for the header filters for v8.

the opposite scenario is also valid, where users are not expecting the operator to reset.

The positioning of "X" icon right next to the text and before the operator icon makes this argument solid, indeed! Some users might associate it with only the text and be surprised on the operator reset.
I'm fine with preserving the operator in UI until we come up with a better UI design. 👍

I will update the code so that the UI keeps the state and adds both the operator and the value once the user adds the value to the header filter
For the empty value, item will be deleted from the model

Update:
To avoid adding additional complexity for syncing the state between the UI and the model, I have reverted the code to the state where backspacing does not clear the operator, but clicking on the X button does.

At first, I thought that having a filter item in the model that does not have the value would be strange but then I saw that it happens anyway if you change the operator before typing in any value.

Also it is better to not have a separate UI state since the header filters have to react to the changes made in the filter toolbar

Nitpick: Remove applyFilterChanges and pass apiRef.current.upsertFilterItem directly.

arminmeh · 2024-12-20T18:58:59Z

scripts/x-data-grid-premium.exports.json

@@ -660,7 +660,6 @@
  { "name": "renderEditSingleSelectCell", "kind": "Variable" },
  { "name": "renderRowReorderCell", "kind": "Variable" },
  { "name": "RowPropsOverrides", "kind": "Interface" },
-  { "name": "sanitizeFilterItemValue", "kind": "Variable" },


I have removed this because I have noticed that it was unnecessarily exported here

It will not be back-ported to v7

github-actions · 2024-12-23T08:58:14Z

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

MBilalShafi

LGTM

packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx

MBilalShafi · 2024-12-23T10:45:39Z

packages/x-data-grid-pro/src/components/headerFiltering/GridHeaderFilterCell.tsx

@@ -167,13 +167,9 @@ const GridHeaderFilterCell = React.forwardRef<HTMLDivElement, GridHeaderFilterCe

    const applyFilterChanges = React.useCallback(
      (updatedItem: GridFilterItem) => {
-        if (item.value && updatedItem.value === undefined) {


Nitpick: Remove applyFilterChanges and pass apiRef.current.upsertFilterItem directly.

github-actions · 2024-12-23T12:57:21Z

Cherry-pick PRs will be created targeting branches: v7.x

k-rajat19 commented Dec 10, 2024

View reviewed changes

packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputValue.tsx Outdated Show resolved Hide resolved

k-rajat19 marked this pull request as ready for review December 10, 2024 13:11

k-rajat19 commented Dec 10, 2024

View reviewed changes

zannager added the component: data grid This is the name of the generic UI component, not the React module! label Dec 10, 2024

arminmeh added bug 🐛 Something doesn't work plan: Pro Impact at least one Pro user needs cherry-pick The PR should be cherry-picked to master after merge v7.x feature: Filtering on header Related to the header filtering (Pro) feature labels Dec 10, 2024

k-rajat19 force-pushed the issue-15778 branch 2 times, most recently from 92de0c8 to 4304555 Compare December 12, 2024 07:18

arminmeh force-pushed the issue-15778 branch from 6d984a7 to 8363581 Compare December 16, 2024 10:20

arminmeh reviewed Dec 16, 2024

View reviewed changes

arminmeh changed the title ~~[DataGrid] Fix Header filters do not clear properly when losing focus~~ [DataGrid] Fix Header filters showing clear button while empty Dec 16, 2024

arminmeh changed the title ~~[DataGrid] Fix Header filters showing clear button while empty~~ [DataGrid] Fix header filters showing clear button while empty Dec 16, 2024

arminmeh force-pushed the issue-15778 branch from 8363581 to 3063a4e Compare December 19, 2024 19:10

arminmeh requested review from KenanYusuf and MBilalShafi December 19, 2024 19:10

arminmeh force-pushed the issue-15778 branch 2 times, most recently from ba06c2b to 35c539d Compare December 20, 2024 13:02

arminmeh added the breaking change label Dec 20, 2024

arminmeh reviewed Dec 20, 2024

View reviewed changes

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

MBilalShafi approved these changes Dec 23, 2024

View reviewed changes

k-rajat19 and others added 13 commits December 23, 2024 13:30

handle values

78ff45f

store previous operator in filter cache

7200f18

fix

6ffa625

optimize logic

d73acd2

test new way

b514128

Revert useGridColumnHeaders update

1f27dd4

Do not clear operator

079cb4b

Add types

d876991

Revert comment and spacing

7d37bb6

lint

82a8140

Lint for boolean sanitize function

2a2d04b

Keep sanitize fn exported

270950f

Remove util from the public API

1d992fa

arminmeh force-pushed the issue-15778 branch from bb1b286 to 1d992fa Compare December 23, 2024 12:40

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

arminmeh enabled auto-merge (squash) December 23, 2024 12:44

KenanYusuf disabled auto-merge December 23, 2024 12:45

KenanYusuf enabled auto-merge (squash) December 23, 2024 12:46

KenanYusuf merged commit f572940 into mui:master Dec 23, 2024
16 checks passed

arminmeh added a commit that referenced this pull request Dec 23, 2024

Manual cherry-pick of #15829

7015699

arminmeh mentioned this pull request Dec 23, 2024

[DataGrid] Fix header filters showing clear button while empty (@k-rajat19) #15990

Merged

k-rajat19 deleted the issue-15778 branch December 29, 2024 18:04

This was referenced Jan 2, 2025

[data grid] Impossible to type decimal numbers starting with .0 in the filter #16052

Closed

[DataGrid] Fix number filter field formatting values while typing #16062

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Fix header filters showing clear button while empty #15829

[DataGrid] Fix header filters showing clear button while empty #15829

k-rajat19 commented Dec 10, 2024 •

edited by arminmeh

Loading

mui-bot commented Dec 10, 2024 •

edited

Loading

k-rajat19 Dec 10, 2024

romgrk Dec 16, 2024

k-rajat19 Dec 17, 2024 •

edited

Loading

arminmeh commented Dec 10, 2024

k-rajat19 commented Dec 11, 2024

k-rajat19 commented Dec 12, 2024

arminmeh commented Dec 13, 2024

arminmeh Dec 16, 2024

KenanYusuf Dec 16, 2024

arminmeh Dec 19, 2024

MBilalShafi Dec 20, 2024 •

edited

Loading

arminmeh Dec 20, 2024

MBilalShafi Dec 20, 2024 •

edited

Loading

KenanYusuf Dec 20, 2024

MBilalShafi Dec 20, 2024

arminmeh Dec 20, 2024 •

edited

Loading

MBilalShafi Dec 23, 2024

arminmeh Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 23, 2024

MBilalShafi left a comment

MBilalShafi Dec 23, 2024

github-actions bot commented Dec 23, 2024

[DataGrid] Fix header filters showing clear button while empty #15829

[DataGrid] Fix header filters showing clear button while empty #15829

Conversation

k-rajat19 commented Dec 10, 2024 • edited by arminmeh Loading

Changelog

Breaking changes

mui-bot commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k-rajat19 Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

arminmeh commented Dec 10, 2024

k-rajat19 commented Dec 11, 2024

k-rajat19 commented Dec 12, 2024

arminmeh commented Dec 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBilalShafi Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBilalShafi Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arminmeh Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arminmeh Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 23, 2024

MBilalShafi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 23, 2024

k-rajat19 commented Dec 10, 2024 •

edited by arminmeh

Loading

mui-bot commented Dec 10, 2024 •

edited

Loading

k-rajat19 Dec 17, 2024 •

edited

Loading

MBilalShafi Dec 20, 2024 •

edited

Loading

MBilalShafi Dec 20, 2024 •

edited

Loading

arminmeh Dec 20, 2024 •

edited

Loading

arminmeh Dec 20, 2024 •

edited

Loading