[DataGrid] Merge `page` and `pageSize` props into `paginationModel` #7147

MBilalShafi · 2022-12-08T18:12:06Z

Before: https://codesandbox.io/s/serverpaginationgrid-material-demo-forked-rts4d?file=/demo.tsx
After: https://codesandbox.io/s/serverpaginationgrid-material-demo-with-paginationmodel-0yhefr?file=/demo.tsx

Docs Preview: https://deploy-preview-7147--material-ui-x.netlify.app/x/react-data-grid/pagination/#pagination-model

Changelog

Breaking changes

The page and pageSize props and their respective event handlers onPageChange and onPageSizeChange were removed. Use paginationModel and onPaginationModelChange instead.

-<DataGrid page={page} pageSize={pageSize} onPageChange={handlePageChange} onPageSizeChange={handlePageSizeChange} />
+<DataGrid paginationModel={{ page, pageSize }} onPaginationModelChange={handlePaginationModelChange} />

The property initialState.pagination.page and initialState.pagination.pageSize were also removed. Use initialState.pagination.paginationModel instead.
```
-initialState={{ pagination: { page: 1, pageSize: 10 } }}
+initialState={{ pagination: { paginationModel: { page: 1, pageSize: 10 } } }}
```

The rowsPerPageOptions prop was renamed to pageSizeOptions.

-<DataGrid rowsPerPageOptions={[10, 20, 50]} />
+<DataGrid pageSizeOptions={[10, 20, 50]} />

github-actions · 2022-12-14T22:06:01Z

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

mui-bot · 2022-12-31T06:38:23Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-7147--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	624.8	1,192.4	631.9	841.42	211.091
Sort 100k rows	ms	618.6	1,204.3	618.6	945.32	190.029
Select 100k rows	ms	169.6	294.6	226	231.84	40.172
Deselect 100k rows	ms	174.2	218	192.4	193.58	15.915

Generated by 🚫 dangerJS against a6a7d5a

alexfauquette · 2023-01-02T15:03:05Z

docs/data/data-grid/api-object/UseGridApiRef.tsx

+        <DataGrid
+          {...data}
+          apiRef={apiRef}
+          pagination


Out of scop: pagination is forced to true in the MIT version. I assume it's a legacy from a demo with pro component

Suggested change

pagination

alexfauquette · 2023-01-02T15:08:08Z

docs/data/data-grid/pagination/CursorPaginationGrid.tsx

    }
-  }, [page, isLoading, pageInfo?.nextCursor]);
+  }, [paginationModel, isLoading, pageInfo?.nextCursor]);


Not sure

Suggested change

}, [paginationModel, isLoading, pageInfo?.nextCursor]);

}, [paginationModel.page, isLoading, pageInfo?.nextCursor]);

docs/data/data-grid/pagination/PageSizeInitialState.tsx

alexfauquette · 2023-01-02T15:13:30Z

docs/data/data-grid/pagination/pagination.md


-To initialize the page size without controlling it, provide the page size to the `initialState` prop.
+To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.


Suggested change

To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.

To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.

It allows to specify the `pageSize` and the current `page`.

docs/data/data-grid/pagination/pagination.md

alexfauquette · 2023-01-02T15:23:23Z

packages/grid/x-data-grid/src/components/GridPagination.tsx

-        const newPageSize = Number(event.target.value);
-        apiRef.current.setPageSize(newPageSize);
+        const pageSize = Number(event.target.value);
+        apiRef.current.setPaginationModel({ pageSize, page: paginationState.paginationModel.page });


Would be nice to be able to do the following: specifying only the elements you want to update

Suggested change

apiRef.current.setPaginationModel({ pageSize, page: paginationState.paginationModel.page });

apiRef.current.setPaginationModel({ pageSize });

Same idea for handlePageChange

You've created setPage and setPageSize for those purpose ?

Yes, I kept them for easing up the migration pain a bit. It's opinionated but I was thinking to deprecate them in v6 and remove them in v7 release basically to make the API simple, but they can be helpful too, like in this use-case. 🤔

I don't have a strong opinion about keeping/removing those methods

packages/grid/x-data-grid/src/hooks/features/pagination/gridPaginationInterfaces.ts

alexfauquette · 2023-01-02T15:30:39Z

packages/grid/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

+  const paginationModel =
+    (props.paginationModel ?? props.initialState?.pagination?.paginationModel) ||
+    getDefaultGridPaginationModel(props.autoPageSize);


To allows partial specification in initialState

Suggested change

const paginationModel =

(props.paginationModel ?? props.initialState?.pagination?.paginationModel) ||

getDefaultGridPaginationModel(props.autoPageSize);

const paginationModel = {

...getDefaultGridPaginationModel(props.autoPageSize),

...(props.paginationModel ?? props.initialState?.pagination?.paginationModel)

};

alexfauquette · 2023-01-02T15:33:47Z

packages/grid/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

