mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.07k stars 1.26k forks source link

[DataGrid] Can a custom filtering implementation make use of MUI's filter state? #11355

Closed apedretti closed 9 months ago

apedretti commented 9 months ago

The problem in depth 🔍

Custom filters

I've added a custom filtering solution to the DataGrid. Here's how it looks: image

That CustomDataGrid is being used on a couple places. Some places use the custom filtering, and other places use the native, MUI filter system. Because any of those two could be used, I tried to integrate my custom filtering solution to the native state management.

With that I mean, I set the filters using: apiContext.current.setFilterModel(newFilterModel);

And get them through: const filterModel = useGridSelector(apiContext, gridFilterModelSelector);

So far so good and everything works.

The issue

The issue that I have is that some actions trigger a filtering of the filterModel.items, only allowing those that refer to a field present in the column definition.

I've tracked that behavior to the columnsChange handler in useGridFilter.tsx (don't know if it's present anywhere else), see my comment:

const handleColumnsChange = React.useCallback<GridEventListener<'columnsChange'>>(() => 
{
    logger.debug('onColUpdated - GridColumns changed, applying filters');
    const filterModel = gridFilterModelSelector(apiRef);

    const filterableColumnsLookup = gridFilterableColumnLookupSelector(apiRef);

    //--------------------------------------------------------------------
    //HERE: Only items that refer to a present "field" are allowed
    //--------------------------------------------------------------------
    const newFilterItems = filterModel.items.filter(
      (item) => item.field && filterableColumnsLookup[item.field],
    );

    if (newFilterItems.length < filterModel.items.length) {
      apiRef.current.setFilterModel({ ...filterModel, items: newFilterItems });
    }
  }, [apiRef, logger]);

Because my filters are custom, and do not refer to fields present in the definition of columns, I lose the state (i.e. I lose the filterModel.items). You can test the issue here: https://codesandbox.io/p/sandbox/adoring-austin-lgnf2w

Questions

I'd really like to use MUI's native apiRef, so that I don't have to solve the state management of the filterModel myself. Then again, allowing only filters with present fields must be a design decision you've made.

So I guess my first question is, do you think custom filters in the filterModel should be allowed or not?

Thank you!

Your environment 🌎

`npx @mui/envinfo` ``` System: OS: Windows 10 10.0.17763 CPU: (12) x64 Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz Memory: 5.46 GB / 15.94 GB Binaries: Node: 16.14.2 - C:\Program Files\nodejs\node.EXE npm: 8.6.0 - C:\Program Files\nodejs\npm.CMD Managers: Composer: 2.5.8 - C:\ProgramData\ComposerSetup\bin\composer.BAT pip3: 22.1.1 - C:\Program Files\Python310\Scripts\pip3.EXE Utilities: Git: 2.21.0. - C:\Program Files\Git\mingw64\bin\git.EXE FFmpeg: 11.2.0 - C:\Users\Alejandro\AppData\Local\FFMpeg\bin\ffmpeg.EXE Curl: 7.64.0 - C:\Program Files\Git\mingw64\bin\curl.EXE Virtualization: Docker: 19.03.13 - C:\ProgramData\DockerDesktop\version-bin\docker.EXE IDEs: Android Studio: AI-222.4459.24.2221.9971841 Visual Studio: 17.3.32929.385 (Visual Studio Community 2022) Languages: Bash: 4.4.23 - C:\Program Files\Git\usr\bin\bash.EXE Perl: 5.26.2 - C:\Program Files\Git\usr\bin\perl.EXE PHP: 7.4.11 - C:\xampp\php\php.EXE Python: 3.10.4 - C:\Program Files\Python310\python.EXE Browsers: Chrome: 119.0.6045.200 Edge: Spartan (44.17763.1.0) Internet Explorer: 11.0.17763.1 ```

Search keywords: datagrid filters setFilterModel Order ID: 67674

cherniavskii commented 9 months ago

Hey @apedretti Could you clarify the expected behavior? This is what I see in the console:

cherniavskii commented 9 months ago

Also, could you clarify what is the expected behavior when the filterModel includes items referring to the non-existent columns?

apedretti commented 9 months ago

Hi there @cherniavskii,

My bad, I kept testing around and forgot to revert the demo back to the bug. Corrected now: image

Notice how after firing the columnsChange event (STEP 2) the custom filterModel.items get cleared (STEP 3).

Anecdotically, in my CustomDataGrid I've even gotten to fire columnsChange just by changing the filterModel, without even changing the state of the rows.

Solution / patch

I had to keep on going with my development, so what I ended up doing is to:

Expected behavior

I would love to be able to have filters with custom fields (and not have to use a patch), but I feel you guys are on a better position to define what the library should support or not so I'll leave that up to you.

In any case, the options are:

michelengelen commented 9 months ago

@cherniavskii gentle ping on this.

cherniavskii commented 9 months ago

Thanks @apedretti

The reason we only support filtering for the fields that are included in the columns prop is that we need to know the type of the field and filter operators available for the field:

Screenshot 2023-12-21 at 19 59 53 Screenshot 2023-12-21 at 19 59 39

Suppose the field is missing in the column definitions. In that case, it's not safe to assume the type, and keeping these filters in the filter model could lead to various inconsistencies in the Data Grid UI - this is why the Data Grid cleans the filter model and removes non-existent fields.

Your current solution to the problem is straightforward and makes sense to me 👍🏻

cherniavskii commented 9 months ago

Here's a demo if anyone else has a similar use case: https://codesandbox.io/p/sandbox/summer-frost-lwsqyt?file=%2Fsrc%2FDemo.tsx%3A94%2C53

But I have to admit that when filtering by hidden fields, it's not clear why specific rows pass the filtering while others don't. I think this is not great in terms of UX.

cherniavskii commented 9 months ago

I think we can close the issue as there's no action for us to take on the Data Grid side. Thanks for your feedback!

michelengelen commented 9 months ago

Thanks a lot @cherniavskii