Add includeContainer option #15
Conversation
There was a problem hiding this comment.
Nice, @vonagam!
Could you add some documentation and a test or two to finish the feature?
index.js
Outdated
| return matches.call(el, candidateSelector); | ||
| }) | ||
| ) { | ||
| candidates.unshift(el); |
There was a problem hiding this comment.
Wouldn't we want to push this directly into orderedTabbables?
There was a problem hiding this comment.
Well, for my use case - no. I wanted to apply same rules to container. So if it has tabindex -1 or disabled or hidden it should not get in resulted list.
There was a problem hiding this comment.
And if it has tabindex = 0, then if we push it into orderedTabbables then it will appear after all basicTabbables but before any other orderedTabbables, which is incorrect result.
| @@ -18,6 +20,18 @@ module.exports = function(el) { | |||
|
|
|||
| var candidates = el.querySelectorAll(candidateSelectors); | |||
|
|
|||
| if (options.includeContainer) { | |||
| var matches = Element.prototype.matches || Element.prototype.msMatchesSelector || Element.prototype.webkitMatchesSelector; | |||
There was a problem hiding this comment.
Interesting! I didn't know about this function.
|
Here, reused "nested" fixture for the test. Added note about options in Usage section. |
|
Thanks again, @vonagam. Published in 1.1.0. |
Here is PR for #14.
Uses matches and some both support IE9.