Skip to content
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(Tab): sync tab id with tab panel id #8074

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Mar 11, 2021

Closes #8070
Related #7511
Related #7519

This PR removes auto ID generation to properly sync tab button and tab panel IDs. Autogeneration can be reimplemented if desired, but it may not be necessary

Testing / Reviewing

Confirm the "4.1.2 Name, Role, Value" violations for tab headers are no longer present and that #7511 remains resolved

@netlify
Copy link

netlify bot commented Mar 11, 2021

Deploy preview for carbon-elements ready!

Built with commit 363dfe1

https://deploy-preview-8074--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Mar 11, 2021

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 363dfe1

https://deploy-preview-8074--carbon-components-react.netlify.app

@tw15egan
Copy link
Collaborator

It seems like if an id is not provided, it will cause an a11y violation. Should we mark the prop to be required (or make a note to make it required in v11) or let the teams handle the violation on their own?

@emyarod
Copy link
Member Author

emyarod commented Mar 11, 2021

the id prop requirement hasn't been changed since the component was created but if we now want to mark the prop as required then I think that strengthens the case for autogenerated IDs

Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me checked against IBM Accessibility Checker 👍🏽

Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 ✅

@kodiakhq kodiakhq bot merged commit 2c42a32 into carbon-design-system:main Mar 22, 2021
@emyarod emyarod deleted the 8070-tabs-aria-controls-target branch March 23, 2021 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"4.1.2 Name, Role, Value" violations for tab headers in certain situations
3 participants