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(role-img-alt): Split rule for role=img with no accessible name #1586

Merged
merged 1 commit into from May 24, 2019

Conversation

AutoSponge
Copy link
Contributor

@AutoSponge AutoSponge commented May 23, 2019

This also removes [role='img'] selector from image-alt rule.

issue: #1192

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: @WilcoFiers

@AutoSponge AutoSponge force-pushed the fix-1192-role-img-alt branch 3 times, most recently from d984b70 to 802260e Compare May 23, 2019 16:48
@AutoSponge AutoSponge marked this pull request as ready for review May 23, 2019 17:30
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.

Couple of minor points.

],
"metadata": {
"description": "Ensures [role='img'] elements have alternate text",
"help": "Images must have alternate text"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this to [role='img'] elements have an alternative text, to distinguish it from other "image" rules.

@@ -0,0 +1,13 @@
<div id="match">Bananas</div>
<div id="hidden-match" style="display:none">Banana bombs</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to put role="img" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's purely for testing aria-labelledby, it's a pattern used in other alt tests.

@AutoSponge AutoSponge dismissed WilcoFiers’s stale review May 24, 2019 12:37

resolved rule message

@WilcoFiers WilcoFiers changed the title feat(role-img-alt): adds new rule for role=img with no accessible name feat(role-img-alt): Split rule for role=img with no accessible name May 24, 2019
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.

None yet

2 participants