material-ui
material-ui copied to clipboard
mui
[Alert] Add `components` and `componentsProps` props to allow close action overrides
| Messages | |
|---|---|
| :book: | Netlify deploy preview: https://deploy-preview-33582--material-ui.netlify.app/ |
Generated by :no_entry_sign: dangerJS against b858412bc0aa125057fbd57427be75bf45d5cd29
I think we should also add a warning that if components.CloseIcon is provided, the onClose should also be provided without which components.CloseIcon is of no use. Something like this.
I get a 406 on that link. I'll look into this, thanks.
@ZeeshanTamboli in my use case I'm wanting to override the default close icon with my company's custom close icon via theme overrides. If I warned on providing CloseIcon with no onClose, users implementing our themed components would get warnings unless they provided onClose, which I don't think is what we want.
@ZeeshanTamboli in my use case I'm wanting to override the default close icon with my company's custom close icon via theme overrides. If I warned on providing CloseIcon with no onClose, users implementing our themed components would get warnings unless they provided onClose, which I don't think is what we want.
Come to think of it, it would be not necessary to have a warning. If CloseIcon is provided, users would provide an onClose action. I think we can ignore it.
@ZeeshanTamboli It's been three days, how long is reasonable to wait?
@mnajdova I updated this on Friday to add your recommendations. I drew inspiration from Backdrop, since most other components that have components and componentsProps are unstyled components in @mui/base. Let me know if this is close to what you had in mind.
@mnajdova @michaldudak It's been a little while - is there anything else that needs to be done with this PR before it gets merged?
Could you please merge in the latest master? This should solve the failing regression tests issue.
