-
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?
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! |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/ecd1f9736cc8795/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/ecd1f9736cc8795/output.txt Total script time: 0.99 mins Published |
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.
I'll leave the decision of if this is something we can/want to merge to @calixteman, but these comments are the things that stand out for me at least.
| height: auto; | ||
| } | ||
| } | ||
| /* Show tooltips for keyboard users */ |
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.
Please ensure that there is always a newline between sections, so above this comment.
| /* Show tooltips for keyboard users */ | ||
| &:focus::after, | ||
| &:hover::after { | ||
| content: attr(title); |
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.
| content: attr(title); | ||
| position: absolute; | ||
| top: 100%; | ||
| left: 50%; |
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.


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