-
Notifications
You must be signed in to change notification settings - Fork 106
feat(ui-tabs): TabPanel shouldn't always recieve focus #2312
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: master
Are you sure you want to change the base?
Conversation
|
313b5b4 to
dbdb065
Compare
dbdb065 to
8c4e5cf
Compare
8c4e5cf to
c85cc9f
Compare
balzss
left a comment
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 code looks good, I've slightly updated the documentation wording and some of the examples to adhere to our own recommendation about tabIndex
packages/ui-tabs/src/Tabs/README.md
Outdated
| </Tabs.Panel> | ||
| <Tabs.Panel renderTitle="Disabled Tab" isDisabled> | ||
| {lorem.paragraphs()} | ||
| <Tabs.Panel renderTitle="Disabled Tab" isDisabled tabIndex={0}> |
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.
What is the point of setting the tabIndex to 0 when the panel is disabled? It will never be 0.
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.
you are right, fixed it
packages/ui-tabs/src/Tabs/README.md
Outdated
| </Tabs.Panel> | ||
| <Tabs.Panel id="tabB" renderTitle="Disabled Tab" isDisabled> | ||
| {lorem.paragraphs()} | ||
| <Tabs.Panel id="tabB" renderTitle="Disabled Tab" isDisabled tabIndex={0}> |
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.
What is the point of setting the tabIndex to 0 when the panel is disabled? It will never be 0.
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.
you are right, fixed it
packages/ui-tabs/src/Tabs/README.md
Outdated
| </Tabs.Panel> | ||
| <Tabs.Panel id="tabB" renderTitle="Disabled Tab" isDisabled> | ||
| {lorem.paragraphs()} | ||
| <Tabs.Panel id="tabB" renderTitle="Disabled Tab" isDisabled tabIndex={0}> |
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.
What is the point of setting the tabIndex to 0 when the panel is disabled? It will never be 0.
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.
you are right, fixed it
c85cc9f to
c2b8253
Compare
…s control (defaults to 0 for backward compatibility)
INSTUI-4878
Summary
The component was hardcoding
tabindex="0"on all panels.According to WCAG guidelines, this should only be set when panels don't contain focusable elements, otherwise it creates unnecessary tab stops.
Adds optional
tabIndexprop toTabs.Panelto allow proper WCAG-compliant focus management.Developers can now set
tabIndex={-1}for panels with focusable content ortabIndex={null}to omit the attribute entirely.Changes
tabIndexprop toTabs.Panelto allow proper WCAG-compliant focus managementtabIndex?: number | nullprop (defaults to0for backwards compatibility)Test plan
On the documentation page, verify the correct focus handling of Tabs.Panel.
Co-Authored-By: 🤖 Claude