shlomiassaf / ngx-modialog

Modal / Dialog for Angular
http://shlomiassaf.github.io/ngx-modialog
MIT License
686 stars 242 forks source link

'modal-open' gets left on body (bootstrap plugin) #217

Open sublime392 opened 8 years ago

sublime392 commented 8 years ago

[*] bug

If the modal is closed when the browser is in the background the animation does not complete and so the modal-open class does not get removed from the body. This makes the screen non-scrollable as the overflow is hidden. This happens on Chrome browser, assumed other browsers as well. I am using the 2.0.0 version, but this was also happening in the 2.0.0-beta.13.

shlomiassaf commented 8 years ago

@iherbivore how did you test this scenario?

It's probably a bit deeper then animation since the animation process is guarded by a timeout, see source code

sublime392 commented 8 years ago

I have a confirm modal that gets closed after a timeout. I switch to a different tab and watch the elements via developer tools. I can see the modal div get removed from the page when the modal closed but the class does not. Another scenario: for some reason in my production build the close animation freezes part way through and after a delay (your timeout?) The modal disappears. In that case the class is also left on the body.

sublime392 commented 8 years ago

Here is a plunk where the alert modal closes after 5 seconds. Watch the plunker body in developer tools. The 'modal-open' class gets applied and removed with alert open/close. Now open it and switch to another tab before it closes. You will see the body tag flash when the dialog closes but the 'modal-open' is not removed.

danielcrisp commented 7 years ago

I'm having the same problem with the .modal-open class remaining on the <body>, except it happens even when the browser is not in the background and it is closed using the buttons.

Any ideas what could be the issue here?

danielcrisp commented 7 years ago

Ok figured it out - it wasn't a problem with angular2-modal

The problem was that I hadn't included the animation partial within Bootstrap (using SCSS). As soon as I added that everything started working correctly.

image

sublime392 commented 7 years ago

Point of clarification: @danielcrisp had a configuration issue that is not related to the issue I reported. The original issue remains.

danielcrisp commented 7 years ago

Me again.

I've just found another case where the class is not removed. It works in Chrome, but not Firefox.

It seems that this subscription never fires in Firefox:

backdrop.myAnimationEnd$()
        .combineLatest(container.myAnimationEnd$(), (s1, s2) => [s1,s2] )
        .subscribe( sources => {
            ...

I'm destroying my modal when its parent component is destroyed (because the route has changed - the user clicked the browser's back button)

    // parent component
    ngOnDestroy () {

        // reference to modal's dialogRef
        if (this.confirmDialogRef) {
            this.confirmDialogRef.destroy();
        }

    } 

It looks like a CSS TransitionEnd event is used to trigger the myAnimationEnd$ subscription, but this MDN page states that it may not fire in some scenarios:

In the case where a transition is removed before completion, such as if the transition-property is removed or display is set to "none", then the event will not be generated.

Perhaps my route change is removing the element before the CSS animation can finish and therefore I get no TransitionEnd event.

But that's the kind of thing I thought the timeout guard should be protecting against... and I've checked it and I can see that Firefox does indeed trigger the guard. Yet the subscription never fires and the modal-open class remains on the <body>.

Will keep investigating.

MartinNuc commented 7 years ago

Having the same issue. I used setTimeout to postpone modal action so there is time to remove modal-open class.