-
Notifications
You must be signed in to change notification settings - Fork 150
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
USWDS-Site - Accessibility Failures: Fixed Accessibility Failures #2783
Conversation
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.
Thanks for looking into these issues @RachelCorsino!
- Deprecated ARIA roles must not be used. We're using kramdown. We should look into:
- Checking if there any updates or alternatives to this dependency.
- Checking if there any updates or alternatives to this dependency.
- If there aren't any viable updates/alternatives we should separate this issue to resolve elsewhere.
- Elements must only use supported ARIA attributes. Where are these attributes being used? If LOE isn't small, we should separate into its own issue.
- Elements must meet minimum color contrast ratio thresholds
- Which text? If the LOE is small we should try to fix.
- Darkening colors seems like a lower LOE.
- All touch targets must be 24px large, or leave sufficient space.
- Separate issue with potential solutions for
family
. - Component preview touch target issue might need to be separated if higher than small LOE.
- Separate issue with potential solutions for
- Create issue in USWDS for date picker touch target (if there isn't one already).
- Each MD page takes a
title
in Frontmatter. We should create a bug report if it's not working.
And just the confirm, the remaining tasks would be to look into kramdown, create issue for date picker touch target, and create bug report for |
aria-multiselectableIt looks like we're only adding MDN Docs mentions using checkboxes in place of multiselectable in this example |
@RachelCorsino re: questions
Yes, let's briefly look into Kramdown or propose ideas for workarounds if there aren't updates available. |
Adding these comments here based on the changes I made: Deprecated ARIA roles must not be used
Elements must meet minimum color contrast ratio thresholds
All touch targets must be 24px large, or leave sufficient spaceTouch Token Family
Elements must only use supported ARIA attributes
Documents must have <title> element to aid in navigation
Let me know if we should take a different route with any of these changes! |
@amycole501 @alex-hull - Can you help to confirm that the following accessibility failures are false positives? All touch targets must be 24px large, or leave sufficient spaceDate Picker Button
I also believe this to be a false positive as it does look like we have sufficient space to meet wcag minimums for these elements, but can you confirm. Thank you! |
I can confirm that the calendar button exceeds the minimum 24x24 requirement. I also checked both the date picker and footer pages in Axe, Wave, and Siteimprove (which flagged issues for not meeting enhanced, AAA sizes) and the button and links didn't get flagged for any AA issues. I'm curious if the testing requirement is instead set to the enhanced target size? We aren't required to meet AAA as part of Section 508 however we have it on our radar to incrementally reach that goal in the future. https://www.w3.org/WAI/WCAG22/Understanding/target-size-enhanced.html |
Can also confirm that these meet the requirements for minimum. There appears to be two false positives in Siteimprove as well. Within that, it is referring to the minimum criteria. I do not think we can change what the automated tests are testing for (unless I'm unaware of this...could be very true). |
I thought SiteImprove had options for flagging false positives or altering criteria slightly, but I could be wrong. Worth poking around in their documentation for, I think |
You can ignore false positives in Siteimprove, you can do it one issue at a time per page. @RachelCorsino which software is flagging the issue you're asking us to investigate? |
@amycole501 - The accessibility scan was done via cloud pages beta accessibility scans where it showed failures for those couple instances. Thank you, Amy and @alex-hull for confirming the false failures! |
For reference, it looks like the accessibility scan might use Axe. The documentation link points to Deque.com/Axe. I'm unable to reproduce certain failures using the Axe browser plugin. |
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.
Found a couple things we might want to address / discuss before moving forward.
polish: Can you add preview links to the affected pages? It was difficult to find some of the pages to test.
polish: Can you provide a formatted list of the failures we should be checking? There are notes in the comments but it can be confusing to find what's outdated and what is relevant.
@@ -7,7 +7,7 @@ image: /img/next/og-next-findings-find.png | |||
permalink: /next/findings/team/ | |||
slug: "team" | |||
|
|||
navbar_color: "next-mint-dark" | |||
navbar_color: "mint-50" |
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.
@@ -7,7 +7,7 @@ image: /img/next/og-next-findings-engage.png | |||
permalink: /next/findings/community/ | |||
slug: "engage" | |||
|
|||
navbar_color: "next-gold-dark" | |||
navbar_color: "gold-50v" |
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.
@@ -7,7 +7,7 @@ image: /img/next/og-next-findings-improve.png | |||
permalink: /next/findings/improve/ | |||
slug: "improve" | |||
|
|||
navbar_color: "next-pink-dark" | |||
navbar_color: "red-cool-50" |
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.
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.
Thanks for your work on this @RachelCorsino and team! This is close - I just vote that we revert all of the footnote markup changes (more details in this comment and in the "details" section below).
Details
I curiously am not able to find any of the beta scans on cloud pages anymore. I am wondering if they removed them. For review, I relied on a locally downloaded version of the scan instead.
In doing this, I discovered what appears to be false positives, so I think it is reasonable to treat the scan as a beta. My attitude is that any confirmed accessibility fix is a win here - we don't need to fix every flagged error.
Here is a summary of the different errors that were flagged and their status after this PR:
- Error: "Element has insufficient color contrast" (serious)
- I confirmed that the color contrast issues have all been addressed.
- Error: "The role used is deprecated: doc-endnote" (minor)
- I recommend we open a new issue to investigate and update the
doc-endnote
items. This role is built into Kramdown and is marked in the scan as a minor error. Feels reasonable to investigate in a follow-up issue. If we are comfortable with this, can you revert the related updates in this PR @RachelCorsino?
- I recommend we open a new issue to investigate and update the
- Error: "ARIA attribute is not allowed: aria-multiselectable="true"" (Critical)
- I confirmed that this has been removed in all instances
- Error: "Document does not have a non-empty <title> element" (Serious)
- This appears to be a false report. It is flagged on pages like the checkbox page and design principles (among others), but I see a
<title>
in the source code and axe devTools doesn't flag any errors.
- This appears to be a false report. It is flagged on pages like the checkbox page and design principles (among others), but I see a
- Error: "All touch targets must be 24px large, or leave sufficient space." (Serious)
- We have opened issue USWDS-Site - Bug: Token Class Touch Target Accessibility Failure #2796 to address as a follow on.
@amycole501 - Related footnotes code has been reverted |
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.
Thanks @RachelCorsino! Looks good to me.
Summary
Updated accessibility failures as found in the accessibility scan done on the USWDS site. These updates resolve accessibility violations.
Related issue
Closes #2688
Preview link
Preview link:
Problem statement
The website should provide an inclusive and accessible experience for all users, including those with disabilities, in compliance with Section 508 and WCAG guidelines. However, the current website contains accessibility failures that prevent users with disabilities from fully accessing and utilizing the website's features.
Solution
Fixes have been made to the USWDS site based on the accessibility scan. Now users with disabilities can fully access and utilize the website's features.
Testing and review
Before opening this PR, make sure you’ve done whichever of these applies to you:
git pull origin [base branch]
to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch ismain
).npm run prettier:scss
to format any Sass updates.npm test
and confirm that all tests pass.