[material-ui][Accordion] AccordionSummary should be rendered as a heading that wraps a button, per W3C Accordion Pattern standards

[natsid]

Steps to reproduce

Link to live example: https://stackblitz.com/edit/github-zesdz2?file=src%2FApp.tsx

Steps:

1. Open stackblitz
2. Observe that the summary section of the Accordion does not have a heading anywhere

Current behavior

The Material UI Accordion component neglects to meet the following W3C Accordion Pattern standard:

Each accordion header button is wrapped in an element with role heading that has a value set for aria-level that is appropriate for the information architecture of the page.

(source: W3C Accordion Pattern (Sections With Show/Hide Functionality))

Expected behavior

The Accordion or AccordionSummary should provide some way to wrap the element that has role="button" in some sort of heading element.

For example:

<div class="MuiAccordion-root">
  <h2>
    <div class="MuiButtonBase-root MuiAccordionSummary-root" role="button" aria-expanded="">
        <!-- Accordion header button goes here -->
    </div>
  </h2>
  <div class="MuiAccordion-region" role="region">
    <div class="MuiAccordionDetails-root">
      <!-- Accordion content goes here -->
    </div>
  </div>
</div>

OR:

<div class="MuiAccordion-root">
  <div role="heading" aria-level="2">
    <div class="MuiButtonBase-root MuiAccordionSummary-root" role="button" aria-expanded="">
        <!-- Accordion header button goes here -->
    </div>
  </div>
  <div class="MuiAccordion-region" role="region">
    <div class="MuiAccordionDetails-root">
      <!-- Accordion content goes here -->
    </div>
  </div>
</div>

Context

I have tried numerous ways to achieve this structure (<h2 ...><div role="button" ...> ... </div></h2>) while still using the Material UI Accordion, AccordionSummary, and AccordionDetails with no success. Here are some of the things I have tried already:

Other solutions considered

I have attempted to avoid a total rewrite of the component, instead continuing to rely on Material UI (MUI) Accordion. However, none of my attempts proved fruitful. I have documented them here to save future toil on these dead-end paths. That being said, perhaps there is some other way to continue using MUI Accordion while still meeting the above requirement that I haven’t attempted.

✗ Pass in a React fragment that is an h2 to the AccordionSummary

Summary of solution

Each instance of the ExpandablePanel is made to pass in a title that is a heading (specifically an h2 or equivalent). This gets forwarded to the AccordionSummary.

Why this didn’t work

The AccordionSummary ultimately renders as a div with role=”button”. This is good, as the accordion summary should be a button. But the requirement is that the heading wraps the button and not the other way around. This setup has the button wrapping the h2 (essentially something like <button …><h2 …>...). While I thought we might be able to get away with this, unfortunately the outer button ends up obscuring the inner h2, so that it is not recognized as a heading. This is evident by the fact that the screen-reader does not read out “heading level 2” when the summary is in focus.

✗ Wrap the AccordionSummary in an h2

Summary of solution

The ExpandablePanel is updated to look roughly like:

<Accordion ...>
  <h2>
    <AccordionSummary ...></AccordionSummary>
  </h2>
  <AccordionDetails ...></AccordionDetails>
</Accordion

Why this didn’t work

At first it seemed promising. The AccordionSummary was rendered with a div with role=”button”, so we did indeed have the h2 wrapping the button, as desired. However, unfortunately, this ended up breaking some of the functionality of the MUI Accordion. For example, the aria-controls and aria-describedby properties were not getting set properly.

✗ Pass in component=“h2” to AccordionSummary

Summary of solution

The ExpandablePanel is updated to look roughly like:

<Accordion ...>
  <AccordionSummary component="h2"...></AccordionSummary>
  <AccordionDetails ...></AccordionDetails>
</Accordion

Why this didn’t work

As mentioned earlier, the AccordionSummary is typically rendered as a div with role=”button”. Setting component=”h2” meant that we ended up with something like <h2 role=”button” …>...</h2>. This causes [...] errors since a heading (such as an h2) implicitly has role=”heading”, which conflicts with the explicit setting of role=”button”. Semantically, one element cannot have two different roles.

Your environment

npx @mui/envinfo

``` System: OS: macOS 12.7.5 Binaries: Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm pnpm: 9.5.0 - ~/.nvm/versions/node/v20.9.0/bin/pnpm Browsers: Chrome: 126.0.6478.127 Edge: Not Found Safari: 17.5 npmPackages: @emotion/react: 11.11.1 => 11.11.1 @emotion/styled: 11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.21 @mui/core-downloads-tracker: 5.14.15 @mui/icons-material: ^5.14.3 => 5.14.15 @mui/material: ^5.14.5 => 5.14.15 @mui/private-theming: 5.14.15 @mui/styled-engine: 5.14.15 @mui/styles: ^5.14.5 => 5.14.15 @mui/system: 5.14.15 @mui/types: 7.2.7 @mui/utils: 5.14.15 @types/react: ^18.2.31 => 18.2.31 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: 4.9.4 => 4.9.4 ```

Search keywords: Accordion AccordionSummary heading role accessibility w3c WCAG standards a11y

[ZeeshanTamboli]

@natsid Thanks for reporting the issue with a clear description 👍. You are correct. @DiegoAndai We should consider this in the component refactor. It might be a breaking change if we implement it now or maybe not, but we should treat it as a bug since we claim to provide full accessibility support. @natsid Feel free to create a pull request if you'd like.

[natsid]

Thank you for the quick reply, @ZeeshanTamboli ! I haven't looked into how involved the PR would be. At first glance it seems simple - just add a wrapper div with role="heading" and aria-level="?". However, I think we'd also need or at least want to add a new (optional) input on AccordionSummary for the aria-level value. I do think you're right that this could be a breaking change since users' CSS selectors may depend on the current structure. In addition, whatever default value we choose for aria-level could also be considered "breaking" since it may conflict with existing heading structures where it is used.

Do you know if we'd be able to patch this into v5 or if it would only make sense to add into v6?

[ZeeshanTamboli]

@natsid Thanks for your insights on why it could be a breaking change. It makes sense to add it in v6 or v7, not into v5.

[natsid]

Ah, OK @ZeeshanTamboli . Just trying to get a temperature check on how firm that is. I'm personally invested in seeing the fix in v5 since that is what my project is in and v6 is still alpha - and we wouldn't be able to update to v6 until it is stable. Is there any way I can make the case for a v5 bug fix?

[ZeeshanTamboli]

@natsid I created PR #42914 to fix this. In my opinion, it's not a breaking change, so it can be included in v5, but it depends on the reviewers. If not, v6 will be stable soon, targeting July 28th as per the latest release note.

[natsid]

Thank you so much @ZeeshanTamboli !