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 accessibility issue - role required when aria-expanded attribute is used #9445

Merged
merged 4 commits into from
Mar 31, 2023

Conversation

keylime-unicorn
Copy link
Contributor

Addresses #9415

Add a role to the div elements with the aria-expanded attribute. The role form is likely the best option in these scenarios.

@keylime-unicorn keylime-unicorn requested a review from a team as a code owner March 30, 2023 19:35
advay26
advay26 previously approved these changes Mar 30, 2023
@@ -577,7 +577,7 @@ var hlp = new AccordionHelper(name, formModelStatePrefix, expanded, page);
</div>
if (!disabled)
{
<div class="panel panel-default panel-collapse collapse @(expanded ? "in" : string.Empty)"
<div role="form" class="panel panel-default panel-collapse collapse @(expanded ? "in" : string.Empty)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for ALL expandable sections (seeing it's a ViewHelpers) right? Are all expandable sections truly forms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I don't believe they are all forms. Unfortunately, aria-expanded attribute can only be used in a handful of roles. The sections that this div element are used for vary significantly as far as I can tell. The roles that the aria-expanded attribute can be used for are listed here. Any thoughts on this matter would be appreciated @joelverhagen @ryuyu

@keylime-unicorn
Copy link
Contributor Author

Addressed the issue pointed out by @joelverhagen. New fix is actually to remove the use of the aria-expanded attribute entirely as it is used incorrectly here for the content element. The header button already has the aria-expanded attribute so the use here is redundant.

@keylime-unicorn keylime-unicorn merged commit 26d3e1b into dev Mar 31, 2023
@advay26 advay26 mentioned this pull request Apr 8, 2023
9 tasks
@joelverhagen joelverhagen deleted the tt-a11y-9415 branch August 22, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants