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

constructor.prototype.matches also initiated for #document #57

Open
NiekvR opened this issue Aug 6, 2019 · 4 comments
Open

constructor.prototype.matches also initiated for #document #57

NiekvR opened this issue Aug 6, 2019 · 4 comments

Comments

@NiekvR
Copy link

NiekvR commented Aug 6, 2019

We are using ngx-smooth-dnd in our project and are running into a problem with the dependency on this library. Since this library uses a polyfill to set the constructor.prototype.matches clashes with jQuery's closest method (https://api.jquery.com/closest/). Once it reaches the #document element it tries to see if it has el.matches initiated it returns true, but the tries to perform el.matches('selector). Since matches does not work on Nodes this gives the following error: Uncaught TypeError: Cannot read property 'querySelectorAll' of null.

In our opinion this problem is easily resolved by removing Node from the constructor.prototype.matches polyfill:

(function (constructor: any) {
if (constructor && constructor.prototype && !constructor.prototype.matches) {
constructor.prototype.matches =
constructor.prototype.matchesSelector ||
constructor.prototype.mozMatchesSelector ||
constructor.prototype.msMatchesSelector ||
constructor.prototype.oMatchesSelector ||
constructor.prototype.webkitMatchesSelector ||
function (s: string) {
var matches = (this.document || this.ownerDocument).querySelectorAll(s),
i = matches.length;
while (--i >= 0 && matches.item(i) !== this) { }
return i > -1;
};
}
})(Node || Element);

as is already suggested in: #29

We are really enthousiastic about the library and we hope you can fix this for us!

@silvan88
Copy link

silvan88 commented Aug 7, 2019

+1

I'm running into the same problems, can this please be merged asap?

@ptrd
Copy link

ptrd commented Aug 7, 2019

+1

@NiekvR
Copy link
Author

NiekvR commented Aug 27, 2019

Any idea on the status?

@silvan88
Copy link

@kutlugsahin can you give an update? Else I need to look for other options...

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

No branches or pull requests

3 participants