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 stays in DOM after closing #665

Closed
sebastianhelbig opened this issue May 25, 2022 · 10 comments
Closed

Modal stays in DOM after closing #665

sebastianhelbig opened this issue May 25, 2022 · 10 comments
Labels
bug Something isn't working

Comments

@sebastianhelbig
Copy link
Contributor

sebastianhelbig commented May 25, 2022

When closing via click on the container, the modal gets "hidden" but stays in the DOM. This essentially disables all interaction with the application which then has to be reloaded to work again.

It does not always happen, but most of the time.

Screencast Chrome 102 Mac with the demo app:
https://user-images.githubusercontent.com/5049311/170370725-1b7b6ef4-29d7-467d-8e99-132d645e7fcb.mp4

I also did some debugging as in one project I often see timeouts in qunit tests with modals. It turned out that when a timeout happens, the event animationend is not being triggered which leads to not calling modal._remove() which leads to not resolving the promise. I am unsure if this is related or another problem.

@sebastianhelbig
Copy link
Contributor Author

I am relatively sure I've found the underlying reason: I have activated "prefers-reduced-motion" on my system. Disabling this and it seems to work fine again and also local qunit tests now run without timeouts.

My guess it that setting the animation durations to 0 is the reason for animationend not always being fired.

@sebastianhelbig
Copy link
Contributor Author

Working fine with PR from @pichfl 👍

@pichfl
Copy link
Contributor

pichfl commented Oct 14, 2022

@sebastianhelbig can you check if the behavior still happens with current master?

@sebastianhelbig
Copy link
Contributor Author

@pichfl Working fine with current main. A new release would be highly appreciated 😊

@pichfl
Copy link
Contributor

pichfl commented Oct 15, 2022

New release should happen soon, I just need to finish up a few things to get it out of the door.

@bitwolfe
Copy link

@pichfl How far out is a release? Out customers are seeing this issue in production and it looks like there's quite a few changes since last release so pointing against the github main branch instead feels risky.

@pichfl
Copy link
Contributor

pichfl commented Nov 22, 2022

@bitwolfe I won't make any promises. If you need this right now, I recommend creating a fork from the current main branch and using that.

@bitwolfe
Copy link

@pichfl Tried to do that, unfortunately Yarn 3.3.0 (latest) doesn't allow me to install from the repo. Fails with an "unsupported workflow" error which is something I've seen in other packages as well after they switched to pnpm. Very strange issue that I haven't been able to figure out. I will have to try and fork and see if it still works if I use yarn instead..

@bitwolfe
Copy link

@pichfl An update with my findings after getting a fork of the main branch installed. This is a slippery one.

Calling close() on the modal from the outside doesn't seem to close the modal if prefer-reduced-motion is enabled on the system and if that applies a 0s duration through CSS media queries. The modal elements are left in the DOM. If I set a non-0s duration, for example 0.01s, then it works.

If however close() is called from inside the modal, then it works fine and the modal elements are removed from the DOM no matter what duration is used. With the 3.0.1 release it broke if you clicked the backdrop to close, but this seems to be fixed in the main branch.

So TLDR: A workaround seems to be to not set a 0s duration, such as 0.01s. Then everything works fine even with 3.0.1.

@pichfl
Copy link
Contributor

pichfl commented Nov 22, 2022

@bitwolfe I created a bug for this. I can't remember why I moved to 0s, it was a non-zero value before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants