coreymckrill / nearby-wordpress-events

A widget to show nearby WP events in the Dashboard
GNU General Public License v2.0
11 stars 4 forks source link

a11y: the edit location button needs and aria-expanded attribute #36

Closed afercia closed 7 years ago

afercia commented 7 years ago

The button to edit the nearby location needs and aria-expanded attribute. Its value should be dynamically changed to true/false depending on the expanded/collapsed status of the box wrapping the edit location form. This is necessary to inform screen reader users about what the Edit location button does: it expands a panel immediately following the button.

iandunn commented 7 years ago

Thanks for pointing that out!

I think c1265c7 fixes this, but please let me know if you see any problems with it, or notice any other a11y issues.

afercia commented 7 years ago

Hm clicking on the button should toggle aria-expanded and the visibility, meaning when the form it's open and the button clicked again, aria-expanded should become false and the form should close. Instead, once it's open it's not possible to close it again clicking on the toggle. Only the button Cancel closes it. Also, moving focus to the input field is not ideal for a11y and the info provided by aria-expanded would be missed. I'd suggest to remove $( '#nearbywp-location' ).focus(); and adjust the callbacks on the click event to toggle the aria-expanded state and the visibility. Let me know if you want me to open a separate issue 🙂

coreymckrill commented 7 years ago

@afercia Thanks for your patience with this. I'm learning a lot :)

Since this is still related to the aria-expanded attribute, let's just re-open this issue.

coreymckrill commented 7 years ago

Alright, I think the above commit addresses the toggle concerns.

iandunn commented 7 years ago

@afercia, is there an accessible way to set the focus? It feels like that's helpful for many users, since they can just click on the icon, then start typing, rather than having to make an extra click to set the focus.

afercia commented 7 years ago

@iandunn yeah, this is a classical "conflict" between best practices for a11y and avoiding an extra click. By the way, I'm having a second thought. My only concern was that the change of state from collapsed to expanded won't be announced when moving focus to the input field. To better understand what happens, I've made a short video using Firefox and NVDA: https://cloudup.com/iUJ-A_RghPh

Notice that, when focus gets moved to the input field, NVDA doesn't announce the button change of state to expanded simply because there's not enough time :) it has to announce the input field. The change of state happens correctly though, and NVDA gets it, see in the screenshot below:

screenshot 80

Things happen so fast that NVDA just starts announcing City name: edit ... and skips expanded.

However, it's a very minor issue, I think moving focus benefits all users so I'm fine with that, as long as aria-expanded works correctly 🙂

iandunn commented 7 years ago

Sounds good, thanks!