-
Notifications
You must be signed in to change notification settings - Fork 229
docs(tabs): update a11y docs #5571
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: main
Are you sure you want to change the base?
Conversation
|
📚 Branch Preview🔍 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
dda064c
to
0b6b332
Compare
0b6b332
to
2d6c12e
Compare
06917f0
to
f0b7e78
Compare
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.
Thanks for taking this one on. Here are some changes I'd suggest.
b17e9a7
to
6515aa7
Compare
6515aa7
to
0f7a805
Compare
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.
One teeny-tiny change and we're good to go. The very first heading should be ## Overview
instead of ## Description
. DM me when you want me to re-review and I'll approve it.
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.
Lots of questions! I think this is probably good to go, but there's quite a bit of repetitive docs that I want your opinion on if we need.
## Vertical Tabs | ||
|
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.
Was there more to delete in this section? Or should we update this documentation to talk about the direction="vertical" property? Right now there's lots of duplicate content, but the
sp-tabs` demos are different/vertical.
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.
@cdransf just wanted to double check on this comment. Any thoughts on updating the docs here to mention the direction="vertical"
stuff?
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.
Oops, yep! Adding:
Vertical tabs should be used when horizontal space is more generous and when the list of sections is greater than can be presented to the user in a horizontal format. Vertical tabs are enabled by setting the
direction
attribute tovertical
onsp-tabs
.
5100ebb
to
6ad949d
Compare
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.
I left a screenshot and a slack message for you about the funky placement of quiet/quiet+compact.
## Vertical Tabs | ||
|
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.
@cdransf just wanted to double check on this comment. Any thoughts on updating the docs here to mention the direction="vertical"
stuff?
Description
Improving the accessibility documentation of components.
Related issue(s)
SWC-412
Author's checklist
Reviewer's checklist
patch
,minor
, ormajor
featuresDevice review