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

[docs] Add warning clarifications #5399

Merged
merged 3 commits into from
Jul 25, 2022
Merged

Conversation

alexfauquette
Copy link
Member

I struggled to understand a warning in row grouping documentation about useKeepGroupedColumnsHidden

This hook is only compatible with the deprecated column property hide or with the controlled columnVisibilityModel prop.

You must use the columnVisibilityModel in the initialState instead.

Not sure my correction is exactly what the hook does

@mui-bot
Copy link

mui-bot commented Jul 5, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 258.9 603.7 485.4 443.52 136.712
Sort 100k rows ms 518.8 991.5 711.9 756.52 160.698
Select 100k rows ms 188.6 279.5 209.3 216.5 32.565
Deselect 100k rows ms 118.1 208.1 181.5 163.38 35.388

Generated by 🚫 dangerJS against f4a1f26

@@ -142,9 +142,9 @@ return (
```

:::warning
This hook is only compatible with the deprecated column property `hide` or with the controlled `columnVisibilityModel` prop.
This hook is not compatible with the deprecated column property `hide`, and you should not control the `columnVisibilityModel`.
Copy link
Member

Choose a reason for hiding this comment

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

Wow my sentence made no sense...
It should be compatible with the controlled columnVisibilityModel prop if you also pass its value to the hook.

See the RowGroupingCustomGroupingColDefCallback example

This hook is not compatible with the deprecated column property `hide`, and you should not control the `columnVisibilityModel`.
This hook is not compatible with the deprecated column property `hide`.
It can be used with controlled `columnVisibilityModel` and `initialState`.
To do so, provide the controlled value and the initial state to the hook as follow:
Copy link
Member

Choose a reason for hiding this comment

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

You just have to provide one of the two, and the example below is redundant with what is currently existing

:::

Bellow are two examples about how to use `columnVisibilityModel`, or `initialState` with `useKeepGroupedColumnsHidden` hook.
You can mix the two examples to support both at the same time.
Copy link
Member

Choose a reason for hiding this comment

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

In which scenario would you use both an initial state and a control state ? The control state would totally override the initial state right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it was in the second example O:)

If I want to use initialState to initialize the filterModel and I don't want to bother myself with mixing it with the initialState generated by the hook

Copy link
Member

Choose a reason for hiding this comment

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

Oh ok, I thought you were talking about setting the same model in both control and initial mode

@alexfauquette alexfauquette merged commit 06bd76e into mui:master Jul 25, 2022
alexfauquette added a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
* [docs] Add warning clarifications

* fix

* try to clarify examples
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants