-
Notifications
You must be signed in to change notification settings - Fork 10.6k
a11y: show toolbar tooltips on keyboard focus (Fixes #19156) #20558
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: master
Are you sure you want to change the base?
Conversation
timvandermeij
left a comment
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.
@calixteman I would imagine this also needs feedback from the UI/UX team; what you do think?
web/viewer.css
Outdated
| } | ||
| } | ||
| /* Show tooltips for keyboard users */ | ||
| .toolbarButton:focus::after, |
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.
This should be put in the correct structure/nesting as the other toolbar button rules.
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.
Thanks for the feedback — I’ve updated the tooltip rule to follow the existing toolbar button structure. Please let me know if any further adjustments are needed.
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.
It's just below .toolbarButton now, but it needs to be included inside the nested structure of .toolbarButton.
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.
Fixed — moved the tooltip focus/hover rules into the .toolbarButton nested structure. Thanks for pointing that out!
Yeah, we need to have a feedback from someone from the a11y team (I already asked here: #19156 (comment) but nothing). I pinged them on our internal chat. |
timvandermeij
left a comment
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.
Once the comment is addressed please make sure it passes linting and squash the commits into one; see https://github.com/mozilla/pdf.js/wiki/Squashing-Commits if you're not familiar with how that can be done.
3af2d97 to
4cc83e0
Compare
4cc83e0 to
0f418b4
Compare
|
Squashed the commits into one as requested. Thanks! |
Improves keyboard accessibility in the PDF.js viewer toolbar by making tooltips appear when a button receives keyboard focus, not only on mouse hover. This helps keyboard-only users understand the purpose of toolbar buttons while navigating with Tab.
Changes
Added CSS to show tooltips on :focus in addition to :hover.
Behavior for mouse users remains unchanged.
Testing
Verified tooltips appear on focus via keyboard.
Verified hover tooltips still work normally.
Fixes #19156