uswds icon indicating copy to clipboard operation
uswds copied to clipboard

USWDS - Bug: Fix invalid aria role in validator list items

Open amyleadem opened this issue 3 years ago • 1 comments

Summary

Removed the aria-checked attribute from list items in the validation component. This attribute is no longer needed now that a <span> element informs the user if the requirement is complete or incomplete.

Related issue

Closes: #4905

Related PRs: This PR incorporates the changes originally merged in https://github.com/uswds/uswds/pull/4719 that were later reverted in https://github.com/uswds/uswds/pull/4906. This PR resolves the errors that caused the reversion while also delivering the original update.

Preview link

Preview link: Validation component

Problem statement

Adding the aria-checked attribute to a <li> item results in the following a11y error:

Errors in http://localhost:4000/components/validation/:

 • Elements must only use allowed ARIA attributes
   (https://dequeuniversity.com/rules/axe/3.5/aria-allowed-attr?application=axeAPI)

   (#validate-code > li:nth-child(1))

   <li class="usa-checklist__item" data-validator="uppercase"
   aria-checked="false"> Use at least one u...</li>

 • Elements must only use allowed ARIA attributes
   (https://dequeuniversity.com/rules/axe/3.5/aria-allowed-attr?application=axeAPI)

   (#validate-code > li:nth-child(2))

   <li class="usa-checklist__item" data-validator="numerical"
   aria-checked="false"> Use at least one n...</li>

These errors reveal that items with the aria-checked attribute also need an appropriate role. Reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-checked#associated_roles

However, adding role="checkbox" to the <li> item also returns an error:

Detected the following accessibility violations!
       
       1. aria-allowed-role (ARIA role should be appropriate for the element)
       
          For more info, visit https://dequeuniversity.com/rules/axe/4.3/aria-allowed-role?application=axeAPI.
       
          Check these nodes:
       
          - html: <li class="usa-checklist__item" data-validator="uppercase" role="checkbox" aria-checked="true">Use at least one uppercase character</li>
            summary: Fix any of the following:
                       ARIA role checkbox is not allowed for given element

Solution

The <span class="usa-sr-only" data-checklist-label="">Incomplete</span> element (created in this PR, but originally introduced in https://github.com/uswds/uswds/pull/4719) informs the keyboard user if the validation requirements are complete or incomplete. There is no longer a need to force the checkbox role and use an attribute like aria-checked to communicate complete or incomplete to the user.

Tasks completed:

  1. Integrated changes created in cm-validation-a11y branch 0684b17
  2. Removed the aria-checked attribute from the list item f077275
  3. Added tabindex="0" to make the items in the validation checklist focusable 29c2b61
  4. Moved aria-label, aria-live, and aria-atomic to the input element 07ad7de

Testing and review

  1. Confirm that validation status is discoverable via keyboard navigation using the screen readers you have available.
  2. Confirm that the validation component passes a11y tests.

Tested on:

  • VoiceOver on Firefox, Safari, Edge for MacOs
  • VoiceOver Gestures on iOS

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

  • [x] 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 develop).
  • [x] Run npm test and confirm that all tests pass.

amyleadem avatar Aug 08 '22 18:08 amyleadem

@mejiaj I wasn't sure how best to work with the previously merged PR, so I simply brought in the changes from that PR into this branch. Please let me know if there is a cleaner path.

amyleadem avatar Aug 11 '22 21:08 amyleadem

@mejiaj I made some updates to improve the readout in VoiceOver. Surprisingly, an <i> element generated a more reliable readout than the <span> (at least in VO). Will you check this on your screen readers and let me know your thoughts?

amyleadem avatar Sep 22 '22 00:09 amyleadem

@amyleadem it does read better in voiceover! The only thing I've noticed is that the list item doesn't read out the updated status when I tab to it. Instead, it will just read the requirements without the current status.

The only way I can get the status to be read is to focus out and back in to the input element.

mejiaj avatar Sep 22 '22 16:09 mejiaj

@mejiaj Switched it up a bit and generated an aria-label for the checklist items instead of generating a new text element. Will you let me know what you think?

amyleadem avatar Sep 22 '22 22:09 amyleadem

@mejiaj Thanks for the helpful review. I wanted to show you one more possible approach that has gotten the cleanest readout so far and would not interfere with with the aria-label on the input. This option generates a span that is a peer to the input and checklist elements, has the aria-live and aria-atomic attributes, and gathers the status updates for all checklist items into a single element for readout.

The current proposed solution also generates an aria-label for each checklist item that includes the item's current status, so users can still get check the status of each individual element.

The only concern I have is that creating a fully new text element might be too clumsy and cause some unexpected problems. It would be great to hear your thoughts.

Additionally, I created an enhanceValidation() function, but I am not sure I fully understand how best to do this. Will you take a look and let me know if this is doing what you hoped?

amyleadem avatar Sep 27 '22 16:09 amyleadem

@mejiaj Thanks a ton for the helpful review. I believe I have addressed all the comments above. Please let me know if you see anything else!

Tasks completed:

  • Moved enhanceValidation to init in https://github.com/uswds/uswds/commit/a57aff1446383a37b762637069c4b63f9691c52c
  • Added aria-describedby to input in a57aff1
  • Split out the functions in https://github.com/uswds/uswds/commit/853fb7db8eb64803231807e0db68c71e7a132c31
  • Added a delay for the screen reader readout in 9162036
  • Updated handleChange() to be more explicit in https://github.com/uswds/uswds/pull/4914/commits/aa38ecbcb2f829a4b6c03bc293407798d2a8433b

amyleadem avatar Oct 06 '22 18:10 amyleadem