[Modal][material-ui] Update it to use the useModal hook #38498
Conversation
mnajdova
commented
Aug 16, 2023
mnajdova
commented
- I have followed (at least) the PR section of the contributing guide.
mnajdova
added
component: modal
Netlify deploy previewhttps://deploy-preview-38498--material-ui.netlify.app/ @material-ui/core: parsed: -0.23% 😍, gzip: -0.20% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
What's the rationale for going in this direction? I mean, what problem does it solve vs. using Modal?
I mean, as I understand it, in a perfect world we would style Material UI on top of Base UI component, since this provides the highest abstraction level, hence fewer duplications.
So I wonder when we take a step in the opposite direction. Is this about having a simpler React tree? Like better performance, when opening the React component tree, less noise, etc?
| /** | ||
| * If `true`, the modal will not automatically shift focus to itself when it opens, and | ||
| * replace it to the last focused element when it closes. | ||
| * This also works correctly with any modal children that have the `disableAutoFocus` prop. | ||
| * | ||
| * Generally this should never be set to `true` as it makes the modal less | ||
| * accessible to assistive technologies, like screen readers. | ||
| * @default false | ||
| */ | ||
| disableAutoFocus?: boolean; |
There was a problem hiding this comment.
Are these types duplicated? If I change the description here, does it fail the CI?
If no, why not import from ModalOwnProps?
