8000 USWDS - Step indicator: Remove aria-label from wrapper by amyleadem · Pull Request #6146 · uswds/uswds · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

USWDS - Step indicator: Remove aria-label from wrapper #6146

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

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

amyleadem
Copy link
Contributor
@amyleadem amyleadem commented Oct 23, 2024

Summary

Removed the aria-label from the wrapper of the step indicator component. This resolves an automated testing error related to having an invalid attribute on a div element.

Markup change

To remove automated testing errors, users must update the markup to remove the aria-label on the usa-step-indicator element:

- <div class="usa-step-indicator" aria-label="progress">
+ <div class="usa-step-indicator">

Breaking change

This is not a breaking change.

Related issue

Closes #6113

Related pull requests

Changelog PR

Preview link

Step indicator component

Problem statement

The step indicator component fails SiteImprove accessibility tests because it has an invalid aria-label attribute on a <div> element.

Solution

This PR removes the aria-label from the .usa-step-indicator div element. This was done for the following reasons:

  • The aria-label attribute is not valid on a <div> element. In some cases this means the label is not read out, and in others, it causes accessibility scan failures.
  • This aria-label was determined to be an unnecessary addition. The accessibility team reported that user interviews suggested the label did not provide necessary context.
    • Note: JAWS does not currently read out the aria-label, but VoiceOver does

Testing and review

  1. If available, confirm that this issue is now resolved in SiteImprove scans
  2. Confirm that the aria-label has been successfully removed
  3. Confirm that removing the aria-label is appropriate
  4. Confirm no regressions

@amyleadem
Copy link
Contributor Author

@amycole501 and/or @alex-hull, can you confirm that this fix resolves the SiteImprove error described in the related issue?

Copy link
@amycole501 amycole501 left a comment

Choose a 8000 reason for hiding this comment

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

Removing the label is the cleanest and most straightforward fix. Thank you!

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.

Looks good to me! Left one comment about a small change in screen reader behavior.

Testing checklist

  • If available, confirm that this issue is now resolved in SiteImprove scans
  • Confirm that the aria-label has been successfully removed
  • Confirm that removing the aria-label is appropriate
  • Confirm no regressions

@@ -1,4 +1,4 @@
<div class="usa-step-indicator {{ modifier }}" aria-label="progress">
<div class="usa-step-indicator {{ modifier }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

Note (non-blocking): I noticed that my screen reader no longer focuses the entire time picker "group".

Before:

  1. Screen reader focuses entire component group
  2. Scr 8000 een reader then focuses step indicator bar and reads through steps
  3. Screen reader then reads out header element

After:

  1. Screen reader focuses step indicator bar and reads through steps
  2. Screen reader then focuses header element

This doesn't seem to be an issue but wanted to flag since the behavior changes slightly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging!

@amycole501 and @alex-hull, would this change in behavior count as an accessibility regression? Wanting to check in with you both before moving forward with this.

Choose a reason for hiding this comment

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

I agree with Charlie. I don't think this will be an issue. It appears that the flow is not that much different between the before and after interaction, so that should not be too big of a change. I also think as long as the user hears what current step they are on, this should not be an issue. @amyleadem

Copy link
@alex-hull alex-hull left a comment

Choose a reason for hiding this comment

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

Address changes to focus on whole component. Approve on my end.

@thisisdano thisisdano merged commit f5189fa into develop Nov 6, 2024
5 checks passed
@thisisdano thisisdano deleted the al-step-indicator-label branch November 6, 2024 19:16
@thisisdano thisisdano mentioned this pull request Nov 12, 2024
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 - Bug: Accessibility Aria Failure on Step Indicator
5 participants
0