-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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
[Joy UI] Add Modal
component
#34043
[Joy UI] Add Modal
component
#34043
Conversation
@mui/joy: parsed: +4.91% , gzip: +5.15% |
Modal
componentModal
component
I think we should have consider on these two :1. In the demo of Full-screen Modal, it will be good to have a exit button. |
Good call, will add it to the demo.
In the first stage of Joy, I won't add built-in transition props to keep the component lean as much as possible. However, I do understand that the transition is quite important from the UX perspective, so I will add a demo on how to integrate other transition/animation libraries with Joy UI. |
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.
This is looking great so far! A few considerations that popped up while reviewing it:
- We probably could make this clearer but are the
ModalDialogTitle
andModalDialogDescription
able to receive the same props as the Typography component? I see how that wouldn't make sense_but_ I also see how that would be intuitive to expect (I did, in fact, while doing the small doc demos tweaks). - Directly related to that, should we say something about using a Typography element inside any of these two? Is this ok to do or to avoid? What are the use cases I'd want to do that?
- Are we missing a
ModalOverlay
component? How do I customize that? - Should we add a prop for easily centering the modal both horizontally and vertically? That would be handy.
- Should we add the capacity control whether or not clicking on the overlay closes the modal/dialog?
@danilo-leal Hey, I decided to remove the |
Sweet, ok! Two quick questions before we wrap this up:
|
Agreed!
Added! |
Awesome, this is looking great! One last thing I'm seeing is a discrepancy on the |
The |
Should we make |
So, only |
Yup, that's it! |
Done! |
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.
Amazing, nice work Jun! 🎉
Modal
componentModal
component
Preview: https://deploy-preview-34043--material-ui.netlify.app/joy-ui/react-modal/
Joy provides 3 modal components (I decided not to create a Dialog component yet):
role="dialog"
modalModalDialogTitle (similar to Material UI's DialogTitle) bind with(continue in a separate PR)ModalDialog
'saria-labelledby
attribute.ModalDialogDescription (new): this component handles the binding logic with the(continue in a separate PR)ModalDialog
similar toModalDialogTitle
but witharia-describedby
attribute.According to WAI ARIA, dialog can be modal or non-modal.
It is more explicit to render a modal dialog with
ModalDialog
component. The good part is that it produces a single DOM which is easier to customize compare to the Material UI's Dialog. To display an alert dialog, just switch torole="alertdialog"
.Note: Material UI's Dialog is specific to Material Design's Dialog which is not the same as WAI ARIA.