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

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.
 

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)

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()
       })