-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(aria/menu): Add label property for proper aria-label #32710
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
base: main
Are you sure you want to change the base?
Conversation
47dd30d to
ee239a9
Compare
|
Deployed dev-app for 2df5448 to: https://ng-dev-previews-comp--pr-angular-components-32710-dev-i65jstu7.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
|
Nice. We definitely missed the labelling story from the initial launch. A couple things to consider
|
d0c57ea to
2df5448
Compare
Thanks for the additional context. I will remove the label property in that case, to not confuse the issue. I also see we have searchTerm here but it doesn't seem to be used for anything. I see just one example using the Label behavior, and at least one other place where we set aria-label to the value. I'll just do the latter for now to unblock testing, and then do another PR for clean-up and a better solution for this and others. |
Without an explicit aria-label set on the menu item, screen readers will just concatenate the text contents of the inner elements, which can result in a confusing announcement including the label as well as other secondary content like icon names.
Instead, setting the aria-label (to a specified label or else defaulting to the value) will result in a proper announcement.