-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Feature Request]: Mount and unmount Modal/ComposedModal #18935
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
Comments
Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team. |
@timfuhrmann thanks for opening this up! I can definitely see how this nuance of the current Modal is a hassle.
Yes, I think the animation is the sole reason.
We've recently refactored Modal behind a feature flag to use the native I think this should make it so you can conditionally render the modal and still have animations work. I'm curious though if you'd agree based on your experience. I'm not opposed to exploring a solution similar to what Radix or Spectrum are doing, but we might not need it if this solves for all the use cases you mention. What do you think? |
@tay1orjones Thanks for getting back to me on this so fast!
I am not 100% familiar with the new Happy to do a little more research on this or help explore potential solutions! |
The problem
As of right now
Modal
/ComposedModal
are immediately mounted, even if closed. This is a bottleneck in terms of performance and state management. For example:That's why I have seen many teams either conditionally render its children or the entire modal. Both approaches are not optimal:
The solution
I assume the component is mounted incoherent to the open state for the enter/exit animation. Hence I would suggest to utilise the HTML element's animation event listeners to delay the unmount until the animation has finished.
Examples
<Presence />
: https://github.com/radix-ui/primitives/blob/main/packages/react/presence/src/presence.tsx, see popover.useEnterAnimation
anduseExitAnimation
: https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/utils/src/animation.ts, see popover.Package
@carbon/react
Application/PAL
No response
Business priority
None
Available extra resources
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: