Skip to content

Commit

Permalink
fix(isFocusable): return true for summary element and false for detai…
Browse files Browse the repository at this point in the history
…ls element with summary child (#1957)
  • Loading branch information
straker committed Jan 22, 2020
1 parent 937a99f commit 34ec2d7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 6 deletions.
4 changes: 3 additions & 1 deletion lib/commons/dom/is-focusable.js
Expand Up @@ -65,9 +65,11 @@ dom.isNativelyFocusable = function(el) {
return el.type !== 'hidden';
case 'TEXTAREA':
case 'SELECT':
case 'DETAILS':
case 'SUMMARY':
case 'BUTTON':
return true;
case 'DETAILS':
return !el.querySelector('summary');
}
return false;
};
Expand Down
4 changes: 2 additions & 2 deletions test/checks/keyboard/focusable-not-tabbable.js
Expand Up @@ -44,7 +44,7 @@ describe('focusable-not-tabbable', function() {
);
});

it('returns false when focusable SUMMARY element, that cannot be disabled', function() {
it('returns true when focusable SUMMARY element, that cannot be disabled', function() {
var params = checkSetup(
'<details id="target" aria-hidden="true"><summary>Some button</summary><p>Some details</p></details>'
);
Expand All @@ -53,7 +53,7 @@ describe('focusable-not-tabbable', function() {
assert.lengthOf(checkContext._relatedNodes, 1);
assert.deepEqual(
checkContext._relatedNodes,
Array.from(fixture.querySelectorAll('details'))
Array.from(fixture.querySelectorAll('summary'))
);
});

Expand Down
38 changes: 35 additions & 3 deletions test/commons/dom/is-focusable.js
Expand Up @@ -134,13 +134,29 @@ describe('is-focusable', function() {
assert.isFalse(axe.commons.dom.isFocusable(el));
});

it('should return true for a details element', function() {
it('should return true for a summary element', function() {
fixture.innerHTML =
'<details><summary id="target">Summary</summary><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isTrue(axe.commons.dom.isFocusable(el));
});

it('should return true for a details element without a summary element', function() {
fixture.innerHTML = '<details id="target"><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isTrue(axe.commons.dom.isFocusable(el));
});

it('should return false for a details element with a summary element', function() {
fixture.innerHTML =
'<details id="target"><summary>Summary</summary><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isFalse(axe.commons.dom.isFocusable(el));
});

it('should return false for a div with no tabindex', function() {
fixture.innerHTML = '<div id="target"></div>';
var el = document.getElementById('target');
Expand Down Expand Up @@ -360,11 +376,27 @@ describe('is-focusable', function() {
assert.isFalse(axe.commons.dom.isNativelyFocusable(el));
});

it('should return true for a details element', function() {
it('should return true for a summary element', function() {
fixture.innerHTML =
'<details><summary id="target">Summary</summary><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isTrue(axe.commons.dom.isFocusable(el));
});

it('should return true for a details element without a summary element', function() {
fixture.innerHTML = '<details id="target"><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isTrue(axe.commons.dom.isNativelyFocusable(el));
assert.isTrue(axe.commons.dom.isFocusable(el));
});

it('should return false for a details element with a summary element', function() {
fixture.innerHTML =
'<details id="target"><summary>Summary</summary><p>Detail</p></details>';
var el = document.getElementById('target');

assert.isFalse(axe.commons.dom.isFocusable(el));
});

it('should return false for a div with no tabindex', function() {
Expand Down

0 comments on commit 34ec2d7

Please sign in to comment.