[TablePagination][base] Improve actions type in slotProps
#36458
Conversation
Netlify deploy previewhttps://deploy-preview-36458--material-ui.netlify.app/ Bundle size report |
| @@ -67,5 +67,11 @@ const styledTablePagination = ( | |||
| toolbar: Toolbar, | |||
| spacer: Spacer, | |||
| }} | |||
| slotProps={{ | |||
There was a problem hiding this comment.
Test , for the change
zannager
added
package: base-ui
hbjORbj
changed the title
actions type in slotPropsactions type in slotProps
There was a problem hiding this comment.
This change makes a lot of sense to me. We shouldn't require people to add overrides to TablePaginationUnstyledActionsSlotPropsOverrides if the default unstyled component is used. cc @michaldudak what do you think about this? If we agree on this, we would probably need to udpate some other slot types as well.
| @@ -12,7 +13,8 @@ export interface LabelDisplayedRowsArgs { | |||
| export type ItemAriaLabelType = 'first' | 'last' | 'next' | 'previous'; | |||
|
|
|||
| export interface TablePaginationUnstyledRootSlotPropsOverrides {} | |||
| export interface TablePaginationUnstyledActionsSlotPropsOverrides {} | |||
| export interface TablePaginationUnstyledActionsSlotPropsOverrides | |||
There was a problem hiding this comment.
The Overrides interface should be empty (by convention). If you'd like to specify the type of the slotProps field, define slotProps.actions as actions?: SlotComponentProps<typeof TablePaginationActionsUnstyled, TablePaginationUnstyledActionsSlotPropsOverrides, TablePaginationUnstyledOwnerState>, similarly to how SelectUnstyled defines its slotProps.popper.
The first generic parameter to SlotComponentProps defines the type of a component whose props will be accessible to provide to a slot.
There was a problem hiding this comment.
@michaldudak Ah okay wasn't aware of convention. I've applied your suggestion in this commit bb5aa2f
packages/mui-base/src/TablePaginationUnstyled/TablePaginationActionsUnstyled.tsx
Show resolved
Hide resolved
|
Could you please merge in the latest master to ensure the tests are run on the current codebase? |
| @@ -1,6 +1,8 @@ | |||
| import * as React from 'react'; | |||
| import { OverrideProps } from '@mui/types'; | |||
| import { SlotComponentProps } from '../utils'; | |||
| import TablePaginationActionsUnstyled from './TablePaginationActionsUnstyled'; | |||
| import * as CommonTypes from './common.types'; | |||
There was a problem hiding this comment.
Doesn't import { ItemAriaLabelType } from './common.types' work? It would be more inline with our other imports.
There was a problem hiding this comment.
@michaldudak it won't work, since ItemAriaLabelType is already defined here https://github.com/mui/material-ui/pull/36458/files#diff-4068bfcfe4d8f607231754518309840d09efc1b3bac8a119478a9c1fe984541fR14
There was a problem hiding this comment.
It doesn't have to be, does it?
If you import { ItemAriaLabelType } from './common.types'; line 14 can be deleted. Or am I missing something?
There was a problem hiding this comment.
Wouldn't it be breaking change if export ItemAriaLabelType is deleted from TablePaginationUnstyled.types.ts?
cc @michaldudak
There was a problem hiding this comment.
Resolved in this commit 2494ff7
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
before:
after :
codesandbox after: https://codesandbox.io/s/k4s8qy?file=/demo.tsx