[BUG] AccordionItemPanel does not have role at all

[DraikoNick]

React Accessible Accordion documentation says that it applies appropriate role attributes (button, heading, region).
But sometimes this action does not work properly.
For example there are an examples from React Accessible Accordion examples:

1. example with existing role (region) in the AccordionItemPanel: click to check example of the highlighted title
   <div data-accordion-component="AccordionItemPanel" class="accordion__panel" role="region" aria-labelledby="accordion__heading-raa-0" id="accordion__panel-raa-0">In pariatur <a href="#">excepteur</a> ut do aliquip qui mollit aliqua exercitation <a href="#">excepteur</a> consequat reprehenderit nostrud laborum voluptate veniam non dolore dolore aliqua incididunt amet nisi minim cillum elit.</div>

2. example without any role of the AccordionItemPanel: click to check example of the highlighted title
   <div data-accordion-component="AccordionItemPanel" class="accordion__panel" aria-hidden="false" aria-labelledby="accordion__heading-raa-6" id="accordion__panel-raa-6">In pariatur <a href="#">excepteur</a> ut do aliquip qui mollit aliqua exercitation <a href="#">excepteur</a> consequat reprehenderit nostrud laborum voluptate veniam non dolore dolore aliqua incididunt amet nisi minim cillum elit.</div>

There is explanation from axeDevTools: axe.deque.com

[holloway]

Good catch, I'll look at this today.

[holloway]

@DraikoNick it seems the role="region" logic is based on allowMultipleExpanded,

 role: this.allowMultipleExpanded ? undefined : 'region',

https://github.com/springload/react-accessible-accordion/blob/main/src/helpers/AccordionStore.ts#L91

I'll ask the previous developers what the justification for this is, but I'd guess that it's based on this Aria advice,

Optionally, each element that serves as a container for panel content has role region and aria-labelledby with a value that refers to the button that controls display of the panel.

• Avoid using the region role in circumstances that create landmark region proliferation, e.g., in an accordion that contains more than approximately 6 panels that can be expanded at the same time.
• Role region is especially helpful to the perception of structure by screen reader users when panels contain heading elements or a nested accordion.

Although "Optional" the omission of role="region" in the current behaviour of RAA is for accordions with multiple panels, not just 6+ panels, so I think there's a good justification for changing this behaviour in RAA to better adhere to this advice.

"approximately 6 panels that can be expanded" so there's a question of how fuzzy the "approximately 6" is (exactly 6 or not?), and whether we should vary behaviour based on the number of panels exceeding "approximately 6".

I think RAA should apply the region regardless, but I'll consult with others before making this change.

[CoraDale]

Whether or not the accordion panels should have region roles is dependant on the content in them. If it is significant content, like an entire section of the page, then it should have a region role. If it is something minor like some extra info on a subject, then it should not have a region role and can just be a <div>.

Using 'region' only on accordions which only allow one panel open at a time is a decent base rule to make sure there aren't too many regions on the page. But ultimately, what and where landmarks should be used depends entirely on the page content.

The AXE issue being picked up is that <div> elements don't support aria-labeldby or aria-label very well. These attributes are allowed to be used on <div> elements, but they won't do anything. This doesn't break any requirements, but it does fail best practice. (it would actually be bad if the <div> element did use the label, as that may 'overwrite' the text within the div.)

So if the role="region" is not used on an accordionpanel, it needs to also not have the aria-labelledby attribute.

[holloway]

So we think the solution is,

• Have regions off by default but opt-in via a prop on the panel isPageRegion={true | false}
• Update docs accordingly
• Only use aria-labelledby if isPageRegion={true}
• Release a new major version number because this is a breaking change in behaviour

Thoughts?

[CoraDale]

For people's information, we settled on this because:

We definitely want users to fully control whether their accordion panels are regions or not, as accordions can be used either for extra info, or for major page sections, regardless of what other settings the accordions are using. It's much easier for us to make this configurable than for users to edit their own accordions when needed.

We're defaulting to having the panels not as regions because:

• People will probably have fewer 'region' accordions than 'non-region' accordions.
• It's a much bigger accessibility issue if websites use too many regions than if they don't use regions when they should.
• W3C's ARIA practices suggest the 'region' role is optional on accordion panels, rather than expected.

[holloway]

Thanks to @synecdokey and @CalinDale this is now released as version 4.0.0. I'll close this ticket now @DraikoNick but feel free to comment when you try it. Cheers