search

ivanvorobei / SPStorkController

Now playing controller from Apple Music, Mail & Podcasts Apple's apps.
https://opensource.ivanvorobei.io
MIT License
2.73k stars 204 forks source link

Improve accessibility (fix #80) #81

Closed irskep closed 5 years ago

irskep commented 5 years ago

Fixes #80 by

  1. Adding accessibility labels to the chevron and close button
  2. Adding a larger tap target when VoiceOver is active
ivanvorobei commented 5 years ago

If I understand correctly, if VoiceOver is available, you are adding CloseButton, yes?

irskep commented 5 years ago

No, that isn't what's happening. The closeButton is still only added if you request it. I do have it add an accessibility label regardless of whether it's shown in case the user does something weird to add it manually.

irskep commented 5 years ago

It does add an extra invisible close button over the chevron to add a bigger tap target, maybe that's what you're referring to?

ivanvorobei commented 5 years ago

@irskep you use

NSLocalizedString("Close", comment: "")

but in project no file with localisation... it need?

irskep commented 5 years ago

Seeing as how I don't know any other languages, that sounds like future work. Surely it's better to have one translation than zero?

irskep commented 5 years ago

It won't crash, if that's what you mean.

ivanvorobei commented 5 years ago

@irskep class ns localization try get translation by key ‘Close’. Maybe set simple string?

irskep commented 5 years ago

I'm not sure what you mean. If you run this in practice, it says "close," and if you're not using English it will still fall back to the English word. No translation file is required for this code to run. It's just standard practice to wrap all user-facing strings in NSLocalizedString().

ivanvorobei commented 5 years ago

@irskep I am tested and found bug in this line:

accessibleIndicatorOverlayButton.bottomAnchor.constraint(equalTo: self.indicatorView.bottomAnchor),

Indicator optional, and sometimes not added to superview. Need set fix height. Please, fix it.

irskep commented 5 years ago

@ivanvorobei I'm confused how that's possible. The invisible button is not added if the indicator is not added. What were the steps to reproduce?

ivanvorobei commented 5 years ago

@irskep sorry, I did not read the changes carefully. You are right, this code will work correctly. Everything is fine, I will accept the request today or tomorrow.

Thanks for your work!