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

USWDS-Site - Accessibility Failures: Fixed Accessibility Failures #2783

Merged
merged 6 commits into from
Sep 20, 2024

Conversation

RachelCorsino
Copy link
Contributor

@RachelCorsino RachelCorsino commented Aug 16, 2024

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

  1. For color contrast failures, ensure updated color meets WCAG requirements.
  2. *See comments for remaining failures

Before opening this PR, make sure you’ve done whichever of these applies to you:

  • Confirm that this code follows the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run git pull origin [base branch] to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch is main).
  • Run npm run prettier:scss to format any Sass updates.
  • Run npm test and confirm that all tests pass.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.

@RachelCorsino
Copy link
Contributor Author

Most of the accessibility failures still need fixes but wanted to get some opinions on next steps for them.

1. Deprecated ARIA roles must not be used

  1. There are 6 instances where doc-endnotes is being used which is now deprecated. It's recommended to update this to use listitem as the role value.
    How can we update this markup?

2. Elements must only use supported ARIA attributes

  1. There are 2 instances where aria-multiselectable is being used which is now deprecated. It's recommended to remove this entirely, or add a <details> tag which would be able to take in the aria-multiselectable value.
    However, this code is component code, so should we create a new ticket for this to be updated within the necessary component?

3. Elements must meet minimum color contrast ratio thresholds

  1. The text being flagged for these failures are technically meeting WCAG requirements as they are considered a bold font and are at least 18.5px.
    Do we still want to update this and make this text larger?
  2. A couple of these were updated, however the remaining colors that are failing are custom sass variables that are not meeting color contrast minimums. Should we update to use USWDS color tokens or should we darken these variables?
$pink-20v: #f8a897;
$pink-50v: #bc5843;

$gray-20-next: #eaece9;

3. All touch targets must be 24px large, or leave sufficient space

  1. For these failures, they are occurring on any of the family (in this example) instances that are also serving as links
    Screenshot 2024-08-16 at 10 39 30 AM
    Do we want to add an underline text-decoration to these since they are links? Or do we want to increase the margin around them to add sufficient space?
  2. Other failures occurring under this topic have to do with insufficient space on links within the component preview.
    Screenshot 2024-08-16 at 10 46 08 AM
    Would we want to create a separate ticket for these to have the component code updated?
  3. For the date picker button (calendar icon), there is not sufficient space between neighbors.
    Screenshot 2024-08-16 at 10 57 00 AM
    Do we want to increase the size of the icon or account for a larger margin?
    This is also component code. Do we want a separate ticket for this?

5. Documents must have <title> element to aid in navigation.
How can we add to the markup to add a <title> element?

@RachelCorsino RachelCorsino marked this pull request as ready for review August 16, 2024 16:03
Copy link
Contributor

