-
-
Notifications
You must be signed in to change notification settings - Fork 505
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
Use icon button label as the aria-label to improve accessibility. #1066
Conversation
…it as the aria-label to improve accessibility.
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a7411ef:
|
Converted to draft as I investigate a similar aria problem with the field autocomplete widgets. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1066 +/- ##
=======================================
Coverage 82.07% 82.07%
=======================================
Files 211 211
Lines 10844 10846 +2
Branches 1316 1318 +2
=======================================
+ Hits 8900 8902 +2
Misses 1341 1341
Partials 603 603 ☔ View full report in Codecov by Sentry. |
Thanks for the PR! |
Oops sorry, didn't realise I'd committed the lock file as well.
I'll do that now.
…On Wed, Jun 19, 2024 at 2:19 PM Denys Oblohin ***@***.***> wrote:
Thanks for the PR!
Could you please revert changes to the lock file?
—
Reply to this email directly, view it on GitHub
<#1066 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BJIMSMK3POWRKMDZ6HK2GZ3ZIGAPTAVCNFSM6AAAAABJQB35SCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZYG4YDIOBVG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@@ -73,6 +73,10 @@ export default (props) => { | |||
minWidth: minWidth | |||
}; | |||
const placeholder = !readonly ? aPlaceholder : ""; | |||
|
|||
// For accessibility, always give the input field an aria-label | |||
const ariaLabel = config.settings.fieldPlaceholder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please change this to const ariaLabel = placeholder || config.settings.fieldPlaceholder
?
MaterialAutocomplete
can be used to select field in LHS (then aPlaceholder
is null and it's correct to use fieldPlaceholder
) but also can be used in RHS (see "Autocomplete" field in the example app)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And same for MuiAutocomplete
please
Note: Mui*
are for v5, Material*
are for obsolete v4.
To prevent an accessibility violation, mui and material-ui IconButtons should be given an aria-label for use with screen-readers. This uses the button label (which is not otherwise displayed) as the aria-label for screen-readers.
The Autocomplete widgets also need aria-labels, in this case I've used the fieldPlaceHolder from config.