closeOnClickOutside doesn't work when clicking sides

[artemzakharov]

I attempted to migrate from angularx-bootstrap-modal today, but ran into an issue where I could not dismiss the modal by clicking to the left or right of it - only above and below. I know for certain this issue is with the new package because the only thing I swapped was the module, service, and component names.

On a side note, it would be nice if the backdropColor parameter was brought back - it was pretty convenient.

[kevcjones-archived]

Deleted last comment - its early, i thought the issue came from the old repo :)

[kevcjones-archived]

Re: CSS background colour, is there a reason you can't or don't want to use a css style to manage the backdrop colour? Just so we understand.

[kevcjones-archived]

So i can see the cause 👍 the fix is a little clunky though.

this function i updated did away with the manual checking for the modal classes of bootstrap and made an faulty assumption that the first child of the modal being opened was the content...

onClickOutsideModalContent( callback: () => void) {
    this.clickOutsideCallback = callback;
    const containerEl = this.wrapper.nativeElement;
    containerEl.querySelector(':first-child').addEventListener('click', this.stopEventPropagation);
    containerEl.addEventListener('click', this.clickOutsideCallback, false);
  }

However clearly in the case of things, i missed the fact that the first child (even in my demo) is actually the .modal-dialog a container which is used to center the actual content... i could i suppose add a class to the configuration 'contentClass' which you can set based on implementation to your 'content class' of choice, in yours and other Bootstrap user cases .modal-content...

[kevcjones-archived]

@artemzakharov take at #15 's PR and tell me if this would see ok for you? I'm trying to NOT bring in bootstrap's classNames to the selectors etc. Tested on the demo internally and it worked fine

[artemzakharov]

@KevCJones That would work for me. It's a wee bit less maintainable but directly passing in the class is pretty powerful and opens up new possibilities, so an improvement on the whole imo 👍

Re: CSS, it's true that it's possible to style the modals giving them unique classes/ids, but it's a little messy in my particular case since the modal pops up well outside of the Angular component that calls it, so the styling would have to go in a global style file that's in a completely different part of the project.

On second thought, I haven't previously considered that I'll need to style the rest of the modal anyway, so even if one option was an argument, the rest of the styling would still need to be done in the same place, so making it an option is kind of moot. It could still be useful if you'd like to make the modals a little easier to look reasonably good out of the box, but I've come to realize it's far from requirement.

[kevcjones-archived]

Yeah this was a similar line of thinking we had when we decided to remove the option. If i need to make my modals look right for 'said' project, why seperate the background color alone as a configurative component. The only time we saw this being useful was if the implementer was using the out-of-the-box styles bootstrap or other components used by default... which tbh is more the exception than the rule.

I'll let @celsomtrindade review but all in all should merge and bump the version later on today.

[celsomtrindade]

@artemzakharov one observation I have on this topic.

I was doing some tests with the project and I tried to focus on this issue specific. I was able to close the modal box by clicking anywhere on the page (tested on chrome, firefox and edge - all on windows).

I said this only happens when you try to click on the sides, isn't it possible that you have something in bettwen, or any other css related issue that affects the modal container (or the container it's within)?

I'm using the application on the demo folder of this project and everything is working fine. I'm not saying the fix shouldn't be merged (maybe it's a improve to the code). But maybe there is something else in your code that is affecting this.

If, for example, I remove the position fixed from the .modal css it will affect the close outside.

[kevcjones-archived]

I was able to reproduce in our demo weirdly enough. The implementation of alert, with bootstrap. The first child was .modal-dialog and was full width and it child .modal-content was margin auto to center it. The issue was that first child was our click blocker because of the recent update i put in to remove bootstrap class name coupling from the component.

I suppose you 'could' change bootstrap's css, but, i think we owe our implementers some amount of friction-free implementing.. I don't doubt we'll want to relook at this later, its not my ideal situation to be in. It might be something we want to configure once during the modal configuration? Do once, decided by your platform?

On 18 December 2017 at 11:15, Celso Trindade notifications@github.com wrote:

@artemzakharov https://github.com/artemzakharov one observation I have on this topic.

I was doing some tests with the project and I tried to focus on this issue specific. I was able to close the modal box by clicking anywhere on the page (tested on chrome, firefox and edge - all on windows).

I said this only happens when you try to click on the sides, isn't it possible that you have something in bettwen, or any other css related issue that affects the modal container (or the container it's within)?

I'm using the application on the demo folder of this project and everything is working fine. I'm not saying the fix shouldn't be merged (maybe it's a improve to the code). But maybe there is something else in your code that is affecting this.

If, for example, I remove the position fixed from the .modal css it will affect the close outside.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/14#issuecomment-352397306, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqe8H0u5L2XNTtibnpAXBekhhsc9L00ks5tBkk4gaJpZM4REkyZ .

-- Kevin C Jones