Timing issue with transitionend event, preventing Modal overlay from being closed.

[jcbrand]

Hi there

Thanks for a great lib.

I'm running into a timing issue when I close modals. Sometimes (every 3rd or 4th attempt), the event listener which is registered for transitionend doesn't get called, which means that the Modal overlay doesn't get removed, making the page unusable.

In my CSS I increased the transition time for .fade from 0.15s to 0.2s and that seems to solve the problem (also confirming that it's a timing issue).

I'd suggest you add some kind of timeout so that if the transitionend event never fires, that the Modal overlay still gets removed.

I'm using version 2.0.21 and Chromium 63.0.3239.132 (Developer Build)

[Vuurvlieg]

Same problem for me. Thanks for your commit, it should be marged into the official project.

[thednp]

I remember this has been discussed before, I will think of a way to work around this issue.

[sect2k]

This seems to have been discussed in #184 before.

Looking at the code the issue seems to be the timeout. Pardon my ignorance, but why is it there in the first place? Since dialog is only shown/hidden when the transition completes, it seems redundant.

[thednp]

I cannot reproduce the issue even with the latest Bootstrap 4.0.0 CSS, can you guys create a pen or something?

[sect2k]

@thednp it's hard to reproduce, but it should be easy to simulate by decreasing transition time in CSS, for overlay and modal itself.

But the underlying issues is apparent even by just observing the code, if transition finishes before event handler for `transtiononend' is added, then it will never be run.

Why it works in most cases are transitions that happen on the dialog itself and not the overlay, on which gets the handler attached.

To handle this properly, you should drop setTimeout all together and just rely on the transitiononend event. The catch here is that it will fire of for any transition on the element and child elements, so you need to check in the handler itself if target element is indeed the overlay and it also helps to only fire it on the transition that takes the longest time.

let animationEndHandler = function(e) {
    if (e.target.matches(this.formSelector) && e.propertyName == 'opacity') {
        e.target.removeEventListener(e.type, animationEndHandler)
        //show|hide code here
    }
}

Example of event handler, this is from a project I've been working on recently, where I had to deal with handling code with transtiononend.

[sect2k]

https://codepen.io/anon/pen/jzZZqo

This should illustrate the issue, I've adjusted the timings on transitions, so they finish before event handler gets attached, and as you can see, the overlay never disappears.

[thednp]

@sect2k it's true the setTimeout is not the cleanest/best solution, but there is something to be done, a short delay of at least one millisecond after display:block/none is set in order for the animation to work and eventually trigger transitionend.

There was a setImmediate tool for that but only for select browsers, so I can only use self executable anonymous function for that, now testing and seems to be working properly, at least with Chrome. It even works with 0.05 second animation duration no problem.

[sect2k]

If 1 millisecond is all that is needed, maybe lower the timeout from 150ms to something lower, which should eliminate this issue and is indeed the simplest solution to this problem.

Another possibility could be requestAnimationFrame.

if (hasClass(modal,'fade')) {
    requestAnimationFrame(function() {
        removeClass(modal,showClass);
        modal[setAttribute](ariaHidden, true);
    })

    emulateTransitionEnd(modal, triggerHide)    
} else {
    removeClass(modal,showClass);
    modal[setAttribute](ariaHidden, true);
    triggerHide():
}

[thednp]

That solution is too sexy for this issue, however we're not interested in max performance or max accuracy, we're focused on browser consistency.

I'm gonna close this issue right here, please be sure to check back later for my commits comin in about 30 min or so.

[thednp]

@sect2k @Vuurvlieg @jcbrand all good with the latest master version? Are we ready for a new release tag?

[sect2k]

@thednp my codepen above no longer exhibits the issue, so I'd say it's all good.

[jcbrand]

@thednp I'm not able to test this soon (I'm taking a 2 week break starting Monday). I trust you guys got down to the bottom of this, looks like it. Thanks!

[Vuurvlieg]

Thanks @thednp for the updates!

After testing the latest "bootstrap-native-v4.js" from the "dist" folder of the latest master, so far the modal transition issue doesn't showed up. However, the bug from #169 is there again: When initializing an Carousel with interval set to false or 0, it stills autoplay the slides. Check https://codepen.io/anon/pen/OvvErr

[thednp]

@Vuurvlieg I cannot replicate the issue with Carousel, with data-interval="false" or interval = false JavaScript initialization. Perhaps you are using some outdated version.

[Vuurvlieg]

@thednp Well, if you check the codepen in my previous post you will see that Im using the latest version. Also data-interval="false" is working while interval = false via JS is not.

[thednp]

@Vuurvlieg you are right, I tested and seem to be an issue with manual JavaScript initialization, so we'll get this fixed with the next update.

Thanks for sticking to it.

[Vuurvlieg]

Sure, no problem, and thanks for creating and maintaining this amazing project!