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: don't set aria-activedescendant immediately on autocomplete textarea #1513

Merged
merged 1 commit into from Sep 22, 2019

Conversation

nolanlawson
Copy link
Owner

After some testing on NVDA on Windows on Chrome/Firefox/FirefoxDeveloperEdition and on VoiceOver on Safari macOS, I believe this is the best implementation.

This changes the code so that we don't set aria-activedescendant or aria-owns until autocomplete results are available. We remove and add the aria-activedescendant and aria-owns attributes as the autocomplete results appear and disappear. This should make is so that the aria-hidden element which is referenced by aria-activedescendant/aria-owns is not referenced until it's no longer aria-hidden.

@MarcoZehe I don't think I could reproduce the exact issue you describe in NVDA on Firefox Developer Edition, but I believe this fix is an improvement in VoiceOver on Safari at least. The best screenreader seems to be NVDA on Chrome, because the others have odd bugs:

  1. In VoiceOver on Safari, the first autocomplete result is not spoken. Autocomplete results aren't spoken until you actually press the down or up arrows to move through the list. (Without this PR, autocomplete results aren't spoken at all.)
  2. In Firefox and Firefox Developer Edition using NVDA, there is an odd issue where it tries to speak the text that I've typed in, but it comes out garbled. For instances, sometimes it speaks the numbers "1" or "2" or "3" several times. I see this both in the current implementation on dev.pinafore.social and in this PR.

I couldn't figure out how to solve either of these issues.

@nolanlawson
Copy link
Owner Author

nolanlawson commented Sep 22, 2019

This should fix #1512 but I'm not 100% sure.

@nolanlawson
Copy link
Owner Author

This will be live on dev.pinafore.social soon, please give it a test and let me know if it works better than before. :)

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

1 participant