-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
improvement: improve a11y #4133
Conversation
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.
I reviewed and rebased this PR. Good improvements!
Everything looks fine, only he type error needs to be fixed:
Error: packages/frontend check:types: src/components/AccountListSidebar/AccountItem.tsx(199,7): error TS2322: Type 'RefObject<HTMLDivElement>' is not assignable to type
but the ref is passed through, so it refers to a Div Element and on a Button Element....
Keyboard-focusable
Not adding one for the "Settings" button because I'm lazy + it's already available in the "File" menu as an "accelerator".
Fixed the error, added a changelog entry and rebased. |
I plan on working on accessibility (a11y) more, but generally this can be merged at any point.
Recommended review commit by commit.