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

Announce whitespace in screen reader announcements of visually hidden text #960

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

frankieroberto
Copy link
Contributor

@frankieroberto frankieroberto commented Jun 8, 2024

This fixes a bug in the nhsuk-u-visually-hidden helper class where any whitespace surrounding visually hidden text isn't announced by screen readers.

This fix was added to govuk-frontend in pull request #3868 and released in v4.7.0 and has been re-applied here.

Thanks to @hannalaakso for the work upstream. 🙌

I’ve also followed the approach of the GOV.UK Design System team of adding a Typography page with examples of using visually-hidden text, to make testing easier.

packages/core/tools/_mixins.scss Outdated Show resolved Hide resolved
@frankieroberto frankieroberto dismissed mcheung-nhs’s stale review July 9, 2024 22:29

Moved position:absolute back down

@anandamaryon1
Copy link
Contributor

Tested on VoiceOver and I can hear it working when we have a space inside a visually hidden span 👍

Would like to remove the 'Styles' example testing pages before merging though.

… text

This fixes a bug in the nhsuk-u-visually-hidden helper class where any whitespace surrounding visually hidden text isn't announced by screen readers.

This fix was added to `govuk-frontend` in [pull request #3868](alphagov/govuk-frontend#3836) and has been re-applied here.

Thanks to @hannalaakso for the work upstream.

Co-Authored-By: hannalaakso <[email protected]>
@frankieroberto frankieroberto force-pushed the include-space-in-visually-hidden-text branch from 3df9ef7 to 6b0d156 Compare October 15, 2024 13:31
@frankieroberto
Copy link
Contributor Author

@anandamaryon1 @edwardhorsford I’ve removed the test pages from the examples so this is now just the CSS changes (and I’ve squashed all the commits into 1).

Think this can now be merged?!

@anandamaryon1
Copy link
Contributor

anandamaryon1 commented Oct 16, 2024

Backstop to the rescue… caught something…

Visually hidden change has affected the responsive table head styling, showing extra white space elements:

image

@frankieroberto frankieroberto merged commit bc50201 into main Oct 17, 2024
2 checks passed
@frankieroberto frankieroberto deleted the include-space-in-visually-hidden-text branch October 17, 2024 08:11
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.

4 participants