a11y: show toolbar tooltips on keyboard focus (Fixes #19156)#20558
a11y: show toolbar tooltips on keyboard focus (Fixes #19156)#20558enterayush wants to merge 1 commit intomozilla:masterfrom
Conversation
timvandermeij
left a comment
timvandermeij
left a comment
There was a problem hiding this comment.
@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
timvandermeij
left a comment
There was a problem hiding this comment.
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.
enterayush
force-pushed
the
fix-focus-tooltips-a11y
branch
from
3af2d97 to
4cc83e0
Compare
enterayush
force-pushed
the
fix-focus-tooltips-a11y
branch
from
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
timvandermeij
left a comment
There was a problem hiding this comment.
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.
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.
For non-keyboard use, so on hover, this now duplicates the tooltips that the browser already shows up default, and they are on top of each other which doesn't seem like good UX:
| content: attr(title); | ||
| position: absolute; | ||
| top: 100%; | ||
| left: 50%; |
There was a problem hiding this comment.
This doesn't take in account strings that are longer or are at the edge of the viewer, such as the secondary toolbar button in the Dutch locale, which also creates a horizontal scrollbar:
4 participants
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