@mejiaj mejiaj left a 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!

  1. Deprecated ARIA roles must not be used. We're using kramdown. We should look into:
    1. Checking if there any updates or alternatives to this dependency.
    2. Checking if there any updates or alternatives to this dependency.
    3. If there aren't any viable updates/alternatives we should separate this issue to resolve elsewhere.
  2. 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.
  3. Elements must meet minimum color contrast ratio thresholds
    1. Which text? If the LOE is small we should try to fix.
    2. Darkening colors seems like a lower LOE.
  4. All touch targets must be 24px large, or leave sufficient space.
    1. Separate issue with potential solutions for family.
    2. Component preview touch target issue might need to be separated if higher than small LOE.
  5. Create issue in USWDS for date picker touch target (if there isn't one already).
  6. Each MD page takes a title in Frontmatter. We should create a bug report if it's not working.

css/uswds-next.scss Show resolved Hide resolved
@RachelCorsino
Copy link
Contributor Author

Thanks for looking into these issues @RachelCorsino!

  1. Deprecated ARIA roles must not be used. We're using kramdown. We should look into:

    1. Checking if there any updates or alternatives to this dependency.
    2. Checking if there any updates or alternatives to this dependency.
    3. If there aren't any viable updates/alternatives we should separate this issue to resolve elsewhere.
  2. 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.

  3. Elements must meet minimum color contrast ratio thresholds

    1. Which text? If the LOE is small we should try to fix.
    2. Darkening colors seems like a lower LOE.
  4. All touch targets must be 24px large, or leave sufficient space.

    1. Separate issue with potential solutions for family.
    2. Component preview touch target issue might need to be separated if higher than small LOE.
  5. Create issue in USWDS for date picker touch target (if there isn't one already).

  6. Each MD page takes a title in Frontmatter. We should create a bug report if it's not working.

  1. Elements must only use supported ARIA attributes - it looks like these failures are actually happening within the fieldset component. So this may be a component update rather than site update? It's happening for on the Patterns: Create a user profile - Race and ethnicity page and the Patterns: Select a language - Preferred language page

  2. Elements must meet minimum color contrast ration thresholds - this is being flagged on the .site-title class. Each page that it's being flagged on is using a different color behind the USWDS title, so if we want to take the darkening the color route, we'd be changing multiple colors. I can make that adjustment if that sounds like a good solution! This is what they currently look like:
    Screenshot 2024-08-16 at 12 29 37 PM
    Screenshot 2024-08-16 at 12 30 06 PM
    Screenshot 2024-08-16 at 12 30 16 PM

  3. All touch targets must be 24px large, or leave sufficient space - I think I need a little bit of clarification on the plan for this one.
    Should I create an issue in the USWDS repo for the component preview failures (should this be one issue for each separate component or one ticket containing both components - I believe this is only happening in two components)
    For the family related error, this is happening with more than just the family links, but it looks like they all have the token class which may be the only update we need to make for this. Should I go ahead and create an issue for updating the token class?

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 title issue.

@mahoneycm
Copy link
Contributor

aria-multiselectable

It looks like we're only adding aria-multiselectable to checkbox fieldsets. Since checkboxes support multi-selection by default, we should be able to remove them from the pattern previews.

MDN Docs mentions using checkboxes in place of multiselectable in this example

@mejiaj
Copy link
Contributor

mejiaj commented Aug 19, 2024

@RachelCorsino re: questions

  1. It's a component issue if there aren't any Site styles affecting this page. For example, custom site styles in preview.
  2. Agreed, let's update those colors.
  3. Again, let's create a USWDS issue or find one for this touch target error.
    1. Yes, we'd be updating the clickable token class to improve touch target size in a separate issue.

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 title issue.

Yes, let's briefly look into Kramdown or propose ideas for workarounds if there aren't updates available.

@RachelCorsino
Copy link
Contributor Author

Adding these comments here based on the changes I made:

Deprecated ARIA roles must not be used

  • Didn't see any updates for Kramdown
  • Updates have been made to md files to remove the {:footnotes } code that was adding the deprecated ARIA roles to elements. It has been changed to just using an ordered list which is working the same way in terms of styling as the previous code was, just without the added deprecated roles.

Elements must meet minimum color contrast ratio thresholds

  • Background colors against the U.S. Web Design System (USWDS) site title have been updated to meet color contrast minimums using USWDS color tokens rather than sass variables.
  • Text on /next page has been updated to meet color contrast minimums using USWDS color tokens.

All touch targets must be 24px large, or leave sufficient space

Touch Token Family

Elements must only use supported ARIA attributes

  • Removed aria-multiselectable from pattern previews

Documents must have <title> element to aid in navigation

  • I believe this is a false failure on the accessibility scan results. After searching the html using dev tools on the pages flagging this failure, it has been confirmed that they do in fact have a <title> element on the page.

Let me know if we should take a different route with any of these changes!

@RachelCorsino
Copy link
Contributor Author

@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 space

Date Picker Button

  • On the ./components/date-picker page, it is flagging the date picker calendar icon button as having "insufficient space between targets". After reviewing this, our calendar button touch target is larger than minimum requirement of 24px by 24px.
  • On the ./footer page, the following elements are being flagged for having insufficient space between neighbors as well:
<a href="tel:1-800-555-5555">&lt;(800) 555-GOVT&gt;</a>
<a href="mailto:[email protected]">&lt;[email protected]&gt;</a>

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!

@amycole501
Copy link

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

@alex-hull
Copy link

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).

@annepetersen
Copy link
Contributor

annepetersen commented Aug 28, 2024

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

@amycole501
Copy link

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?

@RachelCorsino
Copy link
Contributor Author

@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!

@mahoneycm
Copy link
Contributor

@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.

image

Copy link
Contributor

@mahoneycm mahoneycm left a 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.

css/uswds-next.scss Outdated Show resolved Hide resolved
css/uswds-next.scss Show resolved Hide resolved
@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming color meets color contrast minimums

Before After
image image

@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming color meets color contrast minimums

Before After
image image

@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming color meets color contrast minimums

Before After
image image

_next/04-00-looking-ahead.md Outdated Show resolved Hide resolved
Copy link
Contributor

@amyleadem amyleadem left a 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:

  1. Error: "Element has insufficient color contrast" (serious)
    1. I confirmed that the color contrast issues have all been addressed.
  2. Error: "The role used is deprecated: doc-endnote" (minor)
    1. 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?
  3. Error: "ARIA attribute is not allowed: aria-multiselectable="true"" (Critical)
    1. I confirmed that this has been removed in all instances
  4. Error: "Document does not have a non-empty <title> element" (Serious)
    1. 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.
  5. Error: "All touch targets must be 24px large, or leave sufficient space." (Serious)
    1. We have opened issue USWDS-Site - Bug: Token Class Touch Target Accessibility Failure #2796 to address as a follow on.

@RachelCorsino
Copy link
Contributor Author

@amycole501 - Related footnotes code has been reverted

Copy link
Contributor

@amyleadem amyleadem left a 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.

@annepetersen annepetersen merged commit 5b01c68 into main Sep 20, 2024
11 checks passed
@annepetersen annepetersen deleted the rc-publish-header-checklist branch September 20, 2024 19:45
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.

USWDS-Site: Resolve accessibility failures
7 participants