Use event.target in cases where _firstTarget has been removed from DOM

[sgparrish]

I have a use case in vis-timeline where _firstTarget ends up getting removed from the DOM during a pan event. This causes the pan to stop immediately because it no longer a descendent of the hammer root element.

[josdejong]

Thanks @sgparrish .

I can imagine that document.contains(...) can be slow when you have a large document. Is there a faster way to check whether the target is still attached to the DOM? Maybe loop over all parentNodes to check if it ends up in the root element where we've attached hammer?

[josdejong]

Thanks for the update. I think this makes sense. However I'm thinking about cases like when a DOM element is rendered but does not have a parent <document>, for example when using Shadow DOM. And I think the same holds for your first approach with document.contains.

I was reading up on this StackOverflow question: https://stackoverflow.com/questions/5649534/html-element-is-attached-to-a-document

This suggests using yourElement.isConnected, which should work in Shadow DOM and IFRAME's too. What do you think about using isConnected?

On a side note: I realize I just assumed that document.contains is slow, but I actually do not know if that is the case 🤔. Anyway.

[sgparrish]

Oh yeah, isConnected looks like the right solution.

Do you think I should include this as an option? I could imagine this breaks some behavior for someone.

Re: side note:
I did some brief research on document.contains after your first comment and did find a benchmark, but I think either I misinterpreted it or it was outdated. Turns out, under the hood (at least for chromium based browsers) document.contains does something similar to the function I wrote. (chromium source, 1406 it does the parentNode traversal)

[josdejong]

I think isConnected can indeed be used instead, like you do with your latest commit.

Do you think we should take care of a possible absence of the isConnected property? It is supported by all browsers except IE (which is EOF already for a long time), so, effectively propagating-hammerjs will not work on IE 11 anymore with this PR.

[josdejong]

I think isConnected can indeed be used instead, like you do with your latest commit.

Do you think we should take care of a possible absence of the isConnected property? It is supported by all browsers except IE (which is EOF already for a long time), so, effectively propagating-hammerjs will not work on IE 11 anymore with this PR.

What do you think @sgparrish ?

[sgparrish]

IE has been dead for over a year at this point. I think just bumping the major version should be sufficient to prevent any existing behavior breaking for consumers expecting propagating-hammerjs to behave in IE11 environments.

If anyones starting new development intending it to be IE11 compatible (god help them) they could be directed to an older version?

If it's actually a strong concern- I suppose the check could be modified to check typeof _firstTarget.isConnected. I think it should resolve to undefined in the IE11 case and boolean in all modern browsers. It's really tough to know, though- my windows 10 box won't even let me open IE anymore. In the IE11 case then some other operation could be done

[josdejong]

Good point. Let's not spend too much effort on keeping it working in IE11. Instead, I'll simply publish it as a major, breaking change.

Thanks for providing this fix!

[josdejong]

Published now in v3.0.0