-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
add help text (and links) to a11y audits #1589
Conversation
Seems OK to me. I'm aware from the computer for a bit but I'll check it in my smoke test when I get back. Thanks for doing this! |
Just started reviewing :) |
@@ -33,6 +33,9 @@ class ImageAlt extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'image-alt', | |||
description: 'Every image element has an alt attribute', | |||
helpText: 'Screen readers can\'t read images to their users. The `alt` ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest changing to something like "Screen reader users rely on alt text to provide descriptions for images. It's also used as fallback content if the image fails to load"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -34,6 +34,9 @@ class ColorContrast extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'color-contrast', | |||
description: 'Background and foreground colors have a sufficient contrast ratio', | |||
helpText: 'Some users with low vision have a difficult time sensing contrast. ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest rewording to "Low contrast text is difficult or impossible for many users to read."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -33,6 +33,8 @@ class ARIAAllowedAttr extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'aria-allowed-attr', | |||
description: 'Element aria-* attributes are allowed for this role', | |||
helpText: 'Invalid role-attribute combinations can disable the accessibility ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest rewording to something like "Each ARIA role
defines a specific subset of aria-*
attributes that it supports. Mismatching these will invalidate the aria-* attributes."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -33,6 +33,8 @@ class ARIARequiredAttr extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'aria-required-attr', | |||
description: 'Elements with ARIA roles have the required aria-* attributes', | |||
helpText: 'Some ARIA roles have required attributes that describe the state ' + | |||
'of the role to screen readers. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/required-aria-attributes).', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might change this to "state of the element".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -33,6 +33,8 @@ class ARIAValidAttr extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'aria-valid-attr-value', | |||
description: 'Element aria-* attributes have valid values', | |||
helpText: 'Screen readers may not be able to read elements with invalid ARIA ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest rewording to something like "Assistive technology, such as a screen reader, will not be able to interpret ARIA attributes with invalid values"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -33,6 +33,8 @@ class ARIAValidAttr extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'aria-valid-attr', | |||
description: 'Element aria-* attributes are valid and not misspelled or non-existent.', | |||
helpText: 'Screen readers may not be able to read elements with invalid ARIA ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest rewording to something like "Assistive technology, such as a screen reader, will not be able to interpret ARIA attributes with invalid attribute names"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -33,6 +33,9 @@ class Label extends AxeAudit { | |||
category: 'Accessibility', | |||
name: 'label', | |||
description: 'Every form element has a label', | |||
helpText: 'Although the purpose of each form element may be obvious ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest rewording to "Labels ensure that form controls are announced properly by assistive technology, such as a screen reader"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Hallo mr @robdodson "updates were made" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
WDYT @robdodson PTAL LGTY?