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

Using "role" as prop name conflicts with ARIA attribute #448

Closed
athill opened this issue Aug 26, 2024 · 3 comments · Fixed by #449
Closed

Using "role" as prop name conflicts with ARIA attribute #448

athill opened this issue Aug 26, 2024 · 3 comments · Fixed by #449
Assignees
Milestone

Comments

@athill
Copy link
Contributor

athill commented Aug 26, 2024

The Badge and Button components both have a role prop that can take the value "secondary". When linting with the jsx/a11y plugin, this results in the following error:

 error  Elements with ARIA roles must use a valid, non-abstract ARIA role

We should change the name of this prop in these components to something that does not conflict. Maybe rivetrole?

@athill athill added this to the 2.0.0 milestone Aug 26, 2024
@burnumd
Copy link
Contributor

burnumd commented Aug 26, 2024

I think changing it to something like type may be the simplest change here. I worry that rivetrole may get confused with an ARIA role specific to Rivet.

burnumd added a commit that referenced this issue Aug 26, 2024
Role is a reserved ARIA attribute. We already refer to the `Button` property as `modifier` though the PropTypes don't reflect that.

Closes #448
@burnumd burnumd self-assigned this Aug 26, 2024
@burnumd burnumd linked a pull request Aug 26, 2024 that will close this issue
@burnumd
Copy link
Contributor

burnumd commented Aug 26, 2024

I opened a PR using modifier as the prop since that's what we actually use in Button (though its PropType calls it role).

burnumd added a commit that referenced this issue Aug 29, 2024
Role is a reserved ARIA attribute. We already refer to the `Button` property as `modifier` though the PropTypes don't reflect that.

Closes #448
@iu-deployer
Copy link

🎉 This issue has been resolved in version 2.0.0-beta.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants