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

Move from using aria-expanded to data-expanded #209

Closed
wants to merge 0 commits into from

Conversation

yatil
Copy link
Contributor

@yatil yatil commented Aug 15, 2020

As the tool is changing the text of the buttons, aria-expanded is misleading. It basically announces:

  • Show info -> Collapsed
  • Hide Info -> Expanded

It also creates more verbose output.

It is best practice to either use aria-expanded or the accessible name of the element to communicate the status of a button. I was not aware of this when implementing the current version back in 2015 or so. 😃

This PR changes all aria-expanded attributes to data-expanded attributes which should preserve the functionality but removes the ambiguity. It needs to be tested as I probably have overlooked some aspect.

Thanks to Dennis Lembrée aka @weboverhauls for bringing it to my attention.

@mfairchild365
Copy link

This is great! I agree that it is a best practice to not change the name AND the state at the same time.

That being said, please note that inner-text name changes for buttons are not conveyed by several popular screen reader/browser combinations. See https://a11ysupport.io/tests/tech__html__button-name-change for details. The same is true for aria-label and aria-labelledby.

Using aria-expanded to convey the state is the only way that I'm aware of to consistently convey the state change.

You could use a hack, such as applying aria-live or sending focus to an empty element and then back after a timeout, but those might come with their own overhead.

@yatil
Copy link
Contributor Author

yatil commented Aug 18, 2020

@mfairchild365 I think here for visual users it is better to change the text which rules out aria-expanded. I think using a live region would be possible. This is basically just the first step. What to do next is a good question :-D

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