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 reason to onFilterModelChange #4938

Merged
merged 5 commits into from
Jun 7, 2022

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented May 19, 2022

Fixes #1803

Always when someone asks how to know why a specific model has changed, we suggest to pass a reason argument. We had a few moments where this suggestion was brought: #3516 (comment), #4160 (comment), #1803 (comment), #1141 (comment). This PR implements the foundation to pass a reason param to the callback props and the events published by the controllable states. To give a concrete example, I added it to onFilterModelChange but the other states will also receive it in follow-up PRs.

The high number of files changed is because I renamed the method below. The "update" word didn't make sense where it was being used, but I needed it for the new unstable_updateControlState.

-apiRef.current.unstable_updateControlState
+apiRef.current.unstable_registerControlState

From now on, to update a sub-state that is controlled (e.g. filtering) we need to use unstable_updateControlState instead of setState. This new method has the reason argument and correctly types the options available. In the future, setState will be used only for non-controlled states.

@mui-bot
Copy link

mui-bot commented May 19, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 251 512.5 315.9 353.44 97.493
Sort 100k rows ms 450.6 805.6 754.6 712.88 133.027
Select 100k rows ms 111.2 216.7 176.3 165.82 35.419
Deselect 100k rows ms 109.6 230.2 177.2 165.48 44.568

Generated by 🚫 dangerJS against cffff89

/**
* Additional details passed to the callbacks
*/
export interface GridCallbackDetails {
export interface GridCallbackDetails<K extends keyof GridControlledStateReasonLookup = any> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Once apiRef.current.unstable_updateControlState stops being only a wrapper for setState we can replace the any with the correct types. The any is necessary because we don't know which key of the state was changed to match with GridControlledStateReasonLookup.

@m4theushw m4theushw added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request feature: Filtering Related to the data grid Filtering feature labels May 20, 2022
@m4theushw m4theushw self-assigned this May 20, 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 May 23, 2022
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels May 25, 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 removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 7, 2022
@m4theushw m4theushw merged commit 22b7dd4 into mui:master Jun 7, 2022
@m4theushw m4theushw deleted the add-reason-to-callback-prop branch June 7, 2022 21:05
joserodolfofreitas pushed a commit to joserodolfofreitas/mui-x that referenced this pull request Jun 13, 2022
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
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! feature: Filtering Related to the data grid Filtering feature new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Distinguish removed filter in onFilterModelChange
3 participants