-
-
Notifications
You must be signed in to change notification settings - Fork 74
feat(design): add roving tab index #4174
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: develop
Are you sure you want to change the base?
Conversation
|
@xelaint I need you to test out the behavior and see if you need any changes. For example, arrow keys on the outermost elements doesn't move focus, only inside a group, seems like we might always want arrow keys to move focus around. |
@griest024 Are you saying the arrow key should be able to move focus from |
yes |
I don't think so. Native behavior for moving around main interactive elements on a page without extra features is typically with tabs. e.g. moving from button to button |
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.
Tabbing from this group <div rti rtiBoundary="group"> should move focus to the next focusable element on the page, in this case it's the HTML tab, but tab cycles focus back to <div rti>I get tabbed to first</div>
libs/design/roving-tab-index/src/roving-tab-index-group.service.ts
Outdated
Show resolved
Hide resolved
|
So, this appears to have a few issues, once tabbed into a area with an RTI field, you're no longer able to tab to anything else labelled with RTI. Additionally, if there are not RTI labelled elements (think |
I thought this is what we wanted, tab moves focus outside of the group, arrow keys moves focus inside the group
So we should retain browser native behavior of tabbing then? |
3904471 to
975f921
Compare
|
975f921 to
a58c494
Compare
|
@damienwebdev I've made the requested changes, the implementation is fairly clean as well! |
|
focus trap inside rti boundary |
implemented |
|
lgtm! can be moved to core |
cc4d5d3 to
9bb5a79
Compare

PR Checklist
PR Type
Current behavior
Fixes: #1955
Part of: #
New behavior
Breaking change?
Additional context