-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Fixing console a11y failures #57520
Fixing console a11y failures #57520
Conversation
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
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.
@myasonik thanks for fixing this! I had a question about a dependency that was added, but otherwise changes LGTM.
ac96099
to
32e2bba
Compare
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.
Removed a class 👍
I took a look at this as well.Looks pretty good. I noticed that the test failures are for unrelated tests. Perhaps merging upstream will fix that. |
There's also a i18n check that is looking for some labels. |
retest |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Summary
Though the workaround present before satisfied axe, it both didn't fix the original a11y problem and in some ways made the issue worse by setting the textarea label to be the entirety of the inner contents of the div (namely, the "press enter to edit and esc to exit" string).
This fixes the underlying a11y issue which unblocks axe and fixes the regression.
Supersedes: #52968
Original issue: #52136
Side note: my editor rearranged the order of imports. Hope that's all right but can revert if people don't like it.
Checklist
For maintainers