-
Notifications
You must be signed in to change notification settings - Fork 480
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
Update the panebuttons to support tab navigation #45162
Conversation
function onKeyDownWrapper(event) { | ||
const {onClick} = props; | ||
if ( | ||
event.key === ' ' || |
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 is new to me - what key does this represent?
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.
Oof. To be completely honest, I'm not sure either 😬. I copied it straight from here: https://benfrain.com/converting-divs-into-accessible-pseudo-buttons/
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.
Oh huh. Looks like it's the newer version of "SpaceBar"
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
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.
Oh interesting! Today I learned!
apps/src/templates/PaneHeader.jsx
Outdated
return ( | ||
<div | ||
role="button" | ||
tabIndex={0} |
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.
Nit - I don't think this needs to be an object, I think it can just be a string - "0"
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.
LGTM! Yay for accesibility!
Just a quick update to make the PaneButton subcomponent of PaneHeader tab navigable.
Before
After
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: