Closed wlepinski closed 5 years ago
Looks like there's a bunch of accessibility issues that should be fixed with the combo popup, including #708 that ironically I cannot reproduce.
(1) The outer node should probably have role=dialog
.
(2) The <label for="id-of-input">
tag doesn't make sense if the <input>
is hidden. Should probably be replaced by a <span>
where both the root node and the <input>
have aria-labelledby="id-of-the-span"
.
(3) aria-expanded
is currently set on the <input>
but that's invalid.
(4) With the change in this PR I think you need an aria-haspopup="listbox"
on the <input>
. As stated in https://www.w3.org/TR/wai-aria-1.1/#aria-autocomplete:
If an element has aria-autocomplete set to list or both, authors must ensure both of the following conditions are met:
- The element has a value specified for aria-controls that refers to the element that contains the collection of suggested values.
- Either the element or a containing element with role combobox has a value for aria-haspopup that matches the role of the element that contains the collection of suggested values.
Finally, it's weird for me to not have a role=combobox
at all but I guess it's not necessarily an error. I couldn't find any examples on the web though where aria-autocomplete
is used without role=combobox
. If it turns out that we do need (or want) a role=combobox
after all, I think it's easy to implement with something like shown on https://www.w3.org/TR/wai-aria-1.1/#h-combobox, i.e:
<div role=dialog>
<div aria-label="Tag" role="combobox" aria-expanded="true" aria-owns="owned_listbox" aria-haspopup="listbox">
<input type="text" aria-autocomplete="list" aria-controls="owned_listbox" aria-activedescendant="selected_option">
</div>
<ul role="listbox" id="owned_listbox">
<li role="option">Zebra</li>
<li role="option" id="selected_option">Zoom</li>
</ul>
<button>OK</button>
<button>Cancel</button>
</div>
In other words a role=combobox
doesn't need to contain the buttons. It doesn't even need to contain the role=listbox
.
OK thanks, that's looking better. A few things:
1) The dialog title Hmm, I guess it does work after all, strangely; I see it working for the bottom button container.<span>
has d-shown="{{ header !== '' }}"
, which presumably won't work because the this.
is only optional in simple cases like {{header}}
, not in expressions like header !== ''
. I'm not sure setting d-shown=false
on an empty <span>
does much anyway.
2) Looks like the <input>
doesn't have aria-labelledby
but presumably it's needed (all <input>
s need labels).
3) <button>
nodes shouldn't need a tabindex=0 setting as that's the default.
Oh also, you need to run grunt jshint
, it's getting errors.
This was actually checked in a long time ago in 5947b3cbc4c7c75ee6d5846342bbb738b8c72c48.
These attributes were "closing" the "focus context" for users using iOS + VoiceOver. Removing the attributes open the sibling context and user is able to focus the buttons using swipe or keyboard shortcuts.
Fix #708