The dialog closes when removing the target DOM element of a click event

[CaselIT]

I've found a bug, when I remove an item on the onclick event the dialog closes.
See this example

• click Open Modal
• click Remove
• the dialog closes.

I think I've found the issue also: outside-event-plugin.ts
The function does not find the target element inside the dialog, since it was removed, and runs the clickOutside routine.
At the moment I'm bypassing the issue wrapping the slice in a setTimeout

Also, why use the do while cycle when you could just do as below? (This does not solve the problem btw)

if(element.contains(event.target))
  return;

[shlomiassaf]

Hi @CaselIT

function bubbleNonAncestorHandlerFactory(element: HTMLElement, handler: (event) => void) {
    return (event) => {
        let current = event.target;
        do {
            if ( current === element ) {
                return;
            }
        } while ( current.parentNode && ( current = current.parentNode ) );

        handler(event);
    };
}

You refer to the do/while here?
I go up the tree to make sure the click was not on an dialog element or it's children.

You can have this tree:
DialogElement -> Div Container -> h1, div, span, button .....

Say the click was on button, I have to go up button -> div -> DialogElement
This will cause cancelling the close since were in the dialog, we didn't click outside of it.

[shlomiassaf]

As for why it's closing, It happening because you remove the Button element before the event handler is invoked.
i.e: when bubbleNonAncestorHandlerFactory is invoked the element.target has no parent node and so it "thinks" the click was made outside.

The best solution is to stop the event from bubbling.
In your code:

 remove($event, i){
    $event.stopPropagation();
    this.list.splice(i,1);
  }

If stopPropagation() is not an option due to needs, I believe setTimeout() will also work. Run the removal code in the next VM turn, this will ensure running after the event was handled.

Plunker:
http://plnkr.co/edit/fJb675Ks4vf8ux9t9Rfp?p=preview

[CaselIT]

As for why it's closing, It happening because you remove the Button element before the event handler is invoked.

Yes that is what I figured. I guess there is no simple way to automate this use case. Maybe using elementFromPoint could work even in this case. Still it took me a while to figure out why it happened.

Regarding the code I understood what you are doing

function bubbleNonAncestorHandlerFactory(element: HTMLElement, handler: (event) => void) {
    return (event) => {
        if(element.contains(event.target))
          return;
        handler(event);
    };
}

This should be equivalent but faster since element.contains is native

[CaselIT]

Regarding elementFromPoint I meant somethind like this

function bubbleNonAncestorHandlerFactory(element: HTMLElement, handler: (event) => void) {
    return (event) => {
        const el =  document.elementFromPoint(event.clientX, event.clientY);
        if(element.contains(el))
          return;
        handler(event);
    };
}

I haven't tested it, so I'm not sure it if works. ( also not sure if the position from clientX/clientY is the one to use)

[shlomiassaf]

element.contains will do a check when it was already removed, same async issue here.

One thing that will solve this easily is looking at event.path which is a snapshot of the tree at the moment of click.

so event.path.indexOf(element) will be true sync or async... (same like contains but in the past)

I didn't use it because I don't know the browser competability. I didn't find information in MDN, CanIUse and W3C. It's not clear. So I dropped it.

Anyway, this doesn't impact performance, yes its O(n) but performance is noticed when n is very high, so if your tree's depth is 1,000,000 then it will be noticed, but in such case angular2-modal is the least of your problems :)

[CaselIT]

The second version, with elementFromPoint should work in when removing (except maybe in the case where the dialog has shrinked so mutch that the event position is now outside the dialog. End even here I'm not sure when the shrinking will occur, if before or after the end of the click handle)

Anyway, this doesn't impact performance, yes its O(n) but performance is noticed when n is very high, so if your tree's depth is 1,000,000 then it will be noticed, but in such case angular2-modal is the least of your problems :)

Well of course, but since there is a native function that does the same thing (with less code in this case!) I think it should be preferred.

[shlomiassaf]

I agree, everything you said it true.

But it has to be tested against multiple browsers and on mobile, to make sure the element we get is the right one.

After building ng2-chess and handling with drag and drop I can tell you it's not trivial. Chrome/FF/Safari to each his own.

So if i'm not sure I prefer taking the longer, yet safer route.

[CaselIT]

I feel your pain! elementFromPoint is experimental. contains should be supported instead.

I could make a pr with the improved code checking if contains ,elementFromPoint exist (if not still maintain the current code)

[shlomiassaf]

This is exactly the complexity I want to avoid.

Having this will create an unexpected behaviour, sometimes removing a clicked button from the form will close the form, sometimes not... who knows? depends on the browser.

contains will not help us in this case, elementFromPoint will but it's sometimes there sometimes not...

I prefer "forcing" the developer to "stopPropagation", the other option is to hide the bug from him, letting his users find out about it.

[CaselIT]

I had not thought about it. I agree with you.
I can add this issue to https://github.com/shlomiassaf/angular2-modal#issues-and-todos

[shlomiassaf]

It's better be a bug, just edit this issue to have a proper title

[obSoftMind]

How to fix this : Error parsing header X-XSS-Protection: 1; mode=block; report=https://www.google.com/appserve/security-bugs/log/youtube: insecure reporting URL for secure page at character position 22. The default protections will be applied.

my openModel methode works fine and i get data of result. but i had the error above
openModal(video: any) {
const dialogRef = this.modal.prompt()
.size('lg')
.isBlocking(true)
.showClose(true)
.keyboard(27)
.title('Ajouter à mes fovoris')
.titleHtml('title')
.body('Commentaire facultatif ("à voir demain", "montrer à Snoopy :)"...)')
.bodyClass('modal-body')
.footerClass('modal-footer')
.okBtn('ajouter aux favoris')
.okBtnClass('btn btn-primary')
.open();
dialogRef.result
.then( result => this.modalOk(result, video));
}
modalOk(){
...
}