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

[Story] Ensure the <Box aria-label> is flagged as inaccessible markup #170

Closed
1 task
khiga8 opened this issue May 10, 2024 · 2 comments · Fixed by #176
Closed
1 task

[Story] Ensure the <Box aria-label> is flagged as inaccessible markup #170

khiga8 opened this issue May 10, 2024 · 2 comments · Fixed by #176
Assignees

Comments

@khiga8
Copy link
Contributor

khiga8 commented May 10, 2024

Context

I noticed there is a gap in flagging aria-label/aria-labelledby misuse on the Box component!

I noticed that the following markup will be flagged by the a11y-role-supports-aria-props rule:

<Box as="div" aria-label></Box>
<Box as="span" aria-labelledby></Box>

This is because we have the polymorphicPropName config set to as, so the linter knows how to map the component to an element.

However, the following markup will not be flagged:

<Box aria-label></Box>
<Box aria-labelledby></Box>

It looks like we can expand coverage and ensure this Box is interpreted as a div by mapping Box to div in the component mapping for github!

Acceptance criteria

  • Ensure that invalid ARIA markup on Box is flagged.
@khiga8 khiga8 self-assigned this May 10, 2024
@khiga8 khiga8 transferred this issue from primer/react May 10, 2024
@joshblack
Copy link
Member

Thanks for suggesting this, @khiga8! Just wanted to ask, where would be the place for us to set the default role for Box? 👀 Was having a hard time tracking it down today during triage.

@khiga8
Copy link
Contributor Author

khiga8 commented May 13, 2024

Hi @joshblack! components.js seems to be the appropriate file. I believe @TylerJDev will have more insights!

This mapping currently gets passed into apply to both the jsx-a11y and the github linting plugins.

I would suggest only passing the mapping of Box to div for the github plugin. We have not enabled polymorphic component linting support for jsx-a11y yet, so if we pass in a mapping of Box to div there, we'll run into risk of false positives as it will always treat Box as div!

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.

2 participants