jdg / MBProgressHUD

MBProgressHUD + Customizations
http://www.bukovinski.com/
MIT License
16.01k stars 3.56k forks source link

UIAccessibility usability improvements for VoiceOver #535

Open mathewa6 opened 6 years ago

mathewa6 commented 6 years ago

Thanks for the library! I came across it on Twitter after it was listed in the top downloaded pods.

Since most of this project is based on UIKit elements, it is accessible. However, there are some usability issues that get in the way of a VoiceOver (VO) user being aware of what is happening on screen.

1) When the HUD is shown, there is no indication given to a VO user unless they swipe. 2) When they do swipe, the list of elements from the HUD spill onto elements from underneath the HUG. 3) Occasionally, the dismissal of the HUD refocuses VO to the top of the superview, which isn't ideal.

This PR attempts to address these issues in a non intrusive manner. All tests passing on my end. These changes do the following:

Happy to make changes as needed.

TLDR: Heres a 20s video(no audio): Just pay attention to the VO focus box.

matej commented 6 years ago

👏 Thank you for taking interest in this. I'm putting this on my TODO list for testing. Thanks!

ay8s commented 6 years ago

This is definitely an improvement. Awesome work @mathewa6! 👍

I do see the focus switch between the HUD and the previously select element midway through progress. https://cl.ly/32ba0000d2e0 Would also be great to read the label if the indicator has one.

mathewa6 commented 6 years ago

Thanks. I had to find my notes from when I worked on this to figure out why I hadn't read the label automatically. It seems like I couldn't simply set the accessibilityLabel on the UIActivityIndicatorView to use the self.label text for some reason. (Maybe I'm missing something?).

The other option would be to set the entire MBProgressHUD to be its own accessibilityElement whose label and traits are changed depending on the internal classes used, which I thought is best saved for a bigger PR.

matej commented 6 years ago

Took a closer look at this. While I believe that making the whole indicator a single accessibility element would be a nicer solution, I still think this is a step in the right direction and improves the behavior over the current one. One problem is that "Progress" isn't really localized. Taking the label value, when set, would make that better, as I would assume the host app would already make sure the label text is localized.

mathewa6 commented 6 years ago

Thanks @matej !

I've made 2 additional commits, the first of which directly addresses the feedback you've posted.

The second uses KVO on the label's text property and then uses that as accessibilityLabel on the indicator. I've hence removed label as an accessibilityLabel except for text only and custom modes to prevent redundant VoiceOver speech. Do note that this does result in an additional call to -updateIndicators: one at init and one if self.label.text is set. Not sure if this is the best way to go about it, but happy to make changes as it's the closest to just making the entire HUD a single element.