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

[A11y][Fastpass] Remove aria-expanded attribute where not allowed #9348

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

keylime-unicorn
Copy link
Contributor

Removed the aria-expanded attribute from the following div elements:

URL: https://www.nuget.org/packages/manage/upload

  • #verify-package-block
  • #import-readme-block
  • #submit-block

Addresses #9320

@keylime-unicorn keylime-unicorn requested a review from a team as a code owner January 11, 2023 18:35
advay26
advay26 previously approved these changes Jan 11, 2023
@joelverhagen
Copy link
Member

How did we decide on removing the attributes rather than adding additional attributes to make the situation compliant? From the document linked in the issue (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-expanded#:~:text=The%20aria-expanded%20indicates%20to%20assistive%20technology%20whether%20the,on%20the%20elements%20that%20own%20expandable%20grouping%20containers) it seems like we may need an aria-controls or some other additional attribute to make things work? Generally, this seems to be making the DOM less descriptive of the visible page state rather than more so I wonder if this is the right approach.

@keylime-unicorn
Copy link
Contributor Author

@joelverhagen So I noticed that there is an element within this div element that also has the aria-expanded label and actually changes it between true and false when the element is collapsed and expanded. This instance of the aria-expanded attribute seems redundant and essentially seems to serve no purpose other than throwing this error. So it seems to me that this outer div doesn't need the aria-expanded attribute since it is not the element actually being expanded and collapsed. Although I could be wrong with my analysis.

@ryuyu Thoughts?

lyndaidaii
lyndaidaii previously approved these changes Jan 11, 2023
@ryuyu
Copy link
Contributor

ryuyu commented Jan 11, 2023

Since the element this is being removed from is not actually the interactable element, I am fairly certain that this is the correct fix. :shipit:

ryuyu
ryuyu previously approved these changes Jan 11, 2023
@keylime-unicorn keylime-unicorn merged commit b8c3912 into dev Jan 11, 2023
@agr agr mentioned this pull request Jan 26, 2023
10 tasks
@joelverhagen joelverhagen deleted the a11y-fix-aria-expanded branch August 22, 2024 16:34
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.

5 participants