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

[Bug]: Duplicate ids between AccordionItem and Footer #12378

Closed
2 tasks done
Properko opened this issue Oct 24, 2022 · 4 comments · Fixed by #12442
Closed
2 tasks done

[Bug]: Duplicate ids between AccordionItem and Footer #12378

Properko opened this issue Oct 24, 2022 · 4 comments · Fixed by #12442
Assignees
Labels
severity: 2 https://ibm.biz/carbon-severity type: bug 🐛

Comments

@Properko
Copy link

Properko commented Oct 24, 2022

Package

@carbon/react, carbon-components-react

Browser

Chrome, Firefox

Package version

@carbon/react@1.39.0, carbon-components-react@8.15.0

React version

17.0.2

Description

Using AccordionItem from carbon-components-react with a Footer element from @carbon/react on the same page leads to duplicate ids such as accordion-item-1.

Looks like useId somehow does not have the same context between the libs? Can you expose the id prefix string as an optional prop?
Or is there a fix we can do on our side regarding the usage of AccordionItem?

This also shows up as an accessibility violation.
image

Suggested Severity

Severity 3 = User can complete task, and/or has a workaround within the user experience of a given component.

Reproduction/example

https://codesandbox.io/s/rough-grass-o4drs0?file=/src/App.js

Steps to reproduce

  1. Navigate to minimal example.
  2. Open DevTools and search for #accordion-item-1 in the elements tab.

Alternatively

  1. Navigate to live usage with async accordion items.
    1. Open DevTools and search for #accordion-item-11 in the elements tab.

Code of Conduct

@tay1orjones
Copy link
Member

Hey thanks so much for reporting this!

is there a fix we can do on our side regarding the usage of AccordionItem?

I don't think so unfortunately. I'll put up a PR to change the inbuilt prefix for v10 so there won't be conflicts between libraries.

@tay1orjones tay1orjones self-assigned this Oct 27, 2022
@tay1orjones
Copy link
Member

It's worth mentioning that I think this could also be fixed via #11513 but we haven't gotten React 18 support fully baked yet.

@tay1orjones
Copy link
Member

tay1orjones commented Oct 27, 2022

@Properko In thinking about this more - I'm curious, would you prefer a way to customize the prefix used for the ids? I think it might be the best way to keep both v10 and v11 backwards compatible while allowing you control so you can avoid duplicates.

I think we could wire up something similar to ClassPrefix that would enable you to control the id prefix used at any point in your react/app tree.

@Properko
Copy link
Author

It's worth mentioning that I think this could also be fixed via #11513 but we haven't gotten React 18 support fully baked yet.

I'm not sure we could make the move to React 18 anytime soon either. But it's nice to hear that it gets fixed eventually.

I think we could wire up something similar to ClassPrefix that would enable you to control the id prefix used at any point in your react/app tree.

Yep, that's a perfectly fine solution. Filling an extra property on AccordionItem is much better than creating a new custom component we'd have to maintain. But we do need to fix the accessibility violations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: 2 https://ibm.biz/carbon-severity type: bug 🐛
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants