Skip to content
This repository has been archived by the owner on Aug 26, 2021. It is now read-only.

Accordion wrap toggle button in a heading level #867

Open
LaurenceRLewis opened this issue Aug 7, 2019 · 5 comments
Open

Accordion wrap toggle button in a heading level #867

LaurenceRLewis opened this issue Aug 7, 2019 · 5 comments
Labels
accessibility Issue or pull request related to accessibility. good first issue A good issue to work on first. module: accordion An issue or pull request related to the accordion component.

Comments

@LaurenceRLewis
Copy link

LaurenceRLewis commented Aug 7, 2019

For consideration:

Headings (hx) should be used to wrap the accordion triggering button. This would align with the recommendations of the ARIA 1.1 Practices Guide. The heading level as the first element in the <section>s will also align to the recommendations for the <section> element in the HTML 5.1 W3C Recommendation.

Accordion (Sections With Show/Hide Functionality)
https://www.w3.org/TR/wai-aria-practices-1.1/#accordion

The section element
https://www.w3.org/TR/2016/REC-html51-20161101/sections.html#the-section-element

<section class="au-accordion">
<h3>
<button class="au-accordion__title js-au-accordion" aria-controls="accordion-default" aria-expanded="true" onclick="return AU.accordion.Toggle( this )">
Accordion title
</button>
</h3>
<div class="au-accordion__body" id="accordion-default">
<div class="au-accordion__body-wrapper">
Here <a href="#url">is</a> some accordion content
</div>
</div>
</section>

@sukhrajghuman sukhrajghuman added the accessibility Issue or pull request related to accessibility. label Aug 7, 2019
@sukhrajghuman sukhrajghuman added this to 🌱 Product Backlog in Design System via automation Aug 7, 2019
@sukhrajghuman sukhrajghuman added the good first issue A good issue to work on first. label Aug 7, 2019
@sukhrajghuman
Copy link
Contributor

Nice pick up @LaurenceRLewis !

This should be a pretty quick fix :)

@gordongrace gordongrace added the module: accordion An issue or pull request related to the accordion component. label Aug 8, 2019
@electronicwoft
Copy link

consider removing the aria-controls for adjacent content ... but, if it is going to be used, ensure that it only has a value once the controlled content is visible (otherwise any attempt to move focus will fail)

@LaurenceRLewis
Copy link
Author

I believe it is currently best to still include aria-controls even if the content is adjacent. This is best explained in the comments in the issue: What to do about aria-controls #995 w3c/aria#995

I agree aria-controls should only be exposed when there is a value (not hidden).

@con-cat
Copy link

con-cat commented Oct 12, 2019

Hi, I created a PR to address this issue :)

@electronicwoft
Copy link

I don't believe there's a need to use aria-controls for elements that are adjacent in the DOM. the The HTML section element is not well supported by screen readers (the ARIA example uses region). The section or region also requires an accessible name which can be provided by aria-labelledby pointing to the heading (it will compute from the button). also, be sure to provide accompanying advice that the heading level will be dependent on the information architecture of the context in which the accordian widget occurs. And it may also be worth noting that a solitary expandible element does not an accordian make ... that is, an accordian must have at least two expandible elements otherwise it is not an accordian.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Issue or pull request related to accessibility. good first issue A good issue to work on first. module: accordion An issue or pull request related to the accordion component.
Projects
Design System
  
🌱 Product Backlog
Development

No branches or pull requests

5 participants