fix(frame-focusable-content): don't fail for elements with negative tabindex #3493
Conversation
straker
changed the title
| @@ -45,13 +45,6 @@ describe('frame-focusable-content tests', function() { | |||
| assert.isTrue(frameFocusableContent(null, null, vNode)); | |||
| }); | |||
|
|
|||
| it('should return false if element is natively focusable but has tabindex', function() { | |||
There was a problem hiding this comment.
Turns out IE11 will take the empty tabindex and default it to -3876. But that value of tabindex also didn't prevent the browser from tabbing to the element so ¯\(ツ)/¯
| * @method isFocusable | ||
| * @memberof axe.commons.dom | ||
| * @instance | ||
| * @param {HTMLElement} el The HTMLElement | ||
| * @return {Boolean} The element's focusability status | ||
| */ | ||
|
|
||
| function isFocusable(el) { | ||
| export default function isFocusable(el) { |
There was a problem hiding this comment.
I wonder if maybe we should add some option here to say isFocusable(el, { noNegativeTabindex: true })?
This seem like sort of a common occurance.
There was a problem hiding this comment.
I agree. Though maybe the option could be called programmatically? Either way we should do it in a separate pr.
const focusable = isFocusable(el, { programmatically: false })
| var vNode = queryFixture('<button id="target"><span>Hello</span></button>'); | ||
| var vNode = queryFixture('<div id="target"><span>Hello</span></div>'); |
There was a problem hiding this comment.
Why did you have to change these?
There was a problem hiding this comment.
having the frame-focusable-content test have all the elements wrapped in buttons was strange so changed them to divs
The <button><a></a></button> setup was the one that was the most strange
Co-authored-by: Wilco Fiers <[email protected]>
…abindex (#3493) * fix(frame-focsuable-content): don't fail for elements with negative tabindex * revert playground' * remove test case * Update lib/checks/keyboard/frame-focusable-content-evaluate.js Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Wilco Fiers <[email protected]>
Updated the
isFocusablecode since we were looking at it to 1) default export at top and 2) update the description as we spent a bit of time trying to figure out why an element with tabindex=-1 was still considered focusable.Also, having the
frame-focusable-contenttest have all the elements wrapped inbuttonswas strange so changed them todivsCloses issue: #3466