[Breadcrumbs] Add ability to change icon used in BreadcrumbCollapsed through slots
#33812
Conversation
Netlify deploy previewhttps://deploy-preview-33812--material-ui.netlify.app/ Bundle size report |
pratikkarad
changed the title
| * } | ||
| */ | ||
| components: PropTypes.shape({ | ||
| collapsed: PropTypes.elementType, |
There was a problem hiding this comment.
| collapsed: PropTypes.elementType, | |
| Collapsed: PropTypes.elementType, |
Also we need a componentsProps prop. Take a look at how these are implemented in other Material UI components, as well as in the @mui/base components. One example component is:
| * The props used for each slot inside. | ||
| * @default {} | ||
| */ | ||
| componentsProps?: { |
There was a problem hiding this comment.
The keys in the componentsProps should be the same as the keys in components (just lowercased).
| const CustomBreadcrumbCollapsedIcon = | ||
| components && components.Collapsed ? components.Collapsed : <React.Fragment />; | ||
|
|
||
| const BreadcrumbCollapsedIcon = styled( |
There was a problem hiding this comment.
This needs to be created outside of the parent component. You can use the as prop for setting up the components.Collapsed.
| <span>first</span> | ||
| <span>second</span> | ||
| <span>third</span> | ||
| <span>fourth</span> | ||
| <span>fifth</span> | ||
| <span>sixth</span> | ||
| <span>seventh</span> | ||
| <span>eighth</span> | ||
| <span>ninth</span> |
There was a problem hiding this comment.
Do we really need tese many elements for the purpose of the test?
|
No I don't think so |
There was a problem hiding this comment.
Check #33797 (comment) We need to come up with a solution to the problem pointed out by @siriwatknp first
michaldudak
added
the
component: breadcrumbs
|
We need to use |
|
Hi @ZeeshanTamboli, let me fix this. |
|
Hi @ZeeshanTamboli, everything looks good but I'm getting the error of sorting the proptypes
Do we have to sort it manually or have a command for that? Also facing the same issue on #33797 |
ZeeshanTamboli
changed the title
BreadcrumbCollapsed
There was a problem hiding this comment.
I pushed the remaining changes. It looks good now. @pratikkarad Thanks for your contribution!
ZeeshanTamboli
changed the title
BreadcrumbCollapsedBreadcrumbCollapsed through slots
I have tested this fix locally and it works as expected.
Fixes #33232