[Accordion] Fix invalid HTML inside heading

[ZeeshanTamboli]

Fixes #44153

This PR resolves a regression introduced in #42914 by making the following changes to ensure valid HTML structure for the Accordion component:

• Changed the Accordion summary element to button.
• Updated the Accordion summary content and icon wrapper elements to span.
• Keep Typography on the demos as before, but with component='span' since the p element rendered by Typography is invalid inside a span.

Minor version change

• The HTML elements of the Accordion summary have been updated:
  • The root element is now button (previously div).
  • Summary content and the icon wrapper are now span (previously div).

How might this impact you:

• Developers using the previous div element for styling in the AccordionSummary should update their styling.

How to fix similar invalid HTML in your codebase

Part of this PR is fixing invalid HTML in our demos. If you copied code from these demos in the past, you might have to do similar changes to your codebase.

If you are using Typography inside the AccordionSummary, which defaults to rendering a p tag, you should use the component prop to replace the HTML tag as such: <Typography component="span" />, as shown in the updated demos.

---

These changes are critical to fix the regression but introduce a breaking change. IMO, this should be released in a minor version (v6.x) and documented in the migration guide. I know this is not ideal but the fix is also important.

---

If approved, I’ll include a note in the migration guide detailing these changes.

[mui-bot]

Netlify deploy preview

• docs/data/material/migration/upgrade-to-v6/upgrade-to-v6.md

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4962eb8

[ZeeshanTamboli]

What about removing the button styles?

How to do that? That's what I did by spreading theme.typography.body1.

[DiegoAndai]

I'm a bit lost.

This is the option I think we should go for:

Option 2: Remove theme.typography.body1 and keep Typography in the demos but use for valid HTML. However, this feels like a workaround.

Why, then, are the typography styles not applied if we're still using the Typography component?

[ZeeshanTamboli]

I'm a bit lost.

This is the option I think we should go for:

Option 2: Remove theme.typography.body1 and keep Typography in the demos but use for valid HTML. However, this feels like a workaround.

Why, then, are the typography styles not applied if we're still using the Typography component?

@DiegoAndai See this sentence: This forces the user to always use a Typography component for styling the summary text.

[DiegoAndai]

This forces the user to always use a Typography component for styling the summary text.

But this was the case previously as well, isn't it? This is why we have Typography on the demos.

Incidentaly, the first demo doesn't use Typography, and here is its sandbox: https://codesandbox.io/embed/np3qck?module=/src/Demo.tsx&fontsize=12. (Doesn't look like that on the docs thanks to the docs theme)

In contrast, here's on of the demos that use Typography: https://codesandbox.io/embed/6gjm9h?module=/src/Demo.tsx&fontsize=12

[DiegoAndai]

Maybe we could:

• Change the Accordion summary element to button.
• Update the Accordion summary content and icon wrapper elements to span.
• Keep Typography on the demos as before, but with component='span'

And release that as a minor. Would that fix the issue?

cc: @aarongarciah

[ZeeshanTamboli]

@DiegoAndai

I've made the changes. The first demo no longer uses Typography, so Argos CI flagged the change (aside from the flaky progress component test). I wasn't aware the first demo differed between CodeSandbox and the docs, but now they're the same. The docs theme isn't applied—is that okay? I think it is because the HTML button styles are applied, as shown in the screenshot:

This PR: https://deploy-preview-44408--material-ui.netlify.app/material-ui/react-accordion/#introduction

Production: https://mui.com/material-ui/react-accordion/#introduction)](https://mui.com/material-ui/react-accordion/#introduction

I've also pushed a commit to fix in the Marketing page template: db65312

[aarongarciah]

@ZeeshanTamboli can you update the first demo to include the Typography component so it looks the same as the other demos?

[ZeeshanTamboli]

@ZeeshanTamboli can you update the first demo to include the Typography component so it looks the same as the other demos?

@aarongarciah Updated.

[aarongarciah]

Looks good! Thanks for your patience.

@ZeeshanTamboli I think only 2 things are missing:

• Add an entry in the migration guide explaining the change.
• Update the PR description to reflect the latest changes in the PR.

In the next release, we'll explain the change, too.

[ZeeshanTamboli]

@aarongarciah

• Add an entry in the migration guide explaining the change.

Added. https://deploy-preview-44408--material-ui.netlify.app/material-ui/migration/upgrade-to-v6/#accordion-starting-from-v6-3-0

• Update the PR description to reflect the latest changes in the PR.

Updated

[DiegoAndai]

LGTM ✅

As always, @ZeeshanTamboli, thanks for working on this and willingness to discuss approaches to get the best solution for the community.

Let's wait for a final review from @aarongarciah.

[DiegoAndai]

CC: @mnajdova for next week's release: this requires a minor bump and a mention on the CHANGELOG.

[aarongarciah]

Nice!

[DiegoAndai]

Some thoughts about this being "breaking" or not

This should be considered a fix and not a breaking change. It doesn't change any of our public APIs.

Imagine a function that doesn't work as expected, and we fix it (without changing the public API). Users who accounted for the previous (broken) behavior might perceive it as a breaking change. Users who expect the new (fixed) behavior perceive it as a fix. This is the same.

I agree it's an edge case. Releasing it in a minor version with an explanation of how to adapt is a good compromise between both. We need to be able to make these compromises to move forward.

Besides, I wouldn't consider the tags as public API, which is what Semver uses to classify something as breaking. That's not to say we should change them without caution, but we should be able to do it when necessary.

I'm removing the "breaking" label and instead naming it "minor version change". I also updated the PR's description accordingly.