-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: accessibility updates from alpha #1511
Conversation
* feat: add aria attributes to loaders - Add aria-valuenow attribute to linear loader - Add aria-label attribute to both linear and circular loaders - Add unit tests for loaders * refactor: change aria-label to use hyphen after code review Co-authored-by: Diana Nanyanzi <[email protected]> --------- Co-authored-by: Mozafar Haider <[email protected]> Co-authored-by: Diana Nanyanzi <[email protected]>
* feat(tooltip): accessibility improvements for tooltip * fix(tooltip): solve failing test
* feat: splitButton accessibility improvements * feat: splitButton accessibility improvements * feat: splitButton accessibility improvements * refactor: apply code review comments (pairing session) --------- Co-authored-by: Mozafar Haider <[email protected]>
🚀 Deployed on https://pr-1511--dhis2-ui.netlify.app |
Passing run #3415 ↗︎
Details:
Review all test suite changes for PR #1511 ↗︎ |
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.
Couple of thoughts (might be out of scope of the tickets):
-
Tested the
split button
and i think we should be able to Tab it - just like the button component and activate it when i pressEnter
. Since there are 2 buttons, I can tab to any of the 2 to activate what i want. TheEsc
functionality after viewing the dropdown content works as expected. -
Tested the
Tooltip
. I can move focus to the one with a button trigger by tabbing to it, but the tooltip does not show as expected when the trigger receives focus. The 'Esc' functionality does work for the button trigger example.
@d-rita The issue with the Tooltip arises from the missing tabIndex. Since it is a |
@alaa-yahia Tooltip works as expected 🎉 |
This is a cherry-pick for features already in
alpha
branch. Fixes: