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

Header: Fix issues with search toggle in header #2105

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Conversation

davidrapson
Copy link
Contributor

Prompted by #2099 this PR attempts to fix a few accesibility and markup quality issues with the header search toggle:

  1. Was using title rather than aria-label for the description
  2. Was missing a type attribute on the button element
  3. Was missing aria-controls to associate the button with the search
  4. Both the toggle and the search form were using icon fonts which causes additional text to be read out when interacting with either element. Switch to use SVG icons. See Icon fonts elements are announced to screen readers #1902

@davidrapson
Copy link
Contributor Author

There's a small github actions issue at the moment so this will probably fail right now: https://www.githubstatus.com/incidents/6r4sms3hcj3x

@davidrapson davidrapson marked this pull request as ready for review June 17, 2022 13:50
Copy link
Contributor

@davidsauntson davidsauntson left a comment

Choose a reason for hiding this comment

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

I think you'll also need to update the example HTML in the styleguide folder

@davidrapson davidrapson force-pushed the header-a11y-fixes branch 3 times, most recently from 70096ee to bc530e9 Compare June 22, 2022 15:24
1. Was using title rather than aria-label for the description
2. Was missing a type attribute on the button element
3. Was missing aria-controls to associate the button with the search
4. Both the toggle and the search form were using icon fonts which
   causes additional text to be read out when interacting with either
   element. Switch to use SVG icons
@davidrapson
Copy link
Contributor Author

davidrapson commented Jun 22, 2022

Just added a changelog entry too

@davidrapson davidrapson merged commit d4bd846 into main Jun 22, 2022
@davidrapson davidrapson deleted the header-a11y-fixes branch June 22, 2022 15:36
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.

2 participants