Skip to content

USWDS - Range: Remove redundant ARIA attributes #5413

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 30, 2023
Merged

USWDS - Range: Remove redundant ARIA attributes #5413

merged 2 commits into from
Oct 30, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Aug 1, 2023 •
edited by amyleadem
Loading

Summary

Removed redundant ARIA attributes to improve the screen reader experience. To incorporate these changes, update your range component markup.

Breaking change

⚠️ This is a breaking change.

Related issue

Closes #5038

Related pull requests

Changelog →

Preview link

Preview link:
Range slider →

Problem statement

Range slider keyboard navigation does not work with VoiceOver. This is caused by redundant aria attributes that are reserved for elements that are more or less "faking" being an input with type="range".

Example

Slider can only move one increment and does not update read out correctly:
VoiceOver - Video has audio if you unmute

Kapture.2023-08-01.at.14.25.41.mp4

Note: Some more advanced screen readers such as JAWS may intuitively navigate around this issue

Solution

Remove the following attributes:

  • aria-valuemin
  • aria-valuemax
  • aria-valuenow
  • role="slider"

Sources

MDN Docs: Range slider best practices →

Example

Slider has full range and updates read out correctly

Kapture.2023-08-01.at.14.29.38.mp4

Testing and review

  1. Test range slider on develop branch →
    1. Confirm keyboard navigation works without screen reader
    2. Activate the screen reader and attempt to use keyboard controls
  2. Open range slider fix preview →
  3. Confirm screen reader keyboard navigation is possible
  4. Confirmed read outs are improved
  5. Confirm no visual or usability regression

  • Confirm screen reader navigation is possible
  • Confirm screen reader readouts work as intended
  • Confirm no regression in component

We have discussed potential screen reader callout enhancements which are captured in #5431

@mahoneycm mahoneycm mentioned this pull request Aug 1, 2023
Merged
@mahoneycm mahoneycm marked this pull request as ready for review August 1, 2023 18:33
amycole501
amycole501 approved these changes Aug 4, 2023
View reviewed changes
Copy link

@amycole501 amycole501 left a comment

Choose a reason for hiding this comment

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

This sounds good in JAWS. I can hear "10, 20, 30" etc, and can use the arrow keys to move up and down the range slider. I recommend that whatever the range is indicating be written into the help text. For example "ten percent" instead of just "ten" if the slider is indicating a percentage range.

In NVDA the action is identical to JAWS. I used Chrome to test both.

@mahoneycm
Copy link
Contributor Author

It looks like we can accomplish the help text that @amycole501 mentions using aria-valuetext: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-valuetext

I'll do a bit more research on this and we can discuss if we want this to be included with this PR, or separated out into it's own issue. My primary concern is if we should maintain aria-valuenow to be used with aria-valuetext.

This was referenced Aug 8, 2023
Closed
Closed
@mahoneycm mahoneycm added Affects: Accessibility 🟡 Relates to the accessibility of our components Package: Range Slider Is: Breaking 🔴 Suggestion would be a breaking change Affects: Markup 🟡 Suggestion would change the markup of a component labels Aug 22, 2023
@mahoneycm mahoneycm added this to the uswds 3.7.0 milestone Aug 22, 2023
amyleadem
amyleadem reviewed Aug 23, 2023
View reviewed changes
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.

This works well for me in VoiceOver in Safari and VoiceOver Gestures in iOS Safari. I ran into some hiccups with the screen reader performance in Firefox and Chrome, but these issues don’t seem isolated to USWDS (see notes below for more details).

