[DataGrid] Allow to control the columns visibility #3554

flaviendelangle · 2022-01-05T16:18:13Z

Closes #2198
Closes #1412
Part of #3448

Goals

Allow to control the visible columns just like the other models visibleColumnsModel + onVisibleColumnsModelChange
Do not break existing code with GridColDef.hide even for users switching manually the visibility with apiRef.current.updateColumns (add a temporary sync from the columns to the model)

Advancement

Add visibleColumnsModel and onVisibleColumnsModelChange
When visibleColumnsModel is not controlled, generate it from GridColDef.hide
Add visibleColumnsModel to the initialState prop (for [DataGrid] Add other sub-states to the initialState prop #3448)
Deprecate onColumnVisibilityChange and only use it when the update comes from GridColDef.hide
Deprecate GridColDef.hide
Document
Add tests for controlled / initialState behavior
Duplicate all tests using GridColDef.hide to also test with the model

Release note

🙈 Allow to control the columns visibility ([DataGrid] Allow to control the columns visibility #3554) @flaviendelangle

// To fully control
<DataGrid
  columnVisibilityModel={columnVisibilityModel}
  onColumnVisilibilityModelChange={newModel => setColumnVisibilityModel(newModel)}
/>

// To initialize without controlling
<DataGrid
  initialState={{
    columns: {
      columnVisibilityModel
    }
  }}
/>

See the documentation for more details.

The hide property from GridColDef still works but has been deprecated.

github-actions · 2022-01-07T10:21:41Z

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

github-actions · 2022-01-10T09:32:52Z

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

m4theushw

I'm not sure if by allowing the visible columns to be controlled it won't conflict with other features in the future. I remember that with editRowsModel it caused a lot of problems, since we lost the warranty that the value would be updated synchronously. Now we may need to wait for the next render.

Should visibleColumnsModel receive an object where each key value is true (visible) or false (hidden)? It could be renamed to columnVisibilityModel in this case. With an array, it becomes a pain to hide a specific column. Also, the object doesn't require all fields to be present. Users might be upset when we drop hide and require them to pass all the visible fields.

I noticed a bug if I have checkboxSelection={false} then I change it to checkboxSelection={true}. The checkbox column is never visible nor onVisibleColumnsModelChange is called. See this CodeSandbox.

docs/src/pages/components/data-grid/columns/columns.md

m4theushw · 2022-01-11T14:43:48Z

packages/grid/x-data-grid-pro/src/tests/columnsVisibility.DataGridPro.test.tsx

+      it('should update `visibleColumnsModel` but not `GridColDef.hide` in state', () => {
+        render(<TestDataGridPro initialState={{ columns: { visibleColumnsModel: ['id'] } }} />);
+        apiRef.current.setColumnVisibility('id', false);
+        expect(apiRef.current.state.columns.lookup.id.hide).to.equal(false);


We shouldn't be making assertions on the state, they may change while yielding the same result for the user.

I can listen to columnChange events to get the same result if we really want to avoid state assertions.

You can also check if columnVisibilityChange was not called. My point is that we don't recommend direct state access, but to use selectors or API methods instead. That way, the test should ensure that if someone access the state via one of the recommended ways it will return the correct value. We shouldn't be testing the internal implementation, but what is one the public API or can be seen (UI).

Then I'll use the selector.
It won't impact the end user but users that use the selectors could have problems if we update the hide field by mistake.

packages/grid/x-data-grid-pro/src/tests/columnsVisibility.DataGridPro.test.tsx

packages/grid/x-data-grid/src/tests/columnsVisibility.DataGrid.test.tsx

m4theushw · 2022-01-11T15:14:06Z

packages/grid/x-data-grid/src/tests/layout.DataGrid.test.tsx

@@ -147,9 +147,9 @@ describe('<DataGrid /> - Layout & Warnings', () => {

      it('should support columns.valueGetter using `getValue` (deprecated)', () => {
        const columns = [
-          { field: 'id', hide: true },


Should this test be reverted to ensure that the old API still works?

As valueGetter is deprecated, I chose to switch this test to the new behavior without keeping the old one.

But I could duplicate it and test both valueGetter + hide and valueGetter + columnVisibilityModel.

packages/grid/_modules_/grid/hooks/features/columns/useGridColumns.ts

flaviendelangle · 2022-01-12T07:46:55Z

I'm not sure if by allowing the visible columns to be controlled it won't conflict with other features in the future. I remember that with editRowsModel it caused a lot of problems, since we lost the warranty that the value would be updated synchronously. Now we may need to wait for the next render.

I feel like this one is closer to the other model we already have like selectionModel or filterModel.
A method call or the UI try to update the model => The user can update to controlled prop if he wants => The UI react to the prop model update.
I agree that the question is worth asking though.

Should visibleColumnsModel receive an object where each key value is true (visible) or false (hidden)? It could be renamed to columnVisibilityModel in this case. With an array, it becomes a pain to hide a specific column. Also, the object doesn't require all fields to be present. Users might be upset when we drop hide and require them to pass all the visible fields.

It is one of the main point I would like to discuss yes.
I did an array to follow the selectionModel format, but the more I think about it, the more I like the object, because we can treat the missing columns in the model as visible by default.

m4theushw · 2022-01-12T12:23:26Z

A method call or the UI try to update the model => The user can update to controlled prop if he wants => The UI react to the prop model update.

Yeah, once method or UI try to update the model we stop any further action. The side effects should be handled inside an effect to avoid the problems of editRowsModel with singleSelect.

I did an array to follow the selectionModel format, but the more I think about it, the more I like the object, because we can treat the missing columns in the model as visible by default.

I'm in favor of using an object to ease the transition for users. Internally, the array is converted to an object, so there's one step we can remove here. In the case of selectionModel doesn't make sense to explicitly define that a row is not selected, so an array is used.

…vior

github-actions · 2022-01-18T12:35:59Z

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

docs/src/pages/components/data-grid/columns/columns.md

packages/grid/_modules_/grid/models/events/gridEvents.ts

m4theushw · 2022-01-18T14:28:51Z

packages/grid/_modules_/grid/hooks/features/columns/useGridColumns.ts

  useGridStateInit(apiRef, (state) => {
    const columnsState = createColumnsState({
      apiRef,
      columnsTypes,
      columnsToUpsert: props.columns,
+      shouldRegenColumnVisibilityModelFromColumns: !shouldUseVisibleColumnModel,
+      currentColumnVisibilityModel:
+        props.columnVisibilityModel ?? props.initialState?.columns?.columnVisibilityModel ?? {},


By default, the id column in the Commodity dataset is hidden. However, in https://deploy-preview-3554--material-ui-x.netlify.app/components/data-grid/columns/#column-visibility, all demos that use this dataset and have an initial state or the columnVisibilityModel prop, the id column is visible. With that being said, should the initial state be created also from the columns with hide? In controlled mode I guess there's nothing we can do, unless include the id column in the object.

I should hide manually the id column on the demo I think.
I would advise against using the hide field when the model is defined. The way I built it, the hide field is deprecated. If we use the model, it is not taken into account at all.
That way it is not breaking, but users won't be able to do dirty tricks with both the hide and models.

Note that the brokerId column is also hidden in the dataset. This makes me think if wouldn't be better to include a default initialState={{ columns: { columnVisibilityModel: { id: false } } }} below. Once we drop hide it will break all demos.

https://github.com/mui-org/material-ui-x/blob/f581ac12a3dfc9097f3877b3334a935b5fdbfdac/packages/grid/x-data-grid-generator/src/services/tree-data-generator.ts#L109

Demo will have to manually pass it to the initialState when they override it
But yes better do it now, we will forget if we don't

Is it planned to remove the shouldUseVisibleColumnModel in v6?

Yes, in v6 we would always use the model, the hide field would not exist anymore.

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

flaviendelangle · 2022-01-18T17:38:50Z

I have 2 Argos diff, I will try to fix them tomorrow

packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx

docs/src/pages/components/data-grid/columns/columns.md

packages/grid/_modules_/grid/hooks/features/columns/gridColumnsUtils.ts

alexfauquette · 2022-01-19T11:19:50Z

packages/grid/_modules_/grid/hooks/features/columns/useGridColumns.ts

  useGridStateInit(apiRef, (state) => {
    const columnsState = createColumnsState({
      apiRef,
      columnsTypes,
      columnsToUpsert: props.columns,
+      shouldRegenColumnVisibilityModelFromColumns: !shouldUseVisibleColumnModel,
+      currentColumnVisibilityModel:
+        props.columnVisibilityModel ?? props.initialState?.columns?.columnVisibilityModel ?? {},


Is it planned to remove the shouldUseVisibleColumnModel in v6?

packages/grid/x-data-grid-pro/src/tests/columnsVisibility.DataGridPro.test.tsx

[DataGrid] Add controlled behavior for the visible columns

0db55f7

flaviendelangle self-assigned this Jan 5, 2022

flaviendelangle added the component: data grid This is the name of the generic UI component, not the React module! label Jan 5, 2022

flaviendelangle added 7 commits January 6, 2022 10:20

Work

668e448

Add tests

95d254e

Fix

0d807a6

Gen docs & proptypes

91705fd

Merge branch 'master' into columns-visibility

5dc95c8

Add docsé

229f05f

Add comment

17e2393

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

Merge

c33f10a

flaviendelangle marked this pull request as draft January 7, 2022 12:37

flaviendelangle added the new feature New feature or request label Jan 7, 2022

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

flaviendelangle marked this pull request as ready for review January 10, 2022 08:06

flaviendelangle requested a review from alexfauquette January 10, 2022 08:06

flaviendelangle added 2 commits January 10, 2022 09:07

Merge branch 'master' into columns-visibility

ac7e2ec

Add test

f8cc7e5

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

Merge

5f75d40

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

flaviendelangle requested a review from m4theushw January 11, 2022 08:06

m4theushw reviewed Jan 11, 2022

View reviewed changes

flaviendelangle requested a review from m4theushw January 12, 2022 08:17

Rename visibleColumnsModel into columnVisibilityModel and change beha…

fb0e89a

…vior

Work

d559952

flaviendelangle mentioned this pull request Jan 18, 2022

[DataGridPro] What is the configuration to export all table rows? #3610

Closed

2 tasks

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

m4theushw reviewed Jan 18, 2022

View reviewed changes

flaviendelangle and others added 4 commits January 18, 2022 16:03

Update packages/grid/_modules_/grid/models/events/gridEvents.ts

b981249

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update docs/src/pages/components/data-grid/columns/columns.md

b6592bd

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update packages/grid/_modules_/grid/models/events/gridEvents.ts

0630365

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update docs/src/pages/components/data-grid/columns/columns.md

99f6df0

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

flaviendelangle requested a review from m4theushw January 18, 2022 15:06

Merge

13a83d5

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

flaviendelangle added 6 commits January 18, 2022 16:09

Work

972f781

Code review: Matheus (demo use the columnVisibilityModel

365acb4

Fix

571d0fe

Fix priority doc

a832f7a

Fix

1904e78

Fix

9c1bc66

flaviendelangle added 3 commits January 19, 2022 10:13

Fix

bf72062

Fix demos

f84f456

Work

ddce44f

alexfauquette approved these changes Jan 19, 2022

View reviewed changes

m4theushw approved these changes Jan 19, 2022

View reviewed changes

flaviendelangle added 2 commits January 19, 2022 16:51

Code review: Alexandre

c7a0540

Code review: Alexandre

ee33918

flaviendelangle merged commit b8c1f08 into mui:master Jan 19, 2022

flaviendelangle deleted the columns-visibility branch January 19, 2022 16:23

flaviendelangle changed the title ~~[DataGrid] Add controlled behavior for the visible columns~~ [DataGrid] Allow to control the columns visibility Jan 20, 2022

alexfauquette mentioned this pull request Jan 20, 2022

[DataGrid] Need help on Customising the Column menu and Multi-column Sorting #3673

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Allow to control the columns visibility #3554

[DataGrid] Allow to control the columns visibility #3554

flaviendelangle commented Jan 5, 2022 •

edited

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 10, 2022

m4theushw left a comment

m4theushw Jan 11, 2022

flaviendelangle Jan 12, 2022

m4theushw Jan 12, 2022

flaviendelangle Jan 12, 2022 •

edited

m4theushw Jan 11, 2022

flaviendelangle Jan 12, 2022

flaviendelangle commented Jan 12, 2022 •

edited

m4theushw commented Jan 12, 2022 •

edited

github-actions bot commented Jan 18, 2022

m4theushw Jan 18, 2022 •

edited

flaviendelangle Jan 18, 2022

m4theushw Jan 18, 2022

flaviendelangle Jan 18, 2022

flaviendelangle Jan 18, 2022

alexfauquette Jan 19, 2022

flaviendelangle Jan 19, 2022

flaviendelangle commented Jan 18, 2022 •

edited

alexfauquette Jan 19, 2022

[DataGrid] Allow to control the columns visibility #3554

[DataGrid] Allow to control the columns visibility #3554

Conversation

flaviendelangle commented Jan 5, 2022 • edited

Goals

Advancement

Release note

github-actions bot commented Jan 7, 2022

github-actions bot commented Jan 10, 2022

m4theushw 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

flaviendelangle Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle commented Jan 12, 2022 • edited

m4theushw commented Jan 12, 2022 • edited

github-actions bot commented Jan 18, 2022

m4theushw Jan 18, 2022 • edited

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

Choose a reason for hiding this comment

flaviendelangle commented Jan 18, 2022 • edited

Choose a reason for hiding this comment

flaviendelangle commented Jan 5, 2022 •

edited

flaviendelangle Jan 12, 2022 •

edited

flaviendelangle commented Jan 12, 2022 •

edited

m4theushw commented Jan 12, 2022 •

edited

m4theushw Jan 18, 2022 •

edited

flaviendelangle commented Jan 18, 2022 •

edited