-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Block Styles: Remove unnecessary button role and 'onKeyDown' handler #40427
Conversation
Size Change: -41 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
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.
@Mamaduka While this PR is open, is it possible you could add aria-current="true"
to the currently selected style? Other than that, just one suggestion.
Thanks for the clean up here @Mamaduka Everything tests well with me, including @alexstine's suggestion of removing the tabIndex Keyboard navigation 👍 |
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Thanks for the review, @alexstine, @ramonjd . |
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
Thanks!
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.
Looks good.
What?
PR removes unnecessary button role and
onKeyDown
handler from the block styles item element. Both are leftovers from the time usingdiv
instead of the native button (changed in #34522).Why?
The
role="button"
is already set for the native button and they correctly trigger theonClick
event when using Enter or Space keys.Testing Instructions
Screenshots or screencast