I tested the following:

  • Confirmed the issue exists when using VoiceOver in desktop browsers
  • Confirmed that the updated component is operable and provides appropriate feedback in VoiceOver. (Values from 0-100 are read out in increments of 10 as you navigate forwards and backwards through the range.)
  • In Safari, the component works well with VoiceOver.
  • In Firefox I am not able to move backwards in value (ie, moving from 40 to 20) with VoiceOver. However, this issue doesn’t seem isolated to USWDS because I noticed that the same behavior occurs in the “Cowbell” example on MDN in Firefox as well.
  • In Chrome, I ran into intermittent issues where it will read the same value twice and read 90 and 10 as the end points, rather than 0 and 100. This is not consistent though and I have been able to get a successful readout in Chrome. This also doesn’t seem isolated to USWDS because I noticed that the same behavior occurs in the “Cowbell” example on MDN as well.
  • Both issues in Firefox and Chrome are preferable to the current readout on develop.
  • Interestingly, the issue does not exist in iOS Safari with VoiceOver Gestures and reads out the percentage (”10%”, “20%”, etc). It works well in both develop and this PR branch.

Do you get the same results?

@amycole501
Copy link

amycole501 commented Aug 23, 2023 •
edited
Loading

The component sounds great in JAWS, NVDA, and Narrator on PC desktop in Chrome and Firefox.
The only functionality improvement may be to tell people not to press and hold the arrow keys as nothing is announced if you do that. It's silent. So you have to click then listen. That may be something we can add in ARIA or instructions?

The way the volume slider sounds in Narrator on my PC:
"60 out of 100 percent" after I'm done pressing the button.

@mahoneycm mahoneycm mentioned this pull request Aug 29, 2023
Merged
4 tasks
@mahoneycm
Copy link
Contributor Author

@amyleadem So I was able to replicate the Chrome bug after entering a bunch of inputs to try to trip up VoiceOver. During my testing, I found if you press the left and right arrow keys at the same time it would enable something called "Quick nav". It was only while quick nav was turned on that I would experience this bug!

By pressing both of the arrow keys again and disabled quick nav, the bug went away! I was also able to replicate the Firefox bug you mentioned only after enabling quick nav!

Could you try testing this again when you have the time? I'm curious if you experience the bugs after toggling quick nav.

amyleadem
amyleadem approved these changes Sep 19, 2023
View reviewed changes
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.

@mahoneycm good catch! My system did appear to have "quick nav" turned on. When I turned it off, I did not experience the strange behavior in Chrome and Firefox anymore. Nice discovery!

@mejiaj mejiaj requested a review from thisisdano October 26, 2023 14:01
@mejiaj
Copy link
Contributor

mejiaj commented Oct 26, 2023

Code looks good.

@mahoneycm since this and #5472 have changes to the component. Are there special considerations we need to be aware of when merging?

@mahoneycm
Copy link
Contributor Author

@mejiaj This PR should be merged before #5472. This work captures a bug fix and #5472 is a feature enhancement that builds off of this change.

Beyond that, I don't believe any other accommodations need to be made!

@amyleadem amyleadem mentioned this pull request Oct 27, 2023
Closed
@thisisdano thisisdano merged commit 4993d2c into develop Oct 30, 2023
@thisisdano thisisdano deleted the cm-range-slider-sr branch October 30, 2023 23:19
@amyleadem amyleadem mentioned this pull request Oct 31, 2023
Merged
@thisisdano thisisdano mentioned this pull request Nov 9, 2023
Merged
werdnanoslen added a commit to trussworks/react-uswds that referenced this pull request Dec 27, 2023
@werdnanoslen
Remove redundant ARIA attributes
cd0f994 
uswds/uswds#5413
@werdnanoslen werdnanoslen mentioned this pull request Dec 27, 2023
Merged
7 tasks
@sennhann29 sennhann29 mentioned this pull request Jul 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
4 more reviewers

@mejiaj mejiaj mejiaj approved these changes

@thisisdano thisisdano thisisdano approved these changes

@amyleadem amyleadem amyleadem approved these changes

@amycole501 amycole501 amycole501 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Affects: Accessibility 🟡 Relates to the accessibility of our components Affects: Markup 🟡 Suggestion would change the markup of a component Is: Breaking 🔴 Suggestion would be a breaking change Package: Range Slider
Projects
None yet
Milestone
uswds 3.7.0
Development

Successfully merging this pull request may close these issues.

USWDS - Feature: [Update the Range slider aria-valuenow attribute in real time when the slider is changed by a user]
5 participants
@mahoneycm @amycole501 @mejiaj @thisisdano @amyleadem