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] Dimmer not hiding if multiple modals animating #2441

Open
mareeo opened this issue Aug 30, 2022 · 3 comments
Open

[Modal] Dimmer not hiding if multiple modals animating #2441

mareeo opened this issue Aug 30, 2022 · 3 comments
Labels
state/has-pr An issue which has a related PR open type/bug Any issue which is a bug or PR which fixes a bug
Milestone

Comments

@mareeo
Copy link
Contributor

mareeo commented Aug 30, 2022

Bug Report

When a modal is hidden, it checks for other modals and if there are no other active or animating modals it hides the modal dimmer. This happens on the hiding animation start. If two modals start hiding at the same time, neither of them will hide the dimmer. This leaves you with a dimmer with no modals

Steps to reproduce

  1. Create a modal.
  2. Create another modal.
  3. Hide the second modal.
  4. While the second modal is animating with its hide, hide the first modal.

Expected result

Modal dimmer should be hidden

Actual result

The modal dimmer stay visible and active with no modals in it.

Testcase

https://jsfiddle.net/xmd9eht6/

Version

2.8.8

Potential Solution

  • Don't start hiding first modal until second modal is hidden. This is the current workaround I'm doing.
  • Use the modal's hide all or hide dimmer functions.
  • (Code change) When hiding a modal, instead of doing the check for other modals and hiding the dimmer on hide animation onStart, do all this on hide animation onComplete.

I can make a pull request if others think this would be an appropriate fix

@mareeo mareeo added state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Aug 30, 2022
@lubber-de lubber-de removed the state/awaiting-triage Any issues or pull requests which haven't yet been triaged label Aug 30, 2022
@lubber-de lubber-de added this to the 2.9.x milestone Aug 30, 2022
@lubber-de
Copy link
Member

Yes, if you already have a possible solution, then of course feel free to provide a PR 🙂 so we can test this out then

@lubber-de
Copy link
Member

What will be interesting is this

Don't start hiding first modal until second modal is hidden

Assume you have 4 modals and each other would have to wait for completing its hiding before the next one starts hiding. This could be a potential slowdown and increases waitstates (especially in your, for explanation purposes, slow animation)

I guess your codechange proposal might fix it better (?), so each hide completion should check if there are no more open/animating modals.

we have a somehow related PR here #1438

@mareeo
Copy link
Contributor Author

mareeo commented Aug 30, 2022

You're right, my workaround isn't ideal, but with animations set to 100ms it isn't noticeable to the users. Still slightly annoying to have to do it though.

PR submitted with the change described above.

@lubber-de lubber-de added state/has-pr An issue which has a related PR open and removed state/awaiting-investigation Anything which needs more investigation labels Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state/has-pr An issue which has a related PR open type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

No branches or pull requests

2 participants