Add Element.closest

[alexandernanberg]

Polyfill is taken directly from MDN

[inexorabletash]

Thanks for submitting! Looks good, a few suggestions for additional test cases.

Have you tried in IE8 to make sure it at least doesn't raise an exception? (I forget which incantations are needed around Element.prototype since I've dropped support for IE<10 in my hobby projects)

[inexorabletash]

Can you add a case for:

assert.equal(document.querySelector('#foo').closest('#foo'), document.querySelector('#foo'));

And a case where there is no closest ancestor?

(This will also serve as a reminder to re-review and merge this when I have more time)

[alexandernanberg]

Sure, will do that! I'm having some problems getting Qunit to work, which version of Qunit and node are you using? It might be beneficial if we could add Qunit as a devDependency and a npm script to run the tests. Would make testing much simpler for new contributors.

[inexorabletash]

I don't use node myself. You can just hit load the test file directly in a browser, or e.g.:

https://rawgit.com/inexorabletash/polyfill/master/tests/dom.html

And yes, getting continuous integration would be swell. As noted I don't use node myself for any projects, but happy to field further PRs with tests, if there are appropriate instructions.

[alexandernanberg]

Aha, neat. I've added 2 more tests now, will that suffice? I tried to test it in IE 8 in BrowserStack but the tests wont run (got a blank page) even without my modifications. Work fine in IE 9 and up though. Can you check if you get the same problem?

If I get time sometime I might create a CI PR!

[inexorabletash]

I think QUnit dropped support for IE8, which I discovered after upgrading, alas. I think just looking at the console and ensuring the polyfill script doesn't throw would suffice.

…

On Mon, Jul 24, 2017 at 2:33 AM, Alexander Nanberg ***@***.*** > wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/dom_tests.js <#132 (comment)> : > @@ -174,6 +174,11 @@ QUnit.test("matches", function(assert) { assert.ok(document.querySelector('#baz').matches('.beta')); }); +QUnit.test("closest", function(assert) { + assert.equal(document.querySelector('#bar').closest('#foo'), document.querySelector('#foo')); + assert.equal(document.querySelector('#baz').closest('#foo'), document.querySelector('#foo')); +}); Aha, neat. I've added 2 more tests now, will that suffice? I tried to test it in IE 8 in BrowserStack but the tests wont run (got a blank page) even without my modifications. Work fine in IE 9 and up though. Can you check if you get the same problem? If I get time sometime I might create a CI PR! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#132 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAvF27K4JtyzjRXUIDPvQxaXpcAnSLVKks5sRGTsgaJpZM4OfFWH> .

[inexorabletash]

Thanks! I'll wait a couple days for any error reports to roll in, then make a release/push to npm.