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(page-tabs): nav aria unique labels #1063

Conversation

andreancardona
Copy link
Contributor

@andreancardona andreancardona commented Nov 18, 2020

@andreancardona andreancardona requested review from a team, vpicone and jnm2377 and removed request for a team November 18, 2020 01:57
@vercel
Copy link

vercel bot commented Nov 18, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-design-system/gatsby-theme-carbon/44z1e8mp3
✅ Preview: https://gatsby-theme-carbon-git-1716-avt1-tabs-multiple-nav-roles.carbon-design-system.now.sh

@vpicone
Copy link
Contributor

vpicone commented Nov 18, 2020

I'm not sure the currentTab makes for a good label. It's not like an ID or key where we just need it to be unique. For instance here the navigation is named sidebar. If you change the tab (to tabs) then the label changes to just tabs.

I wonder if using parent would be more appropriate so the label stays the same for that nav element between tabs (probably Navigation)

@andreancardona
Copy link
Contributor Author

I'm not sure the currentTab makes for a good label. It's not like an ID or key where we just need it to be unique. For instance here the navigation is named sidebar. If you change the tab (to tabs) then the label changes to just tabs.

I wonder if using parent would be more appropriate so the label stays the same for that nav element between tabs (probably Navigation)

okay! so changing this to aria-label="navigation" am I understanding that corrrectly? @vpicone

@vpicone
Copy link
Contributor

vpicone commented Nov 18, 2020

@andreancardona sorry it's kinda confusing/meta with these pages haha.

As a different example, the label here would be color: https://www.carbondesignsystem.com/guidelines/color/overview/

(Not overview, usage and code)

@andreancardona
Copy link
Contributor Author

@andreancardona sorry it's kinda confusing/meta with these pages haha.

As a different example, the label here would be color: https://www.carbondesignsystem.com/guidelines/color/overview/

(Not overview, usage and code)

ahh makes a lot more sense just pushed. up a fix @vpicone

@vpicone vpicone merged commit a536b8a into carbon-design-system:main Nov 23, 2020
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.

3 participants