HubSpot / vex

A modern dialog library which is highly configurable and easy to style. #hubspot-open-source
http://github.hubspot.com/vex/docs/welcome
MIT License
6.92k stars 494 forks source link

MSAnimationEnd does not work in IE 11, but animationend does #242

Closed vapalamar closed 6 years ago

vapalamar commented 7 years ago

This event should occur when we close modal window. But it doesn't, so, as the result, the modal window is not closed. What if we always add an event listener for the animationend event? Or there is another fix? Attached version of IE.

image

bbatliner commented 7 years ago

So animationEndEvent/detectAnimationEndEvent exists now to detect the correct animation end to use. Are you saying this doesn't work in that version of IE?

vapalamar commented 7 years ago

@bbatliner Yes, animationEndEvent/detectAnimationEndEvent returns MSAnimationEnd in this version of Internet Explorer, and this isn't working. However, if we return animationend string from this method instead, everything will work as expected.

vapalamar commented 7 years ago

I opened the pull request with a possible fix: https://github.com/HubSpot/vex/pull/243

PixievoltNo1 commented 6 years ago

When I made Pull Request #238, in the process the animation properties got rearranged to put the unprefixed property last. When browsers encounter both prefixed and unprefixed versions of a property, they apply a "last one wins" rule. Since detectAnimationEndEvent uses a "first one wins" rule, animation should be tested for first to match that behavior. #243 doesn't change that; if you like, I can make a different pull request and we'll see if it fixes the problem.

vapalamar commented 6 years ago

@PikadudeNo1 got it. It would be really nice if you create a pull request with a possible fix.

PixievoltNo1 commented 6 years ago

I just tested this by running the grunt command and opening test/build-test.html in IE - vexes seem to be closing properly and everything seems to be working OK, no code changes needed. IE version 11.674.15063.0.

bbatliner commented 6 years ago

I switched the order of the property checks so that animation is checked first. Should work better in modern browsers with no change for older ones. This'll be published with the next release which I'll get to this weekend.