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

Fix accessibility issue in contents list component #3907

Merged
merged 4 commits into from Mar 11, 2024

Conversation

MartinJJones
Copy link
Contributor

@MartinJJones MartinJJones commented Mar 8, 2024

What

Stop dashes before each 'Contents' list item being read by screen readers

Why

Nearly every screen reader (except NVDA of the major ones) reads out the “em dash“ before each item/link in the “Contents“ list. Screen reader users found that frustrating to read through.

Trello card

Accessibility testing

Screen Reader Browser Issue fixed
NVDA - 2023.3.3 Firefox - 123 fixed ✅
Narrator - 21H2 Edge 122 fixed ✅
JAWS 2022 Chrome fixed ✅
JAWS 2023 Chrome fixed ✅
VoiceOver 10 Safari 17.3.1 fixed ✅
VoiceOver 10 Safari 17.3.1 fixed ✅
VoiceOver 10 Firefox 123 fixed ✅

VoiceOver example

VoiceOver Before

content-list-before.mov

VoiceOver After

content-list-after.mov

Further info

Using list-style-type

We considered using list-style-type, however this did present us with 2 issues.

  1. The dash style currently used it not available as a list style, we would have to choose something else such as “circle” or “disc” for example.
  2. When testing in VoiceOver the list item style is still read out as “You are currently on a AXListMarker”

Using speak

We also explored other options, such as using the CSS speak property to turn off voice output for screen readers. On top of this, browser support is very limited currently, so this is not an option - "speak" | Can I use... Support tables for HTML5, CSS3, etc

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-3907 March 8, 2024 15:22 Inactive
@MartinJJones MartinJJones marked this pull request as ready for review March 8, 2024 16:26
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-3907 March 8, 2024 16:26 Inactive
Copy link
Contributor

@davidtrussler davidtrussler left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Nice fix!
Tested locally with Voiceover (using the combination of keys you listed) and worked fine compared to current implementation.

@MartinJJones MartinJJones force-pushed the contents-list-accessibility-fix branch from 780cf41 to 2cdbb88 Compare March 11, 2024 09:53
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-3907 March 11, 2024 09:53 Inactive
@MartinJJones MartinJJones force-pushed the contents-list-accessibility-fix branch from 2cdbb88 to c1f2640 Compare March 11, 2024 09:54
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-3907 March 11, 2024 09:54 Inactive
@MartinJJones MartinJJones merged commit 2a421e8 into main Mar 11, 2024
12 checks passed
@MartinJJones MartinJJones deleted the contents-list-accessibility-fix branch March 11, 2024 09:58
This was referenced Mar 11, 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.

None yet

3 participants