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

Enable onAnimationModalInEnd callback #672

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

pichfl
Copy link
Contributor

@pichfl pichfl commented May 27, 2022

Closes #637

@pichfl pichfl added the enhancement New feature or request label May 27, 2022
@pichfl pichfl force-pushed the pichfl/animate-in branch from 17ec421 to 55caf58 Compare May 27, 2022 21:50
@pichfl
Copy link
Contributor Author

pichfl commented Jun 3, 2022

@nickschot and I went on a wild chase Even though this "fixes" the issue, it most likely does so by covering up the actual problem while also introducing a lot of moving parts. My initial assumption that animationend might not trigger when a user enables prefers-reduce-motion is wrong, all tested browsers still trigger the event even if the animation time is reduced to 0s. If there is a "valid" animation, it will trigger the event.

Quoting MDN (emphasis by me):

The animationend event is fired when a CSS Animation has completed. If the animation aborts before reaching completion, such as if the element is removed from the DOM or the animation is removed from the element, the animationend event is not fired.

We want to make sure, that janking the model component still triggers all necessary parts, but we shouldn't do so by introducing timers. Instead #686 is most likely one of the problems behind #665

I'm keeping this PR open for reference until #665 is resolved.

@pichfl pichfl force-pushed the pichfl/animate-in branch from 55caf58 to 62d6742 Compare October 14, 2022 18:48
@pichfl pichfl marked this pull request as ready for review October 15, 2022 22:38
@pichfl pichfl requested a review from nickschot October 15, 2022 22:38
@marcin-wicha
Copy link

Is anything blocking this currently? All of the referenced issues have been merged/closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an incoming animation end hook
2 participants