valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.53k stars 1.69k forks source link

Modal Backdrop Not Removed #1139

Closed rsnider19 closed 6 years ago

rsnider19 commented 8 years ago

I think the issue I am seeing is similar to #853. When in a modal (which has a routerLink), the backdrop is correctly removed when navigating away. When I navigate back to my page and open the modal, I get 2 backdrop elements. Rinse and repeat, I get 3, and so on and so forth.

I am hiding the modal when routing which may be the issue, but I am not sure:

this.router.events.subscribe(e => {
  this.modal.hide();
});

The multiple backdrops are also staying when I close the modal normally and navigate away and back in.

valorkin commented 7 years ago

Please check, should be fine now

kambbado commented 7 years ago

I'm having the same issue, the backdrop is not being removed. This is what is left after calling modal.hide()

<bs-modal-backdrop class="modal-backdrop fade in"></bs-modal-backdrop>

` <div bsModal #modal="bs-modal" class="modal" tabindex="-1" role="dialog" aria-hidden="true" [config]="{'backdrop':'static', 'keyboard': false, 'isAnimated': false }" >

valorkin commented 7 years ago

Change detection.on push?

kambbado commented 7 years ago

Thank you for the information. However, it made no effect. What I have been able to find is that on the first call to hide() it leave behind <bs-modal-backdrop class="modal-backdrop fade in"></bs-modal-backdrop>, if I used Chrome Developer Tools to remove the element, all calls to show() and hide() works as expected and I can call them numerous times with no issues. I'm using this.modal.onHidden() to trigger a callback.

masseelch commented 7 years ago

I sometimes still have this bug. There are two backdrops created and only one gets removed when the modal is hidden.

I do use detection.on push and currently use the v2.0.0-beta.7. Any clues?

valorkin commented 7 years ago

Can't catch it properly to debug, need reproducible demo :(

masseelch commented 7 years ago

Problem is, we ares so far not able to give reproducible steps since we can not say when this occurs. To us, it seems totally random. But i will hand in an example form our code-base for you when im back to my machine.

valorkin commented 7 years ago

I am sure in one scenario when modal close began and error was thrown (somewhere outside) last peace of removal will not be executed

masseelch commented 7 years ago

We can confirm, that there is no error thrown anywhere, when this happens.


Here's the code we are using, which will sometimes not hide the backdrop or create two backdrops and only hide one.

The component holding the modal:

@Component({
    moduleId: module.id,
    changeDetection: ChangeDetectionStrategy.OnPush,
    selector: 'qb-comp',
    template: '<qb-question-wizard (onHidden)="onHidden()"></questionnaire-question-wizard>'
})
export class BuilderComponent implements OnInit {
    @ViewChild(QuestionWizardComponent) questionWizard: QuestionWizardComponent;

    constructor(private bs: BuilderService, private cdr: ChangeDetectorRef) {}

    ngOnInit(): void {
        BuilderService.questionWizardEditQuestion.subscribe(question => this.questionWizard.editQuestion(question));
    }

    onHidden(): void {
        this.cdr.markForCheck();
    }
}

The component with the modal-directive.

@Component({
    moduleId: module.id,
    selector: 'qb-question-wizard',
    changeDetection: ChangeDetectionStrategy.OnPush,
    template: `
        <div bsModal #questionWizard="bs-modal" class="modal fade" tabindex="-1" role="dialog" (onHidden)="onHidden.emit()">
            <div class="vertical-alignment-helper">
                <div class="modal-dialog modal-dialog--center modal-md modal-dialog--vertical-align-center">
                    <div class="modal-content">
                        <button type="button" class="close pull-right" aria-label="Close" (click)="questionWizard.hide()">
                            <span aria-hidden="true">&times;</span>
                        </button>
                        <div class="modal-header">
                            <h4 class="modal-title">
                                <span translate>pages.questionnaire_builder.editor.question_wizard.edit_title</span>
                            </h4>
                        </div>
                        <div class="modal-body">
                            <model-form [model]="question" (submit)="submit($event)"></model-form>
                        </div>
                    </div>
                </div>
            </div>
        </div>
    `,
})
export class QuestionWizardComponent {

    @ViewChild('questionWizard') questionWizard: ModalDirective;

    @Output() onHidden: EventEmitter<void> = new EventEmitter<void>();

    constructor(private bs: BuilderService) {}

    editQuestion(question: Question<any>): void {
        this.question = question;
        this.questionWizard.show();
        this.cdr.markForCheck();
    }

    submit(data: Question<any>): void {
            this.bs.updateQuestion(question).subscribe((response: QuestionInterface<any>) => {
               this.questionWizard.hide();
            }, error => {
                this.questionWizard.hide();
            });
        }
    }
}
valorkin commented 7 years ago

I will reopen this issue, to keep in mind this flickering issue

mattmcardle commented 7 years ago

I am getting this issue when launching the modal from a directive, here is my code:

Modal:

@Component({
  selector: 'app-test-modal',
  template: `
    <div class="modal-header">
      <h4 class="modal-title pull-left">{{title}}</h4>
      <button type="button" class="close pull-right" aria-label="Close" (click)="bsModalRef.hide()">
        <span aria-hidden="true">&times;</span>
      </button>
    </div>
    <div class="modal-body">
      <ul *ngIf="list.length">
        <li *ngFor="let item of list">{{item}}</li>
      </ul>
    </div>
    <div class="modal-footer">
      <button type="button" class="btn btn-default" (click)="bsModalRef.hide()">Close</button>
    </div>
  `
})
export class TestModalComponent implements OnInit {

  constructor(public bsModalRef: BsModalRef) { }

  list = [];

  ngOnInit() {
  }

}

Directive:

@Directive({
  selector: '[trigger]',
  host: {
    '(click)': 'openMenu()'
  },
  providers: [BsModalService]
})
export class TriggerDirective {

  modalRef: BsModalRef;

  constructor(private modalService: BsModalService) {
  }

  openMenu() {
    this.modalRef = this.modalService.show(TestModalComponent);
    this.modalRef.content.title = 'Modal with component';
  }
}
rdurgarao commented 7 years ago

+1, same issue. Looking for a fix.

xhuberdeau commented 7 years ago

+1, same issue also. Using ngx v1.9.3 It happens only the first time I open the modal. If I then remove the additional backdrop manually from the DOM and reopen the modal, everything works fine.

dstaflund commented 7 years ago

We ran into this problem about a month back but found that it was caused because we hadn't unsubscribed from some of the services we had subscribed to. Because we hadn't unsubscribed, Angular held onto a reference of the component (or something like that -- I'm still learning Angular so I am not sure what exactly is happening) and the backdrop wasn't cleaned up. Once we added an 'ngOnDestroy' method to unsubscribe from our services, the backdrop issue went away.

xhuberdeau commented 7 years ago

@dstaflund Thanks for your help it worked for me. Code used with angular4: pastebin

NextTrick commented 6 years ago

+1, I have the same issue. Using ngx v1.9.3.

HalShaw commented 6 years ago

+1, same issue, both ngx v2..0.0 and v1.9.3.

IlyaSurmay commented 6 years ago

Could anyone provide a reproduction of this? You can use one of starter templates:

Plunkr: https://plnkr.co/edit/0NipkZrnckZZROAcnjzB?p=preview

StackBlitz: https://stackblitz.com/edit/ngx-bootstrap?file=app%2Fapp.module.ts

dstaflund commented 6 years ago

Hi Ilya,

I tried to create an example for you in Plunker but I can't seem to reproduce the issue in it although I can easily reproduce the issue at work. I'll see if I can create and upload a small project to Github for you to take a look at.

In our case, the problem can be reproduced by creating a page that does the following:

  1. Within the constructor, subscribes to an Observable returned by a service method -- BUT NEVER UNSUBSCRIBE FROM IT (this is important to reproducing the issue.)
  2. Displays the modal

Once you have these conditions in place, you can reproduce the error by doing the following:

  1. Go to the page
  2. Display and then close the modal
  3. Go to some other page in the app
  4. Return to the page
  5. Display and the close the modal

When the modal is closed in #5, the backdrop remains until you refresh the browser.

When my team member researched the problem, he found that the page containing template couldn't release all references to the backdrop object until it had released all of its Subscriptions. Once we caught onto this and used the onDestroy method to unsubscribe, the problem went away.

I'll see if I can upload a reproducible problem to Github for you this weekend. I'll send you a link if I get this done, otherwise I'll let you know I didn't have time to do it.

Darryl

dstaflund commented 6 years ago

Hi Ilya,

I've uploaded a small application to github that reproduces the modal backdrop issue:

https://github.com/dstaflund/ngx-bootstrap-backdrop-issue/

Just download and run it -- the initial page will tell you how to reproduce the issue and will also describe how the issue can be resolved.

Darryl

IlyaSurmay commented 6 years ago

@dstaflund Hi Darryl, thanks for the reproduction, now I see the issue. Actually, in your case, it seems to me that everything is working as expected. Every time you visit the page with modal, one more subscription is created, so its code will be executed as many times as many subscriptions you have, therefore, a function that shows the modal will be called more than once and it leads to more than one backdrop. Using ngOnDestroy to unsubscribe from everything is a known pattern that is widely used and recommended, so it's definitely should be used if modal is controlled by some subscription.

I don't think that we can fix this in your case, because we can't get a reference to a backdrop that has been created by some subscription from a destroyed component. Of course, there are some dirty workarounds like direct access to DOM but I don't think they should be used.

So, the clearest and the easiest way is to use ngOnDestroy and unsubscribe from subscription that is used for showing modal.

Some links about this topic: http://brianflove.com/2016/12/11/anguar-2-unsubscribe-observables/ https://medium.com/thecodecampus-knowledge/the-easiest-way-to-unsubscribe-from-observables-in-angular-5abde80a5ae3

IlyaSurmay commented 6 years ago

This will be reopened if someone provides a reproduction that shows a scenario when you don't have any subscriptions but the backdrop issue is still reproducible.

abdulfaisal commented 6 years ago

Here is my workoaround to fix this probelm. code is in angular 2+.

Array.from(document.getElementsByClassName('modal-backdrop')).forEach((item) => { item.parentElement.removeChild(item); });

Since you may have multiple unremoved modal-backdrops, you can remove all of them using above code.

gawadaz commented 6 years ago

try the below snippet, it worked for me

modalRef: BsModalRef;
constructor(private modalService: BsModalService) { }
....
decline(): void {
    this.modalRef.hide();
    this.modalService.hide(1);
  }
ldoloscan commented 6 years ago

Hey guys,

I had this problem with backdrops and can confirm that unsubscribing from each subscription in ModalComponent (this is my component) fixes this issue !!

Thanks to ngx-bootstrap TEAM for good work !!

hasi94 commented 9 months ago

i use this to hide remaining backdrop this works for me use

<div bsModal #rentalModel="bs-modal" class="modal fade custom-moda {{hiddenModal ? 'hidden' : ''}}" tabindex="-1"
     [config]="{ backdrop: 'static' ,ignoreBackdropClick: true}" role="dialog" aria-labelledby="dialog-sizes-name1" (onHidden)="onModalHidden()">
  <add-rental-lease-details  [propertyRef]="property_ref" (clickSave)="propertySave($event)" (closeModel)="closeAddNewProperty($event)"></add-rental-lease-details>
</div>

in my component

 onModalHidden() {
    const modalBackdrop = document.querySelector('.modal-backdrop');
    if (modalBackdrop) {
      modalBackdrop.remove();
    }
  }