Skip to content
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

[Modal]: Support nested modals #14104

Closed
1 task done
davidmenendez opened this issue Jun 27, 2023 · 6 comments · Fixed by #14320
Closed
1 task done

[Modal]: Support nested modals #14104

davidmenendez opened this issue Jun 27, 2023 · 6 comments · Fixed by #14320
Labels
component: modal needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. type: enhancement 💡

Comments

@davidmenendez
Copy link
Contributor

The problem

Currently it's not possible to nest a modal within a modal without causing breaking problems. While we can and have argued that modals shouldn't be nested within modals, that hasn't stopped users from doing it and then opening issues about it. This is especially true for carbon for ibm products which extends modal several times. The biggest point of contention being with the Tearsheet component where we have seen numerous complaints and bug reports regarding the inability to use a modal within tearsheet. And yes you could argue (and we have) that if you're using Tearsheet in a way that requires an additional modal that perhaps a tearsheet isn't the ideal solution for you or your product, that still hasn't stopped it from happening.

So, I'm here to advocate for a look at the current modal code to see if we can make adjustments so that nesting modals is possible. I'm also here to offer my services to fix this problem personally.

The solution

After examining the modal code I believe the main problem can be addressed by making some adjustments to the structure of modal and how certain events like onClose are triggered.

Examples

No response

Application/PAL

https://github.com/carbon-design-system/ibm-products/tree/main

Business priority

None

Available extra resources

as a developer i'd be more than happy to try to tackle this

Code of Conduct

@davidmenendez
Copy link
Contributor Author

after discussing this issue in the previous DSAG meeting i've gone ahead and started a mural for solution exploration https://app.mural.co/invitation/mural/watsonassistant2719/1688405516721?sender=dmenend6705&key=ab296ce7-8698-4423-a824-df2c130e2065

@tay1orjones
Copy link
Member

@davidmenendez could you outline what the hard blockers are currently with nested modals?

@davidmenendez
Copy link
Contributor Author

@tay1orjones for IBM Products it's pretty simple.

The Tearsheet component is build on top of the carbon modal. It basically looks like feels like a modal, just a different flavor of it so to speak. The problem is that a lot of our users have been trying to place regular modals within tearsheets. There's been plenty of debate surrounding this design though. We don't think nesting modals makes sense or nesting modals within tearsheets, but that hasn't stopped product designers from asking for them.

From a technical aspect i don't see any reason why modals couldn't be stacked on top of each other. Obviously there are plenty of design / UX concerns and questions around this type of interaction, but it's my opinion that this is a design / UX problem and not a technical one.

I'm working on a proof of concept PR with changes to Modal to demonstrate what changes need to be made to allow supporting nested modals.

@sstrubberg
Copy link
Member

Next steps: @davidmenendez collect usage examples of this.

Ex. A modal on top of a tearsheet... might be the exception to nesting modals.

@sstrubberg sstrubberg moved this from Triage to Ready for community implementation in Roadmap Jul 10, 2023
@sstrubberg sstrubberg added the needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. label Jul 10, 2023
@tay1orjones
Copy link
Member

@davidmenendez Yeah that makes sense. It's been suggested that we could refactor modal to use the native <dialog> element now that the HTML spec for it has solidified. With this too there is nothing explicitly blocking the nesting of <dialog>s within one another.

All this to say I think it's worthwhile to remove any technical blockers that might be in place preventing folks from doing this, despite the UX concerns and debate.

@davidmenendez
Copy link
Contributor Author

@tay1orjones yes i just saw that! funnily enough i just opened a PR to demonstrate some small changes that would be necessary to enable this support. i'm happy to discuss what are options are and continue to fine tune these changes. and yes, believe me, we've debated this to death. but hey the user is never wrong, right? 😂

@github-project-automation github-project-automation bot moved this from Ready for community implementation to Completed in Roadmap Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: modal needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. type: enhancement 💡
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants