-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[material-ui][Checkbox] Add slots and slotProps #44974
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-44974--material-ui.netlify.app/ Checkbox: parsed: +1.98% , gzip: +1.85% Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not following, what's the use case to have?
slotProps: PropTypes.shape({
root: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
}),
slotProps, | ||
}; | ||
|
||
const [RootSlot, rootProps] = useSlot('root', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the runtime cost of this for the component?
I'm not entirely sure about adding slots and SlotProps if a component only has a root slot; maybe it's just for consistency. @siriwatknp is my understanding correct? |
export interface CheckboxSlots { | ||
/** | ||
* The component that renders the root slot. | ||
* @default SwitchBase | ||
*/ | ||
root: React.ElementType; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The checkbox should have slots.input
too. If you track back to SwitchBase, it has SwitchBaseRoot
and SwitchBaseInput
.
User should be able to do this:
<Checkbox slotProps={{
root: { className: '...' },
input: { className: '...' },
}} />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siriwatknp I'm bit confused on how to handle slots.input
here, since input
slot is not passed to SwitchBase
from Checkbox
but only inputProps
are passed.
return ( | ||
<CheckboxRoot | ||
<RootSlot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to deprecate inputProps
in favor of slotProps.input
? cc @DiegoAndai
I mean deprecate the inputProps
of Checkbox, but still passing slotProps.input
to <RootSlot inputProps={slotPropss.input} />
because SwitchBase is an internal component, no need to update it.
part of #41281
This PR adds missing root slot