Skip to content

[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

Open
1 task done
timfuhrmann opened this issue Mar 27, 2025 · 3 comments · May be fixed by #19531
Open
1 task done

[Feature Request]: Mount and unmount Modal/ComposedModal #18935

timfuhrmann opened this issue Mar 27, 2025 · 3 comments · May be fixed by #19531
Labels
component: modal needs: code contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react package: styles @carbon/styles status: waiting for maintainer response 💬 type: enhancement 💡

Comments

@timfuhrmann
Copy link

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:

  1. Form states within in modals are not reset
  2. Open state has to be forwarded to children to delay any data fetching
  3. Huge data tables with overflow menus are mounting a lot of modals
  4. Virtual lists will unmount and mount modals and create portals very frequently

That's why I have seen many teams either conditionally render its children or the entire modal. Both approaches are not optimal:

  1. Conditionally rendering the children will remove the content before the exit animation has ended and the wrapper will still be mounted, the portal sill created
  2. Conditionally rendering the modal will remove all animations

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

  1. Radix is using <Presence />: https://github.com/radix-ui/primitives/blob/main/packages/react/presence/src/presence.tsx, see popover.
  2. React Aria is using useEnterAnimation and useExitAnimation: 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

Copy link
Contributor

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

@timfuhrmann timfuhrmann changed the title [Feature Request]: [Feature Request]: Mount and unmount Modal/Composedmodal Mar 27, 2025
@tay1orjones
Copy link
Member

@timfuhrmann thanks for opening this up! I can definitely see how this nuance of the current Modal is a hassle.

I assume the component is mounted incoherent to the open state for the enter/exit animation.

Yes, I think the animation is the sole reason.

Hence I would suggest to utilise the HTML element's animation event listeners to delay the unmount until the animation has finished.

We've recently refactored Modal behind a feature flag to use the native <dialog> element. It uses a new internal Dialog component that uses @starting-style for the animations, which supports transitioning elements on DOM addition and removal. We've done a similar refactor for ComposedModal as well.

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?

@timfuhrmann
Copy link
Author

timfuhrmann commented Mar 30, 2025

@tay1orjones Thanks for getting back to me on this so fast!

It uses a new internal Dialog component that uses @starting-style for the animations, which supports transitioning elements on DOM addition and removal.

I am not 100% familiar with the new Dialog API yet, but from what I can see and after brief testing the unmount unfortunately still has to be delayed for the exit animation to not be interrupted. The docs example's JavaScript snippet also shows a setTimeout to defer said removal. My guess is that a solution similar to what Radix or Spectrum are doing is still necessary to make this work.

Happy to do a little more research on this or help explore potential solutions!

@timfuhrmann timfuhrmann changed the title [Feature Request]: Mount and unmount Modal/Composedmodal [Feature Request]: Mount and unmount Modal/ComposedModal Mar 30, 2025
@sstrubberg sstrubberg added needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. and removed status: needs triage 🕵️‍♀️ labels Mar 31, 2025
@sstrubberg sstrubberg moved this from Triage to Later 🧊 in Roadmap Mar 31, 2025
@sstrubberg sstrubberg moved this to 🔖 Ready in Community Workgroups Mar 31, 2025
@sstrubberg sstrubberg added the needs: code contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. label Mar 31, 2025
@timfuhrmann timfuhrmann linked a pull request May 30, 2025 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: modal needs: code contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react package: styles @carbon/styles status: waiting for maintainer response 💬 type: enhancement 💡
Projects
Status: 🔖 Ready
Status: Later 🧊
Development

Successfully merging a pull request may close this issue.

3 participants