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

feat(aria-prohibited-attr): add support fallback role #4325

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

takenspc
Copy link
Contributor

@takenspc takenspc commented Feb 5, 2024

This PR adds fallback role support to aria-prohibited-attr. For example, <div role="foo img" aria-label="..."> is legal.

At this time, I do not plan to add fallback roles support for other checks.

Related:

Closes:

@takenspc takenspc requested a review from a team as a code owner February 5, 2024 07:32
@straker
Copy link
Contributor

straker commented Feb 7, 2024

Thanks for the pr. Unfortunately with axe-con quickly approaching we won't be able to look at this any time soon. After axe-con we'll be able to revisit this again.

@takenspc
Copy link
Contributor Author

takenspc commented Feb 8, 2024

Thanks for the pr. Unfortunately with axe-con quickly approaching we won't be able to look at this any time soon. After axe-con we'll be able to revisit this again.

Thank you for providing the information. I wish you a successful event, as in previous years. Please let me know if you need any changes are necessary.

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

This looks good. Just a couple small suggestions. @takenspc thank you for putting this together!

Would you mind adding one or two test cases to test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.html and its .json file as well?

test/checks/aria/aria-prohibited-attr.js Outdated Show resolved Hide resolved
test/checks/aria/aria-prohibited-attr.js Outdated Show resolved Hide resolved
test/checks/aria/aria-prohibited-attr.js Outdated Show resolved Hide resolved
takenspc and others added 2 commits March 5, 2024 19:55
@takenspc
Copy link
Contributor Author

takenspc commented Mar 5, 2024

@WilcoFiers

Thank you for your review.

Would you mind adding one or two test cases to test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.html and its .json file as well?

I have added a few tests to test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.html in b405042.

Although I need to update this branch to pass CI tests, it may be better to wait until #4354 is merged.

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

Successfully merging this pull request may close these issues.

3 participants