-
Notifications
You must be signed in to change notification settings - Fork 345
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
Tabs Examples: Improve high contrast support and code quality #2194
Conversation
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.
@jongund This is looking really good! I did functional testing and editorial testing. Have not done accessibility testing, but I will. I think it's important not to support Up and Down arrow keys in a horizontal tablist. Users like to use Up/Down arrows to scroll the page, and it can be disorienting if those keys are taken by a horizontal tab control.
Edit: Yes, as you mentioned in the call, the pattern explicitly says:
If the tab list is horizontal, it does not listen for Down Arrow or Up Arrow so those keys can provide their normal browser scrolling functions even when focus is inside the tab list.
…ags to quote characters
@jongund, I've finished editorial checks and corrections. There is one change in the example code itself that I didn't make (heading level comment above). Please carefully examine my changes to accessibility features documentation in particular to ensure I didn't unintentionally change meaning and introduce an inaccuracy. I requested re-review from @ZoeBijl since @ZoeBijl previously requested changes. I am having a hard time following the thread to determine which comments are resolved. @ZoeBijl, if your feedback is resolved, please change your review status to approve. Also, @jnurthen , please confirm Jon's fix to the issue you reported during today's meeting is resolved. |
@mcking65 |
@mcking65 I’ll re-review with a fresh pair of eyes. While I might not agree with the visual changes; that’s more a personal thing than an APG thing so I’ll let that go. Should be done this week. |
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.
The changes are fine. I still don’t agree with a lot of it but I’ll leave that for another time.
@@ -0,0 +1,136 @@ | |||
/* |
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.
Can you elaborate?
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.
Editorial review complete. We are ready to ship!!
Thank you to all who helped, especially @jongund!
Also, this is a special pull request as it contains significant input from @carmacleod. Thank you Carolyn! We miss you! We honor the wonderful role you played in our lives and in the world of accessibility!
…2194) This commit updates the two examples of the tabs design pattern by: * Improving support for high contrast settings by changing how keyboard focus and the selected state are rendered. * Improving clarity of guidance related to focusability of tab panels; one example illustrates focusable tab panels and the other illustrates omitting the panel from the page tab sequence. * Reducing complexity by removing the delete tab feature; this advanced functionality may later be illustrated in a separate example. * Improving documentation of accessibility features. * Updating Javascript and CSS to implement practices specified in the latest APG Code Guide. Co-authored-by: Carolyn MacLeod <[email protected]> Co-authored-by: Matt King <[email protected]>
Tab example updates include:
role=tabpanel
tab example with automatic activation
tab example with manual activation
Preview | Diff