From 18e8704221791e70d0bf4ac9ff45d4e897a02e63 Mon Sep 17 00:00:00 2001
From: "Patrick H. Lauke" <redux@splintered.co.uk>
Date: Mon, 10 Apr 2017 14:51:22 +0100
Subject: 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
---
 js/src/collapse.js        |  4 ----
 js/tests/unit/collapse.js | 32 ++++++++++++++++----------------
 2 files changed, 16 insertions(+), 20 deletions(-)

(limited to 'js')

diff --git a/js/src/collapse.js b/js/src/collapse.js
index 88428310d..dec272297 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 e7083f56d..35fcf2108 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 () {
     $('<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()
       })
-- 
cgit v1.2.3