[DataGrid] Refactor: create base Pagination #16759

romgrk · 2025-02-27T22:36:53Z

Part of the design-system agnostic work.

romgrk · 2025-02-27T22:38:34Z

packages/x-data-grid/src/models/gridBaseSlots.ts

+  getItemAriaLabel?: (type: 'first' | 'last' | 'next' | 'previous') => string;
+  labelRowsPerPage?: string;
+  labelDisplayedRows?: (info: {
+    from: number;
+    to: number;
+    count: number;
+    page: number;
+  }) => React.ReactNode | undefined;


Having this here is ugly, but I don't see how to avoid it.

Tbd: move pagination locales from the core to here.

Having this here is ugly, but I don't see how to avoid it.

Alternatively, we can use apiRef.current.getLocaleText() instead of passing the prop.
But passing a prop looks good to me too.

I've handled l10n in the material bindings, other design-systems can easily fork that code and it removes l10n props from the typings.

mui-bot · 2025-02-27T22:40:36Z

Localization writing tips ✍️

Seems you are updating localization 🌍 files.

Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️

Verify if the PR title respects the release format. Here are two examples (depending if you update or add a locale file)

[l10n] Improve Swedish (sv-SE) locale
[l10n] Add Danish (da-DK) locale
Update the documentation of supported locales by running pnpm l10n
Clean files with pnpm prettier.

Deploy preview: https://deploy-preview-16759--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against c60bb4e

romgrk · 2025-02-27T23:03:31Z

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

      {...locales}
      labelDisplayedRows={wrappedLabelDisplayedRows}
-      {...props}


I've had to remove ...props in some places and it makes it imposible for users to use e.g. rootProps.slotProps.pagination to customize the slot, but we can't both be design-system agnostic and pass any material-ui prop.

The solution here would be for users to fork the basePagination slot and customize it there.

I've had to remove ...props in some places and it makes it imposible for users to use e.g. rootProps.slotProps.pagination to customize the slot

Why though? What is different about the Pagination slot?

I meant: it's not going to be possible to pass material props via slotProps.pagination.

I meant: it's not going to be possible to pass material props via slotProps.pagination.

Ahh, only via slotProps.basePagination?

The problem is that base slots are not material slots anymore, they're design-system agnostic. For some of them, it might happen that the grid's base slot API matches the material API, but it's not consistent. I have been thinking about adding something like slotProps.raw to all base components to pass custom props to the underlying design-system, be it material or something else.

cherniavskii · 2025-03-03T22:13:08Z

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

      {...locales}
      labelDisplayedRows={wrappedLabelDisplayedRows}
-      {...props}


I've had to remove ...props in some places and it makes it imposible for users to use e.g. rootProps.slotProps.pagination to customize the slot

Why though? What is different about the Pagination slot?

cherniavskii · 2025-03-03T22:19:09Z

packages/x-data-grid/src/models/gridSlotsComponent.ts

+   * The custom Pagination component used in the grid.
+   * @default Pagination
+   */
+  basePagination: React.JSXElementConstructor<GridSlotProps['basePagination']>;


Why rename the slot?
Regarding the base prefix, we seem to only prefix "primitive" components used in multiple places.
Pagination is a one-off component that is closer to the filter panel than to a "primitive" component.

We can't use @mui/material/TablePagination in the core. If we still want to use it in the material grid, we need it as a base component. The alternative would be to re-implement the pagination in the core grid using the available base components (button, select, etc).

OK, so the base prefix now means "design-agnostic slot", correct?

github-actions · 2025-03-11T04:56:48Z

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

github-actions · 2025-03-11T11:47:14Z

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

michelengelen · 2025-03-21T09:36:52Z

@romgrk shouldn't we spread the slotProps onto the basePagination as well to allow for slots overrides?

--- a/packages/x-data-grid/src/components/GridPagination.tsx
+++ b/packages/x-data-grid/src/components/GridPagination.tsx
@@ -102,6 +102,7 @@ function GridPagination() {
       onPageChange={handlePageChange}
       onRowsPerPageChange={handlePageSizeChange as any}
       disabled={disabled}
+      {...(rootProps.slotProps?.basePagination ?? {})}
     />
   );
 }

This is regarding issue #17045

cc @cherniavskii

romgrk · 2025-03-21T20:07:35Z

That would not solve the user's issue. They're trying to pass material-ui props to a grid base component which has a different set of props. We might want to introduce slotProps.native to pass props to the underlying design-system.

refactor: create base Pagination

dd22eca

romgrk added the component: data grid This is the name of the generic UI component, not the React module! label Feb 27, 2025

romgrk requested a review from a team February 27, 2025 22:36

romgrk commented Feb 27, 2025

View reviewed changes

romgrk added 3 commits February 27, 2025 18:00

refactor: remove props destructuring

737ebc3

lint

ed2a39c

lint

b48b7b9

romgrk commented Feb 27, 2025

View reviewed changes

romgrk added 2 commits February 28, 2025 15:01

lint

45a6bec

refactor: remove material props & classes

6ed580f

cherniavskii reviewed Mar 3, 2025

View reviewed changes

cherniavskii approved these changes Mar 4, 2025

View reviewed changes

romgrk added 6 commits March 5, 2025 18:09

lint

fdab8be

lint

963a312

refactor: remove core locales

3bdfe23

lint

8fef388

lint

a4bdb02

refactor: import more core locales

0db6d46

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

Merge branch 'master' into refactor-agnostic-pagination

505159f

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

lint

7946bbd

cherniavskii approved these changes Mar 11, 2025

View reviewed changes

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

romgrk added 2 commits March 11, 2025 14:41

lint

c9d12b3

Merge branch 'master' into refactor-agnostic-pagination

c60bb4e

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

romgrk merged commit ef26339 into mui:master Mar 11, 2025
18 checks passed

romgrk deleted the refactor-agnostic-pagination branch March 11, 2025 19:12

michelengelen mentioned this pull request Mar 20, 2025

[data grid] In v8.0.0-beta.0 pagination slotProps type became empty #17045

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Refactor: create base Pagination #16759

[DataGrid] Refactor: create base Pagination #16759

romgrk commented Feb 27, 2025

romgrk Feb 27, 2025

romgrk Mar 3, 2025

cherniavskii Mar 3, 2025

romgrk Mar 11, 2025

mui-bot commented Feb 27, 2025 •

edited

Loading

romgrk Feb 27, 2025

cherniavskii Mar 3, 2025

romgrk Mar 4, 2025

cherniavskii Mar 4, 2025

romgrk Mar 5, 2025

cherniavskii Mar 3, 2025

cherniavskii Mar 3, 2025

romgrk Mar 3, 2025

cherniavskii Mar 4, 2025

github-actions bot commented Mar 11, 2025

github-actions bot commented Mar 11, 2025

michelengelen commented Mar 21, 2025

romgrk commented Mar 21, 2025

[DataGrid] Refactor: create base Pagination #16759

[DataGrid] Refactor: create base Pagination #16759

Conversation

romgrk commented Feb 27, 2025

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

mui-bot commented Feb 27, 2025 • edited Loading

Localization writing tips ✍️

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 11, 2025

github-actions bot commented Mar 11, 2025

michelengelen commented Mar 21, 2025

romgrk commented Mar 21, 2025

mui-bot commented Feb 27, 2025 •

edited

Loading