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

bug(modal): if backdrop === 'static' outside click should not close modal #574

Closed
dave-barrineau opened this issue Jun 2, 2016 · 13 comments

Comments

@dave-barrineau
Copy link

dave-barrineau commented Jun 2, 2016

Provided I'm doing this correctly, I've set the modal's config to:

<div bimodal #myModal="bs-modal" class="modal fade" [config]="{ keyboard: false, backdrop: false }" aria-labelledby="myLargeModalLabel" aria-hidden="true"> ... </div>

However this does not prevent the modal from closing when clicking outside of the modal.

@valorkin
Copy link
Member

valorkin commented Jun 2, 2016

oh, thanks! my bad, I forgot to add check here:
https://github.com/valor-software/ng2-bootstrap/blob/development/components/modal/modal.component.ts#L79
it should be

@HostListener('click', ['$event'])
  protected onClick(event:any):void {
    if (this.config.backdrop === 'static' && event.target === this.element.nativeElement) {
      this.hide(event);
    }
  }

and options values should be added to docs (from original bs docs)
backdrop boolean or the string 'static' true Includes a modal-backdrop element. Alternatively, specify static for a backdrop which doesn't close the modal on click.

Nice reason to make PR :)

@valorkin valorkin changed the title Modal closes when clicking outside of modal bug(modal): if backdrop === 'static' outside click should not close modal Jun 2, 2016
@dave-barrineau
Copy link
Author

Thanks!

Sent from my iPhone

On Jun 2, 2016, at 6:29 PM, Dmitriy Shekhovtsov notifications@github.com wrote:

oh, thanks! my bad, I forgot to add check here:
https://github.com/valor-software/ng2-bootstrap/blob/development/components/modal/modal.component.ts#L79
it should be

@HostListener('click', ['$event'])
protected onClick(event:any):void {
if (this.config.backdrop === 'static' && event.target === this.element.nativeElement) {
this.hide(event);
}
}
and options values should be added to docs (from original bs docs)
backdrop boolean or the string 'static' true Includes a modal-backdrop element. Alternatively, specify static for a backdrop which doesn't close the modal on click.

Nice reason to make PR :)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@mpetkov
Copy link

mpetkov commented Jun 8, 2016

Since this is an easy fix, based on the effort tag, is it possible to expedite it?

@mpetkov
Copy link

mpetkov commented Jun 8, 2016

Also, the code is supposed to be:
this.config.backdrop !== 'static'
If it is static, it shouldn't hide the modal when clicked.

@calvinKG
Copy link

Hi,

What's the solution for this bug ?

@mpetkov
Copy link

mpetkov commented Jun 28, 2016

@calvinKG - Bug has been fixed, update you version to get the fix.

@eRez-ebs
Copy link

eRez-ebs commented Jun 28, 2016

hi,
ran into the exact same problem, trying to download the recent version but unsuccesfully.
when i used 'npm install ng2-bootstrap --save' the ng2-bootstarp version in the package.json remained 1.0.17.
when i downloaded ng2-bootstarp-develpoment.zip and replaced the files manually (either those of the modal libarary or the entire folder) i got complation errors.
in both cases the result remained the same - the model was closed when clicking on the background even when i set 'data-backdrop="static"'

can you please help me with that
cheers,
eRez

@mpetkov
Copy link

mpetkov commented Jun 28, 2016

@eRez-ebs - So I am also using 1.0.17. So just passing data-backdrop won't work since that only works for the bootstrap version. To get it to work with this library you have to pass it via the config input like so:

[config]="{'backdrop':'static', 'keyboard': false}"

@eRez-ebs
Copy link

eRez-ebs commented Jun 28, 2016

@mpetkov thanks for the quick reply.
your solution resolved the ESC issue, which no longer closes the modal, but sadly not the background - clicking on it still closes the modal :-(

@calvinKG
Copy link

@mpetkov am also exprinceing the same problem.

"ng2-bootstrap": "^1.0.17", thats the version that am using.

@eRez-ebs
Copy link

i found out why it didn't work for me.
i've looked into 'node_modules\ng2-bootstrap\components\modal' folder localy, where there's a 'modal.component.d.ts' file and 'modal.component.js' file.
the d.ts file has only declerations in it, but indside the js file i found this:

/** Host element manipulations */
// @HostBinding(`class.${ClassName.IN}`) private _addClassIn:boolean;
ModalDirective.prototype.onClick = function (event) {
        if (event.target === this.element.nativeElement) {
            this.hide(event);
        }
    };
    // todo: consider preventing default and stopping propagation
    ModalDirective.prototype.onEsc = function () {
        if (this.config.keyboard) {
            this.hide();
        }
    };

doesn't look much like the file here - https://github.com/valor-software/ng2-bootstrap/blob/development/components/modal/modal.component.ts#L79

anyhow, it explains why the onClick fails but the onEsc works.
i simply added to the condition of the onClick this.config.backdrop !== 'static' and it worked (meaning the modal wasn't closed when the background was clicked).
i just can't figure out why after i update bootstrap library i still get an oudated version.

eRez

@calvinKG
Copy link

How are we going to implement this updates ?

@ctrl-brk
Copy link

ctrl-brk commented Oct 3, 2016

[config]="{backdrop: 'static'}" works for me in 1.1.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants