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

Generate one backdrop per modal #427

Merged
merged 4 commits into from
Aug 12, 2021

Conversation

aoumiri
Copy link
Contributor

@aoumiri aoumiri commented Aug 3, 2021

When we open a modal thanks to the modal service, a modal is created in the DOM with it associated backdrop, which is really nice to emphasis that whatever outside of the modal container is not clickable.

But let's imagine that the first modal opens a second modal: the second modal appears directly on top of it, without any clear separation because we still have a single backdrop. See this video here that demonstrate it:

Before.multiple.backdrops.mp4

To improve how the overlapping modals are displayed to the user, this PR changes the way the backdrop is handled, and proposes to create as many backdrops as there are modals. that way, there is a backdrop separation between the first and the second modal, making it clear that the only interactive modal is the one on top:

After.multiple.backdrops.mp4

I am conscious of the fact that overlapping modals have to be avoided as much as possible, but sometimes you need it for things such as user approval for a sensitive action through MFA or similar: in that case you have to stop the current flow, wait for their approval and resume it.

@pichfl
Copy link
Contributor

pichfl commented Aug 6, 2021

Happy to see this implemented and I agree: There should never be stacked modals, but there is always the odd outlier and it needs to work properly.

aoumiri added 3 commits August 8, 2021 10:22
@aoumiri aoumiri force-pushed the generate-one-backdrop-per-modal branch from 9539a48 to 50be17d Compare August 8, 2021 08:24
@nickschot nickschot added breaking enhancement New feature or request labels Aug 12, 2021
@nickschot nickschot merged commit ca3295d into mainmatter:master Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants