Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix collapse.js aria-expanded behavior #22397

Merged
merged 5 commits into from
Apr 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/components/collapse.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 0 additions & 4 deletions js/src/collapse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
32 changes: 16 additions & 16 deletions js/tests/unit/collapse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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()
})

Expand All @@ -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()

Expand All @@ -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()
})
Expand Down