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

V8: Accessibility improvements for top header #5544

Merged

Conversation

MMasey
Copy link
Contributor

@MMasey MMasey commented May 29, 2019

This PR aims to fix issue 15, 16 and 17 from the A & AA accessibility issue list #5277

I have introduced a new css class called visually-hidden that I imagine will be useful in the long run to adding screen reader specific text to buttons.

I have added a btn-reset class that removed the default browser styling for buttons, which should make it easier to use the correct <button> markup when appropriate.

These two new additions have allowed me to update the icon buttons in the top right hand menu so that they use the correct element (as they are not linking to another page, the <a> tag wasn't appropriate). I've also added in some text that will provide screen readers with some context for the buttons functionality.

I have also added a focus state for the user icon. Note that on the user button, i have used an aria-label rather than the visually-hidden class, this is because the <umb-avatar> directive doesn't allow transclusion, so I couldn't add the additional span element inside it.

image

Cheers

@emmaburstow
Copy link
Contributor

Goooood morning @MMasey,

Thanks for the PR. This is a fantastic idea to aid the future accessibility improvements. I love the approach. We'll have a look over and let you know if we need a thing.

Em

@MMasey
Copy link
Contributor Author

MMasey commented May 30, 2019

Thanks @emmaburstow. Could this be tagged with the accessibility label please 🙂 Cheers

@emmaburstow
Copy link
Contributor

Done :)

@MMasey
Copy link
Contributor Author

MMasey commented Jun 20, 2019

Hey wonderful PR team, I know you're probably all super busy, but is there any update on if/when this will be merged in? The btn-reset and visually-hidden classes will be incredibly useful when it comes to improving the semantics of the backoffice, and in turn the accessibility. For example, there are loads of buttons across that backoffice that use an <a> element. When this is changed to a <button> element, the btn-reset class helps to retain the style and will make it easier for more people to go through and fix those issues.

Thanks 🙂

@nul800sebastiaan @poornimanayar @emmaburstow @abjerner @dawoe

@nul800sebastiaan
Copy link
Member

Thanks very much @MMasey - this indeed looks like super useful updates. The only thing to keep in mind for the future is localization. I will merge this since it's (much) better than what we had before but at some point those hard coded English texts should also be translatable 👍

Thanks again!

@MMasey
Copy link
Contributor Author

MMasey commented Jun 23, 2019

Thanks @nul800sebastiaan, after speaking with @RachBreeze about the visually-hidden class, we’ve decided on sr-only (screen reader only) as we can also do sr-only-focus following the same naming convention making it a little more reusable. You can catch up with the conversation at #5664. I’ll try update this PR accordingly today 🙂

@nul800sebastiaan
Copy link
Member

Cool, I missed the separate conversation there! I'll continue the conversation there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants