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 <div>s and to the actual
trigger/control <a>s
- fix incorrect test assertion text output false -> true

diff --git a/docs/components/collapse.md b/docs/components/collapse.md
index 0a49db562f54bb2e54e1ff2da4ba1be7dc44d5c2..a86678ad18ceb78c9112dbbef665dcf8a493a543 100644 (file)
--- 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 88428310d3943442f9d966224dbad507239a648b..dec272297b5ab5a48a9811ec080d5d88f0f92a00 100644 (file)
--- 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 e7083f56ddc9964248d60358d5d7d0c2dd0aee62..35fcf2108ee0c70867e859680cc9bcf5af5fa83d 100644 (file)
--- 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 () {
     $('<div id="test1"/>')
       .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 () {
     $('<div id="test1" class="show"/>')
       .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 () {
         + '</div>'
     var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')
 
-    var $target1 = $('<a role="button" data-toggle="collapse" href="#body1"/>').appendTo($groups.eq(0))
+    var $target1 = $('<a role="button" data-toggle="collapse" aria-expanded="true" href="#body1"/>').appendTo($groups.eq(0))
 
-    $('<div id="body1" aria-expanded="true" class="show" data-parent="#accordion"/>').appendTo($groups.eq(0))
+    $('<div id="body1" class="show" data-parent="#accordion"/>').appendTo($groups.eq(0))
 
-    var $target2 = $('<a role="button" data-toggle="collapse" href="#body2" class="collapsed" aria-expanded="false" />').appendTo($groups.eq(1))
+    var $target2 = $('<a role="button" data-toggle="collapse" aria-expanded="false" href="#body2" class="collapsed" aria-expanded="false" />').appendTo($groups.eq(1))
 
-    $('<div id="body2" aria-expanded="false" data-parent="#accordion"/>').appendTo($groups.eq(1))
+    $('<div id="body2" data-parent="#accordion"/>').appendTo($groups.eq(1))
 
-    var $target3 = $('<a class="collapsed" data-toggle="collapse" role="button" href="#body3"/>').appendTo($groups.eq(2))
+    var $target3 = $('<a class="collapsed" data-toggle="collapse" aria-expanded="false" role="button" href="#body3"/>').appendTo($groups.eq(2))
 
-    $('<div id="body3" aria-expanded="false" data-parent="#accordion"/>')
+    $('<div id="body3" data-parent="#accordion"/>')
       .appendTo($groups.eq(2))
       .on('shown.bs.collapse', function () {
-        assert.strictEqual($target1.attr('aria-expanded'), 'false', 'inactive target 1 has aria-expanded="false"')
-        assert.strictEqual($target2.attr('aria-expanded'), 'false', 'inactive target 2 has aria-expanded="false"')
-        assert.strictEqual($target3.attr('aria-expanded'), 'true', 'active target 3 has aria-expanded="false"')
+        assert.strictEqual($target1.attr('aria-expanded'), 'false', 'inactive trigger/control 1 has aria-expanded="false"')
+        assert.strictEqual($target2.attr('aria-expanded'), 'false', 'inactive trigger/control 2 has aria-expanded="false"')
+        assert.strictEqual($target3.attr('aria-expanded'), 'true', 'active trigger/control 3 has aria-expanded="true"')
 
         done()
       })