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(material/badge): automatically set aria-hidden #27796

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlexanderMelde
Copy link

This PR solves issue #27705 by automatically setting aria-hidden=true if mat-icons have a visible badge.

Previously, badges on mat-icons caused a warning, instructing the developer to set this attribute appropriately.

Solve issue angular#27705 by automatically setting `aria-hidden=true` if mat-icons have a visible badge. Previously, badges on mat-icons caused a warning instructing the developer to set this attribute appropriately.
@AlexanderMelde
Copy link
Author

Hi @wagnermaciel, this PR acts as an alternative to #27717, which you reviewed, so I thought maybe you can review this one as well :)

@mmalerba mmalerba requested review from wagnermaciel and removed request for jelbourn and andrewseguin September 26, 2023 20:22
@AlexanderMelde
Copy link
Author

Hello @wagnermaciel, is there anything I can do to help you with the review? :)

@AlexanderMelde
Copy link
Author

Hello Reviewers, could you please provide me with an update of the review status?

@AlexanderMelde
Copy link
Author

Good Morning @wagnermaciel , any updates? The issue persists.

@AlexanderMelde
Copy link
Author

Hello @crisbeto, I see you are the most active contributor in this project. Could you please provide me with an status of the current state of development in this repository? Are you still actively developing this and accepting PRs?

@wagnermaciel
Copy link
Contributor

@AlexanderMelde Hi Alexander, I'm not sure how I've missed your previous messages but I wanted to apologize for the delay here. We've been heads down on some other work and this PR has gone unnoticed for way too long

I'm not sure if automatically setting aria-hidden for the user is the desired fix so I will reach out to the rest of the team to see if this change is accepted. I'll post an update here once I have a response. Thank you so much for your patience

@AlexanderMelde
Copy link
Author

@AlexanderMelde Hi Alexander, I'm not sure how I've missed your previous messages but I wanted to apologize for the delay here. We've been heads down on some other work and this PR has gone unnoticed for way too long

I'm not sure if automatically setting aria-hidden for the user is the desired fix so I will reach out to the rest of the team to see if this change is accepted. I'll post an update here once I have a response. Thank you so much for your patience

Thank you for the response and describing your situation, I'm happy to hear that you are discussing the topic!

@wagnermaciel
Copy link
Contributor

Meeting recap

Ok, just got done speaking with the team on this. The final verdict was that we will need to go with a different route for this fix. The reason we can't go with this route is that making the mat-icon aria-hidden="false" is bad for a11y since the mat-icon text is not meaningful

The correct solution

The correct solution for this will be to remove the warning altogether and then change the mat-badge behavior as follows:

  • If the matBadgeDescription is applied, and the element that has the badge on it is interactive (not aria-hidden), then the mat-badge should use aria-describedBy to communicate with screen readers.
  • Otherwise, if the matBadgeDescription is applied, and the element that has the badge on it is not interactive (is aria-hidden), then the mat-badge should append a span with the description as the next sibling to the element that has the badge on it.

@AlexanderMelde would you be interested in making this change?

@AlexanderMelde
Copy link
Author

AlexanderMelde commented Jan 30, 2024

@wagnermaciel Thank you for the detailed recap!
I discussed this with my team and while supporting your proposal, unfortunately we are unable to prioritize this topic at the moment.
I will try to work on an initial draft during my available free time, however, I cannot provide you with a specific timeline yet.
I'll keep you updated on any developments.

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.

None yet

2 participants