+  const setPaginationModel = React.useCallback<GridPaginationApi['setPaginationModel']>(
+    (paginationModel) => {
+      const currentModel = gridPaginationModelSelector(apiRef);
+      if (paginationModel === currentModel) {


Since you're comparing object, I doubt the equality will be true. Maybe better to test the values, but it woull expose to bug if we add other properties latter

Suggested change

if (paginationModel === currentModel) {

if (paginationModel.page === currentModel.page && paginationModel.pageSize === currentModel.pageSize) {

Usually, I'm doing a simple equality check in the model updater and doing the smarter checks in each function.
See the filter plugin.

For the pagination, the state is simple enough so that the detailed check is doable.
But as you said, I think it's risky for the future.

I would prefer to keep exposing setPage / setPageSize / setPaginationModel and have each method check what is relevant.
Updating only one of the two is probably the most common scenario for users, so it should not be more complicated to do it than before

packages/grid/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx

packages/grid/x-data-grid/src/hooks/features/pagination/gridPaginationInterfaces.ts

github-actions · 2023-01-05T20:44:25Z

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

m4theushw · 2023-01-05T23:52:45Z

packages/grid/x-data-grid/src/models/events/gridEventLookup.ts

@@ -317,6 +314,7 @@ export interface GridControlledStateReasonLookup {
    | 'deleteFilterItem'
    | 'changeLogicOperator'
    | 'restoreState';
+  pagination: 'setPaginationModel' | 'setPage' | 'setPageSize' | 'stateRestorePreProcessing';


Only "setPaginationModel" and "stateRestorePreProcessing" are being effectively used. Although we have "setPageSize", setPageSize() calls setPaginationModel() that uses its own reason. I think if nobody asked for the reason we can leave it as undefined.

Suggested change

pagination: 'setPaginationModel' | 'setPage' | 'setPageSize' | 'stateRestorePreProcessing';

Removing this key totally throws ts error while using apiRef.current.updateControlState as it's typed to accept keys defined in GridControlledStateReasonLookup.
How about keeping the valid ones?

pagination: 'setPaginationModel' | 'stateRestorePreProcessing';

Ok to keep only the valid ones.

packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx

packages/grid/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

packages/grid/x-data-grid/src/hooks/features/pagination/gridPaginationInterfaces.ts

github-actions · 2023-01-11T21:13:57Z

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

github-actions · 2023-01-16T12:13:57Z

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

m4theushw · 2023-01-16T19:44:58Z

docs/data/data-grid/pagination/pagination.md

-To initialize the page size without controlling it, provide the page size to the `initialState` prop.
+The pagination model is an object containing the current page and the size of the page. The default value is `{ page: 0, pageSize: 100 }`. To change the default value, make it controlled by `paginationModel` prop or initialize a custom value using `initialState.pagination.paginationModel`.
+
+### Initialize the pagination model


Suggested change

### Initialize the pagination model

### Initializing the pagination model

m4theushw · 2023-01-16T19:46:03Z

docs/data/data-grid/pagination/pagination.md

+const [paginationModel, setPaginationModel] = React.useState({
+  pageSize: 25,
+  page: 0,
+});
 <DataGrid


Suggested change

<DataGrid

<DataGrid

docs/data/data-grid/pagination/pagination.md

docs/data/data-grid/api-object/UseGridApiContext.js

docs/data/data-grid/pagination/PaginationModelInitialState.tsx.preview

docs/data/data-grid/pagination/pagination.md

docs/data/data-grid/row-selection/MultipleRowSelectionGrid.js

docs/data/data-grid/state/UseGridSelector.tsx

packages/grid/x-data-grid/src/hooks/features/pagination/gridPaginationSelector.ts

packages/grid/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

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

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

DanailH · 2023-01-17T08:07:28Z

Looks great!

MBilalShafi added breaking change component: data grid This is the name of the generic UI component, not the React module! labels Dec 8, 2022

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

Implement paginationModel and onPaginationModelChange

2a7fb2a

MBilalShafi force-pushed the pagination-model branch from f42e1b5 to 2a7fb2a Compare December 31, 2022 06:33

Refinement

3a1e4f0

MBilalShafi added the codemod candidate Suitable for writing a codemod for label Dec 31, 2022

MBilalShafi added 2 commits December 31, 2022 13:25

Minor fix

179c3e0

Merge next into 'pagination-model'

ddae38b

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

MBilalShafi requested review from cherniavskii, m4theushw, flaviendelangle, alexfauquette and DanailH December 31, 2022 09:34

MBilalShafi marked this pull request as ready for review December 31, 2022 09:34

alexfauquette reviewed Jan 2, 2023

View reviewed changes

MBilalShafi added 4 commits January 4, 2023 16:00

Resolve a few of Alex's comments

c6efaab

Fix incorrect comment

e10e4b4

Update docs a bit

e27e48b

Fix a few tests

fb10647

DanailH reviewed Jan 5, 2023

View reviewed changes

packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx Show resolved Hide resolved

DanailH reviewed Jan 5, 2023

View reviewed changes

packages/grid/x-data-grid/src/hooks/features/pagination/gridPaginationInterfaces.ts Show resolved Hide resolved

Fix a lot of tests

3460160

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

m4theushw reviewed Jan 6, 2023

View reviewed changes

'rowsPerPageOptions' to 'pageSizeOptions'

49dc073

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

Merge 'next' into 'pagination-model'

d10551c

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

MBilalShafi mentioned this pull request Jan 12, 2023

[DataGrid] Add codemods for v6 breaking changes #7086

Closed

16 tasks

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

MBilalShafi added 3 commits January 16, 2023 18:34

Revert 255a0bc

c15fa56

docs:api

369f647

Update migration guide

e6c1781

MBilalShafi self-assigned this Jan 16, 2023

Merge 'next' into pagination-model

6a2287a

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

Update dep for cursor demo

ed8158b

MBilalShafi requested review from DanailH, alexfauquette and m4theushw January 16, 2023 14:49

m4theushw approved these changes Jan 16, 2023

View reviewed changes

cherniavskii reviewed Jan 16, 2023

View reviewed changes

MBilalShafi added 2 commits January 17, 2023 11:49

Adress comments

370f014

Refactor selectors

36532a9

DanailH approved these changes Jan 17, 2023

View reviewed changes

MBilalShafi added 3 commits January 17, 2023 14:52

Some refactoring as per PR comments

cc982d4

Fix argos

09978f9

Update a demo

a6a7d5a

cherniavskii approved these changes Jan 17, 2023

View reviewed changes

MBilalShafi merged commit afb47b2 into mui:next Jan 17, 2023

MBilalShafi deleted the pagination-model branch January 17, 2023 17:12

MBilalShafi mentioned this pull request Jan 18, 2023

[codemod] Rename rowsPerPageOptions prop to pageSizeOptions #7603

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Merge `page` and `pageSize` props into `paginationModel` #7147

[DataGrid] Merge `page` and `pageSize` props into `paginationModel` #7147

MBilalShafi commented Dec 8, 2022 •

edited

Loading

github-actions bot commented Dec 14, 2022

mui-bot commented Dec 31, 2022 •

edited

Loading

alexfauquette Jan 2, 2023

alexfauquette Jan 2, 2023

alexfauquette Jan 2, 2023

alexfauquette Jan 2, 2023

alexfauquette Jan 2, 2023

MBilalShafi Jan 4, 2023 •

edited

Loading

alexfauquette Jan 4, 2023

alexfauquette Jan 2, 2023

alexfauquette Jan 2, 2023

flaviendelangle Jan 4, 2023

github-actions bot commented Jan 5, 2023

m4theushw Jan 5, 2023

MBilalShafi Jan 6, 2023

m4theushw Jan 10, 2023

github-actions bot commented Jan 11, 2023

github-actions bot commented Jan 16, 2023

m4theushw Jan 16, 2023 •

edited

Loading

m4theushw Jan 16, 2023

DanailH commented Jan 17, 2023

	}, [paginationModel, isLoading, pageInfo?.nextCursor]);
	}, [paginationModel.page, isLoading, pageInfo?.nextCursor]);


		To initialize the page size without controlling it, provide the page size to the `initialState` prop.
		To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.

	To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.
	To initialize the pagination model without controlling it, provide the `paginationModel` to the `initialState` prop.
	It allows to specify the `pageSize` and the current `page`.

	apiRef.current.setPaginationModel({ pageSize, page: paginationState.paginationModel.page });
	apiRef.current.setPaginationModel({ pageSize });

	if (paginationModel === currentModel) {
	if (paginationModel.page === currentModel.page && paginationModel.pageSize === currentModel.pageSize) {

	### Initialize the pagination model
	### Initializing the pagination model

[DataGrid] Merge page and pageSize props into paginationModel #7147

[DataGrid] Merge page and pageSize props into paginationModel #7147

Conversation

MBilalShafi commented Dec 8, 2022 • edited Loading

Changelog

Breaking changes

github-actions bot commented Dec 14, 2022

mui-bot commented Dec 31, 2022 • edited Loading

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

MBilalShafi Jan 4, 2023 • edited Loading

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

github-actions bot commented Jan 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 11, 2023

github-actions bot commented Jan 16, 2023

m4theushw Jan 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanailH commented Jan 17, 2023

[DataGrid] Merge `page` and `pageSize` props into `paginationModel` #7147

[DataGrid] Merge `page` and `pageSize` props into `paginationModel` #7147

MBilalShafi commented Dec 8, 2022 •

edited

Loading

mui-bot commented Dec 31, 2022 •

edited

Loading

MBilalShafi Jan 4, 2023 •

edited

Loading

m4theushw Jan 16, 2023 •

edited

Loading