-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix(Dropdown): improve keyboard ux, WAI-ARIA #1293
fix(Dropdown): improve keyboard ux, WAI-ARIA #1293
Conversation
<DropdownItem disabled>Action</DropdownItem> | ||
<DropdownItem>Another Action</DropdownItem> | ||
<DropdownItem>Some Action</DropdownItem> | ||
<DropdownItem disabled>Action (disabled)</DropdownItem> |
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.
Why this change?
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.
"[Another] Action"
all start with 'a'
Wanted to better demonstrate the "jump to 1st matching char" feature
Tnx for the review @TheSharpieOne. I thought maybe this PR was dead in the water b/c it deviates from BS4 behavior. I'll address your feedback as soon as i have time |
I think a11y trumps bootstrap alignment (at least a little). Plus most people will not even notice the difference and if they do, I'm sure they will be happy that it is more accessible. |
85ee298
to
ad7727a
Compare
Update event handlers * wrap next/prev * handle ctrl+n/ctrl+p * handle home/end * manage focus on aria-expand/collapse * docs(Dropdown): reword action buttons * return Array<Element> (not NodeList) in getMenuItems return an array of els, not NodeList
3a03340
to
92cbd3b
Compare
@TheSharpieOne addressed change requests, rebased, ready for final check. If you disagree re |
Closes #1287
Adds additional event-handlers and focus management for the
Dropdown
component.