From 18e8704221791e70d0bf4ac9ff45d4e897a02e63 Mon Sep 17 00:00:00 2001 From: "Patrick H. Lauke" Date: Mon, 10 Apr 2017 14:51:22 +0100 Subject: [PATCH] Fix collapse.js aria-expanded behavior * Remove aria-expanded from collapse.js target element aria-expanded="true"/aria-expanded="false" only applies to the trigger, not the element that is being expanded/collapsed. * Tweak collapse.js accessibility section ...to make it clearer that the aria-expanded attribute always just goes on the control. * Fix collapse.js unit tests - reword some of the text to make it clear we're checking behavior of trigger/control - move incorrect aria-expanded out of the
s and to the actual trigger/control s - fix incorrect test assertion text output false -> true --- docs/components/collapse.md | 2 +- js/src/collapse.js | 4 ---- js/tests/unit/collapse.js | 32 ++++++++++++++++---------------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/docs/components/collapse.md b/docs/components/collapse.md index 0a49db562f..a86678ad18 100644 --- a/docs/components/collapse.md +++ b/docs/components/collapse.md @@ -119,7 +119,7 @@ You can also create accordions with custom markup. Add the `data-children` attri ## Accessibility -Be sure to add `aria-expanded` to the control element. This attribute explicitly defines the current state of the collapsible element to screen readers and similar assistive technologies. If the collapsible element is closed by default, it should have a value of `aria-expanded="false"`. If you've set the collapsible element to be open by default using the `show` class, set `aria-expanded="true"` on the control instead. The plugin will automatically toggle this attribute based on whether or not the collapsible element has been opened or closed. +Be sure to add `aria-expanded` to the control element. This attribute explicitly conveys the current state of the collapsible element tied to the control to screen readers and similar assistive technologies. If the collapsible element is closed by default, the attribute on the control element should have a value of `aria-expanded="false"`. If you've set the collapsible element to be open by default using the `show` class, set `aria-expanded="true"` on the control instead. The plugin will automatically toggle this attribute on the control based on whether or not the collapsible element has been opened or closed (via JavaScript, or because the user triggered another control element also tied to the same collapsbile element). Additionally, if your control element is targeting a single collapsible element – i.e. the `data-target` attribute is pointing to an `id` selector – you may add an additional `aria-controls` attribute to the control element, containing the `id` of the collapsible element. Modern screen readers and similar assistive technologies make use of this attribute to provide users with additional shortcuts to navigate directly to the collapsible element itself. diff --git a/js/src/collapse.js b/js/src/collapse.js index 88428310d3..dec272297b 100644 --- a/js/src/collapse.js +++ b/js/src/collapse.js @@ -162,7 +162,6 @@ const Collapse = (($) => { .addClass(ClassName.COLLAPSING) this._element.style[dimension] = 0 - this._element.setAttribute('aria-expanded', true) if (this._triggerArray.length) { $(this._triggerArray) @@ -223,8 +222,6 @@ const Collapse = (($) => { .removeClass(ClassName.COLLAPSE) .removeClass(ClassName.SHOW) - this._element.setAttribute('aria-expanded', false) - if (this._triggerArray.length) { $(this._triggerArray) .addClass(ClassName.COLLAPSED) @@ -300,7 +297,6 @@ const Collapse = (($) => { _addAriaAndCollapsedClass(element, triggerArray) { if (element) { const isOpen = $(element).hasClass(ClassName.SHOW) - element.setAttribute('aria-expanded', isOpen) if (triggerArray.length) { $(triggerArray) diff --git a/js/tests/unit/collapse.js b/js/tests/unit/collapse.js index e7083f56dd..35fcf2108e 100644 --- a/js/tests/unit/collapse.js +++ b/js/tests/unit/collapse.js @@ -322,7 +322,7 @@ $(function () { $target3.trigger('click') }) - QUnit.test('should set aria-expanded="true" on target when collapse is shown', function (assert) { + QUnit.test('should set aria-expanded="true" on trigger/control when collapse is shown', function (assert) { assert.expect(1) var done = assert.async() @@ -338,7 +338,7 @@ $(function () { $target.trigger('click') }) - QUnit.test('should set aria-expanded="false" on target when collapse is hidden', function (assert) { + QUnit.test('should set aria-expanded="false" on trigger/control when collapse is hidden', function (assert) { assert.expect(1) var done = assert.async() @@ -364,8 +364,8 @@ $(function () { $('
') .appendTo('#qunit-fixture') .on('shown.bs.collapse', function () { - assert.strictEqual($target.attr('aria-expanded'), 'true', 'aria-expanded on target is "true"') - assert.strictEqual($alt.attr('aria-expanded'), 'true', 'aria-expanded on alt is "true"') + assert.strictEqual($target.attr('aria-expanded'), 'true', 'aria-expanded on trigger/control is "true"') + assert.strictEqual($alt.attr('aria-expanded'), 'true', 'aria-expanded on alternative trigger/control is "true"') done() }) @@ -382,15 +382,15 @@ $(function () { $('
') .appendTo('#qunit-fixture') .on('hidden.bs.collapse', function () { - assert.strictEqual($target.attr('aria-expanded'), 'false', 'aria-expanded on target is "false"') - assert.strictEqual($alt.attr('aria-expanded'), 'false', 'aria-expanded on alt is "false"') + assert.strictEqual($target.attr('aria-expanded'), 'false', 'aria-expanded on trigger/control is "false"') + assert.strictEqual($alt.attr('aria-expanded'), 'false', 'aria-expanded on alternative trigger/control is "false"') done() }) $target.trigger('click') }) - QUnit.test('should change aria-expanded from active accordion target to "false" and set the newly active one to "true"', function (assert) { + QUnit.test('should change aria-expanded from active accordion trigger/control to "false" and set the trigger/control for the newly active one to "true"', function (assert) { assert.expect(3) var done = assert.async() @@ -401,22 +401,22 @@ $(function () { + '
' var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card') - var $target1 = $('').appendTo($groups.eq(0)) + var $target1 = $('').appendTo($groups.eq(0)) - $('